feat(menu): Add quickOpen option. #2127
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2127 +/- ##
==========================================
+ Coverage 99.13% 99.14% +<.01%
==========================================
Files 90 90
Lines 3831 3840 +9
Branches 491 495 +4
==========================================
+ Hits 3798 3807 +9
Misses 33 33
Continue to review full report at Codecov.
|
packages/mdc-menu/foundation.js
Outdated
| @@ -208,6 +210,11 @@ class MDCMenuFoundation extends MDCFoundation { | |||
| this.setSelectedIndex(-1); | |||
| } | |||
|
|
|||
| /** @param {boolean} quickOpen */ | |||
| quickOpen(quickOpen) { | |||
There was a problem hiding this comment.
Should this be called setQuickOpen? The current name in the context of a method makes it sound like it's actually going to open the menu.
| this.adapter_.addClass(MDCMenuFoundation.cssClasses.ANIMATING_OPEN); | ||
|
|
||
| if (!this.quickOpen_) { | ||
| this.adapter_.addClass(MDCMenuFoundation.cssClasses.ANIMATING_OPEN); |
There was a problem hiding this comment.
We should probably also conditionalize the setTimeout code below (and same for within close), although it's effectively a no-op in this case right now.
| td.verify(mockAdapter.addClass(cssClasses.ANIMATING_OPEN), {times: 1}); | ||
| }); | ||
|
|
||
| testFoundation('#open does not add the animation class to start an animation when quickOpen is false', |
There was a problem hiding this comment.
Missing a complementary test for #close? (Also test both addClass and removeClass after addressing my comment above.)
packages/mdc-menu/README.md
Outdated
| @@ -150,6 +153,7 @@ Property | Value Type | Description | |||
| `open` | Boolean | Proxies to the foundation's `isOpen`/(`open`, `close`) methods. | |||
| `items` | Array<Element> | Proxies to the foundation's container to query for all `.mdc-list-item[role]` elements. | |||
| `itemsContainer` | Element | Queries the foundation's root element for the `mdc-menu__items` container element. | |||
| `quickOpen` | Boolean | Proxies to the foundation's `quickOpen()` method. | |||
There was a problem hiding this comment.
Description needs updating to setQuickOpen() now
There was a problem hiding this comment.
It also occurs to me that setQuickOpen needs to be documented under the Foundation API... and make sure to mention that it affects both open and close animations. (Which makes me think maybe this isn't the best name, but not sure what is... maybe flip it to enableAnimations, but maybe that's too vague?)
packages/mdc-menu/README.md
Outdated
| @@ -201,6 +205,7 @@ Method Signature | Description | |||
| `open({focusIndex: ?number}) => void` | Opens the menu. Optionally accepts an object with a `focusIndex` parameter to indicate which list item should receive focus when the menu is opened. | |||
| `close(evt: ?Event)` | Closes the menu. Optionally accepts the event to check if the target is disabled before closing the menu. | |||
| `isOpen() => boolean` | Returns a boolean indicating whether the menu is open. | |||
| `setQuickOpen(quickOpen: boolean) => void` | Sets the menu to open and close without animation when the `open/close` methods are called. | |||
There was a problem hiding this comment.
I changed it to "Sets whether the menu should open and close..." based on every other "set" method description in this readme starting with "Sets".
williamernest
force-pushed
the
feat/menu/open-menu-no-animation
branch
from
4e4a51f to
3b43351
Compare
williamernest
force-pushed
the
feat/menu/open-menu-no-animation
branch
from
3b43351 to
f1d7d4d
Compare
williamernest
force-pushed
the
feat/menu/open-menu-no-animation
branch
from
d286d22 to
64e6319
Compare
Fixes: #1702
Disables open and closing animations for the mdc-menu element.