fixes scroll into view cases above viewport #279
Conversation
Codecov Report
@@ Coverage Diff @@
## master #279 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 4 4
Lines 311 314 +3
Branches 76 77 +1
=====================================
+ Hits 311 314 +3
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Thank you so much! Do you think you could rebase your branch onto master? Thanks!
README.md
Outdated
| @@ -279,8 +278,7 @@ This function is called anytime the internal state changes. This can be useful | |||
| if you're using downshift as a "controlled" component, where you manage some or | |||
| all of the state (e.g. isOpen, selectedItem, highlightedIndex, etc) and then | |||
| pass it as props, rather than letting downshift control all its state itself. | |||
| The parameters both take the shape of internal state (`{highlightedIndex: | |||
| number, inputValue: string, isOpen: boolean, selectedItem: any}`) but differ | |||
| The parameters both take the shape of internal state (`{highlightedIndex: number, inputValue: string, isOpen: boolean, selectedItem: any}`) but differ | |||
There was a problem hiding this comment.
I'm guessing this reformatting happened because you either didn't install the project dependencies or the recent prettier release has some bugs 🤔
There was a problem hiding this comment.
I tried to follow the instructions as best as I could. Not sure where that was introduced, but somewhere between install and doing the contribution script.
|
will do |
Zashy
force-pushed
the
#274-scroll-last-to-first
branch
from
9822d57
to
d84cfa4
Compare
|
Thanks so much for your help! I've added you as a collaborator on the project. Please make sure that you review the |
* update readme as per contribution instructions * fix scrolling when going from bottom to top when down the page * update unit tests to cover newly uncovered cases * fix readme stuff
What: Fixes #274
Why: See #274. Scrolling was weird when going from last to first item with arrow keys.
How: I added a change I used in my project, and then worked on figuring out the unit tests. On reviewing the unit tests I realized the intent of the previous code was meant for multiple cases when the item is above the current viewport. I worked out that there are three cases. They are hard to explain, so I made an image to better explain.
The first two simplified into the same equation after working them out, and was the same as the existing logic. The third case was the one I had fixed already. I then added in the conditional logic to select the cases, and applied the formulas. I also updated one unit test, and added two new unit tests to account for all three cases I found.
Checklist: