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.

Sign up for GitHub

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

fix(menu): recompute index before marking selection #5047

Merged
merged 2 commits into from
Sep 6, 2019
Merged

fix(menu): recompute index before marking selection #5047

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
33b9554
fix(menu): recompute index before marking selection
lasalvavida Sep 3, 2019
ed2640c
Merge branch 'master' into recompute-index
abhiomkar Sep 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions packages/mdc-menu/foundation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,10 @@ export class MDCMenuFoundation extends MDCFoundation<MDCMenuAdapter> {

// Wait for the menu to close before adding/removing classes that affect styles.
this.closeAnimationEndTimerId_ = setTimeout(() => {
if (this.adapter_.isSelectableItemAtIndex(index)) {
this.setSelectedIndex(index);
// Recompute the index in case the menu contents have changed.
abhiomkar marked this conversation as resolved.
Show resolved Hide resolved
lasalvavida marked this conversation as resolved.
Show resolved Hide resolved
const recomputedIndex = this.adapter_.getElementIndex(listItem);
if (this.adapter_.isSelectableItemAtIndex(recomputedIndex)) {
this.setSelectedIndex(recomputedIndex);
}
}, MDCMenuSurfaceFoundation.numbers.TRANSITION_CLOSE_DURATION);
}
Expand Down
24 changes: 24 additions & 0 deletions test/unit/mdc-menu/menu.foundation.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,30 @@ test('handleItemAction item action event inside of a child element of a selectio
{times: 0});
});

test('handleItemAction adds class to the correct child element of a selection group when menu has mutated', () => {
const {foundation, mockAdapter, clock} = setupTest();
const itemEl = document.createElement('li');
td.when(mockAdapter.elementContainsClass(itemEl, listClasses.LIST_ITEM_CLASS)).thenReturn(true);
td.when(mockAdapter.getElementIndex(itemEl)).thenReturn(1);
td.when(mockAdapter.elementContainsClass(itemEl, cssClasses.MENU_SELECTION_GROUP)).thenReturn(true);

td.when(mockAdapter.isSelectableItemAtIndex(1)).thenReturn(true);
td.when(mockAdapter.getSelectedSiblingOfItemAtIndex(1)).thenReturn(-1);
td.when(mockAdapter.getMenuItemCount()).thenReturn(2);

foundation.handleItemAction(itemEl);

// Element at index 1 is now at index 0
td.when(mockAdapter.getElementIndex(itemEl)).thenReturn(0);
td.when(mockAdapter.isSelectableItemAtIndex(0)).thenReturn(true);
td.when(mockAdapter.getSelectedSiblingOfItemAtIndex(0)).thenReturn(-1);
td.when(mockAdapter.getMenuItemCount()).thenReturn(1);

clock.tick(numbers.TRANSITION_CLOSE_DURATION);

td.verify(mockAdapter.addClassToElementAtIndex(0, cssClasses.MENU_SELECTED_LIST_ITEM), {times: 1});
});

test('handleMenuSurfaceOpened menu focuses the list root element by default on menu surface opened', () => {
const {foundation, mockAdapter} = setupTest();

Expand Down