Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Page-Up and Page-Down key for list prompt #69

Closed
wants to merge 1 commit into from

Conversation

crazywhalecc
Copy link
Contributor

This PR allows list-related prompts to support page-turning display using the Page-Up and Page-Down keys.

record

@taylorotwell taylorotwell marked this pull request as draft September 14, 2023 13:00
@jessarcher
Copy link
Member

This is pretty cool, but I'm not sure if the added complexity is worth it. I'm also not sure whether it's a standard terminal convention - it's not listed at https://en.wikipedia.org/wiki/GNU_Readline

I'd be more open to Home/Ctrl+A and End/Ctrl+E that would go to the top and bottom if you're in a list, or to the beginning or end of a typed input.

@crazywhalecc
Copy link
Contributor Author

crazywhalecc commented Sep 14, 2023

oh! I already have modified code for various keystrokes changes locally, including Home and End behaviors. Now because I have a multiple selection that needs to display a large number of items, using the Page button can improve search efficiency.

I have used the dialog command before, which can support Page key turning very well, so I want to implement it first. I understand your considerations: it is not a standard in GNU Readline. But if some terminals do not support this behavior, they will just not record and capture these two keystrokes.

Implementing Home and End is much simpler than Page buttons. If this feature is not acceptable or needs to be reconsidered, I can complete the Home and End functions first. For Home/End/PgUp/PgDn key, btw, if highlightPrevious() could pass in a $offset parameter, or just refactor it with updateHighlighted($highlight), it might save a lot of repetitive code.

@jessarcher
Copy link
Member

Now because I have a multiple selection that needs to display a large number of items, using the Page button can improve search efficiency.

Would the multisearch prompt from #58 be better if you have a large number of items to search?

Implementing Home and End is much simpler than Page buttons. If this feature is not acceptable or needs to be reconsidered, I can complete the Home and End functions first. For Home/End/PgUp/PgDn key, btw, if highlightPrevious() could pass in a $offset parameter, or just refactor it with updateHighlighted($highlight), it might save a lot of repetitive code.

I'm open to a refactor if it simplifies the code and mental overhead. There are quite a lot of moving parts and the more features like this we add, the harder it becomes to maintain and regression test later. If the Home/End stuff is easier, I'd like to see that first. But if your refactor idea essentially gives us the other features for free then that would be cool too. I really like what we did in #43 where I felt some of the code became simpler while also unlocking new features at the same time 🙌

It might also be best to wait until #58 is merged or closed as it would need to be considered in anything we do here.

Incidentally, I have some ideas to improve the testability of these sorts of interactions that would allow us to assert against the complete output of individual render cycles in a sequence.

@crazywhalecc
Copy link
Contributor Author

I'm willing to wait for the multiple searches to complete. This also gave me time to write some tests on the refactored function.

@jessarcher
Copy link
Member

Hey @crazywhalecc, #58 is merged now.

Does the new multisearch prompt remove your need to add these additional navigation keys? It seems better suited to large lists.

If not, feel free to update this PR, and we can review it again. As mentioned previously, I'd prefer to start by adding Home/Ctrl+A and End/Ctrl+E when a list is highlighted, as we already have these keys when a typed input is focused.

If that is successful, we could look at PgUp/PgDown in a future PR.

@crazywhalecc
Copy link
Contributor Author

Thanks! I will work on it. And another small question: What should the behavior of the Home key look like in Suggest and Search? Assuming that I am already selecting an item, should I go back to the input box or highlight the first item?

@jessarcher
Copy link
Member

What should the behavior of the Home key look like in Suggest and Search? Assuming that I am already selecting an item, should I go back to the input box or highlight the first item?

It's hard to know without playing with it, but I think if you're currently navigating the list (i.e. $this->highlighted is not null), it would go to the top/bottom of the list. But if you're in the text input (i.e. $this->highlighted is null), it would go to the start/end of the typed input.

I think you'd need to use the new $ignore callback argument on the trackTypedValue method to have it ignore those specific keys when $this->highlighted is not null. Have a look at MultiSearchPrompt for an example.

@crazywhalecc
Copy link
Contributor Author

Updated in #79.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants