Fix off by one with default value in pagination #207
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Before: The current page for some values would not include the selected index. ```rb irb(main):007* (1..100).filter_map do |index| irb(main):008* page = TTY::Prompt::Paginator.new.paginate((1..101).map(&:to_s), index, 6).to_a irb(main):009* next if page.map(&:last).include?(index-1) irb(main):010* irb(main):011* [index, page] irb(main):012> end => [[12, [["18", 17], ["19", 18], ["20", 19], ["21", 20], ["22", 21], ["23", 22]]], [18, [["30", 29], ["31", 30], ["32", 31], ["33", 32], ["34", 33], ["35", 34]]], [24, [["42", 41], ["43", 42], ["44", 43], ["45", 44], ["46", 45], ["47", 46]]], [30, [["54", 53], ["55", 54], ["56", 55], ["57", 56], ["58", 57], ["59", 58]]], [36, [["66", 65], ["67", 66], ["68", 67], ["69", 68], ["70", 69], ["71", 70]]], [42, [["78", 77], ["79", 78], ["80", 79], ["81", 80], ["82", 81], ["83", 82]]], [48, [["90", 89], ["91", 90], ["92", 91], ["93", 92], ["94", 93], ["95", 94]]], [54, [["96", 95], ["97", 96], ["98", 97], ["99", 98], ["100", 99], ["101", 100]]], [60, [["96", 95], ["97", 96], ["98", 97], ["99", 98], ["100", 99], ["101", 100]]], [66, [["96", 95], ["97", 96], ["98", 97], ["99", 98], ["100", 99], ["101", 100]]], [72, [["96", 95], ["97", 96], ["98", 97], ["99", 98], ["100", 99], ["101", 100]]], [78, [["96", 95], ["97", 96], ["98", 97], ["99", 98], ["100", 99], ["101", 100]]], [84, [["96", 95], ["97", 96], ["98", 97], ["99", 98], ["100", 99], ["101", 100]]], [90, [["96", 95], ["97", 96], ["98", 97], ["99", 98], ["100", 99], ["101", 100]]]] irb(main):013* (1..100).filter_map do |index| irb(main):014* page = TTY::Prompt::Paginator.new.paginate((1..101).map(&:to_s), index, 20).to_a irb(main):015* next if page.map(&:last).include?(index-1) irb(main):016* irb(main):017* [index, page] irb(main):018> end => [[40, [["60", 59], ["61", 60], ["62", 61], ["63", 62], ["64", 63], ["65", 64], ["66", 65], ["67", 66], ["68", 67], ["69", 68], ["70", 69], ["71", 70], ["72", 71], ["73", 72], ["74", 73], ["75", 74], ["76", 75], ["77", 76], ["78", 77], ["79", 78]]], [60, [["82", 81], ["83", 82], ["84", 83], ["85", 84], ["86", 85], ["87", 86], ["88", 87], ["89", 88], ["90", 89], ["91", 90], ["92", 91], ["93", 92], ["94", 93], ["95", 94], ["96", 95], ["97", 96], ["98", 97], ["99", 98], ["100", 99], ["101", 100]]], [80, [["82", 81], ["83", 82], ["84", 83], ["85", 84], ["86", 85], ["87", 86], ["88", 87], ["89", 88], ["90", 89], ["91", 90], ["92", 91], ["93", 92], ["94", 93], ["95", 94], ["96", 95], ["97", 96], ["98", 97], ["99", 98], ["100", 99], ["101", 100]]]] ``` After: The paginator always returns pages with the selected index. ```rb irb(main):001* (1..100).filter_map do |index| irb(main):002* page = TTY::Prompt::Paginator.new.paginate((1..101).map(&:to_s), index, 6).to_a irb(main):003* next if page.map(&:last).include?(index-1) irb(main):004* irb(main):005* [index, page] irb(main):006> end => [] irb(main):007* (1..100).filter_map do |index| irb(main):008* page = TTY::Prompt::Paginator.new.paginate((1..101).map(&:to_s), index, 20).to_a irb(main):009* next if page.map(&:last).include?(index-1) irb(main):010* irb(main):011* [index, page] irb(main):012> end => [] ```
|
I see the code style checked requirement above. How should I run that locally? I don't see Rubocop in the Gemfile, gem spec or CI file. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe the change
What does this Pull Request do?
This updates the logic to make sure that the default select value is visible on the page when using
TTY::Prompt#selectwithdefault:set.Why are we doing this?
Any related context as to why is this is a desirable change.
This fixes the bug outlined in #206.
Benefits
How will the library improve?
It changes the way that the default value is displayed from before to match the way that we scroll through values in the selector. Basically the default value will never be at the top or bottom of the visible options unless it is the first or last value of the entire list. Before it was possible for a value to appear at the top and when it would have been at the bottom it sometimes caused this bug.
Drawbacks
Possible drawbacks applying this change.
None that I can think of.
Requirements
masterbranch?