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.

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

Conversation

lasalvavida
Copy link
Contributor

@lasalvavida lasalvavida commented Sep 3, 2019

Fixed issues with changing menu contents in menus with selectable items. Marking selected items in a menu is deferred until the menu closes, so if the menu contents change before the callback is executed, the wrong item can be marked in the DOM.

@codecov-io
Copy link

codecov-io commented Sep 3, 2019

Codecov Report

Merging #5047 into master will increase coverage by 0.29%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5047      +/-   ##
==========================================
+ Coverage   98.79%   99.09%   +0.29%     
==========================================
  Files         122      122              
  Lines        5830     5831       +1     
  Branches      763      763              
==========================================
+ Hits         5760     5778      +18     
+ Misses         69       52      -17     
  Partials        1        1
Impacted Files Coverage Δ
packages/mdc-menu/foundation.ts 96.29% <100%> (+0.04%) ⬆️
packages/mdc-checkbox/component.ts 96.87% <0%> (+1.04%) ⬆️
packages/mdc-tab/component.ts 98.36% <0%> (+3.27%) ⬆️
packages/mdc-base/component.ts 100% <0%> (+3.57%) ⬆️
packages/mdc-ripple/component.ts 100% <0%> (+3.77%) ⬆️
packages/mdc-auto-init/index.ts 100% <0%> (+4.16%) ⬆️
packages/mdc-ripple/util.ts 97.56% <0%> (+4.87%) ⬆️
packages/mdc-switch/component.ts 98.27% <0%> (+6.89%) ⬆️
packages/mdc-radio/component.ts 95.83% <0%> (+8.33%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7ec18c6...ed2640c. Read the comment docs.

Copy link
Collaborator

@abhiomkar abhiomkar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pull request!

packages/mdc-menu/foundation.ts Show resolved Hide resolved
Copy link
Collaborator

@abhiomkar abhiomkar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, need a unit test test/unit/mdc-menu/mdc-menu.test.js (Preferably without mocks).

packages/mdc-menu/foundation.ts Show resolved Hide resolved
@abhiomkar abhiomkar changed the title fix(mdc-menu): recompute index before marking selection fix(menu): recompute index before marking selection Sep 4, 2019
Fixed issues with changing menu contents in menus with
selectable items. Marking selected items in a menu is
deferred until the menu closes, so if the menu contents
change before the callback is executed, the wrong item
can be marked in the DOM.
Copy link
Collaborator

@abhiomkar abhiomkar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@abhiomkar abhiomkar merged commit 90f6247 into material-components:master Sep 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants