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 shortcuts to navigate Checkbox lists faster #259

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

solanav
Copy link

@solanav solanav commented Oct 5, 2022

What is the problem that this PR addresses?
When offering a very long list of choices in the Checkbox question, traversing it is very slow.

How did you solve it?
I added four new bindings to the following keys:

  • Home: Go to the first choice.
  • End: Go to the last choice.
  • NextPage: Advance 10 positions.
  • LastPage: Go back 10 postions.

Checklist

  • I have read the Contributor's Guide.
  • I will check that all automated PR checks pass before the PR gets reviewed.

@kiancross
Copy link
Collaborator

Thanks for the PR, this looks great. Would you be able to add some unit tests?

@solanav
Copy link
Author

solanav commented Nov 29, 2022

Done, tell me if you see something you want me to change!

Copy link
Collaborator

@kiancross kiancross left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay in reviewing this. I've left some questions/comments below.


@bindings.add(Keys.PageDown, eager=True)
def move_cursor_down_fast(event):
for _ in range(10):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does everything work correctly if there are fewer than 10 items? For example, if there are 9 items, page down should go to the 9th. Could you write a test for this?

for _ in range(10):
ic.select_next()

while not ic.is_selection_valid():
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if you have a list with 1 valid item and 99 invalid items? In such a case, I think this might cause an infinite loop (unless ic.select_next() loops around to the start of the list)? Can you write a test to cover this.

@baturayo
Copy link
Contributor

Great work thanks lot! I hacked the paging in my application. For example, for my use case I need to have 20 items in one page. Can we make 10 items dynamic and configurable? @solanav

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

Successfully merging this pull request may close these issues.

3 participants