-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Dropdown Menu: Document, test, refactor #2141
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2141 +/- ##
==========================================
+ Coverage 20.33% 22.26% +1.92%
==========================================
Files 136 136
Lines 4274 4272 -2
Branches 724 721 -3
==========================================
+ Hits 869 951 +82
+ Misses 2872 2804 -68
+ Partials 533 517 -16
Continue to review full report at Codecov.
|
keyDown( LEFT ); // activeIndex: 0 | ||
keyDown( UP ); // activeIndex: 3 (reset to end) | ||
|
||
expect( wrapper.state( 'activeIndex' ) ).toBe( 3 ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not test the activeIndex
after each of these keypresses?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not test the
activeIndex
after each of these keypresses?
Good call. Updated in 366e98a.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested keyboard interaction, looks good to me!
836f1c6
to
f40ae0d
Compare
Co-Authored-By: Drapich Piotr <drapich.piotr@gmail.com>
Related: #1975
This pull request seeks to...
onSelect
prop (YAGNI)controls
prop exists before calling on itslength
property (since not a required prop)%
operator for "reset to beginning" logicTesting instructions:
Verify that there are no regressions in the behavior of the Dropdown Menu, currently used only as the last control of the table block. Particularly with note of keyboard behaviors, recently refined in #1975.
Ensure that unit tests pass: