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

[IconMenu] Fix controlled IconMenus to honor onRequestChange #5704

Merged
merged 1 commit into from Dec 2, 2016

Conversation

hai-cea
Copy link
Member

@hai-cea hai-cea commented Dec 2, 2016

  • PR has tests / docs demo, and is linted.
  • Commit and PR titles begin with [ComponentName], and are in imperative form: "[Component] Fix leaky abstraction".
  • Description explains the issue / use-case resolved, and auto-closes the related issue(s) (http://tr.im/vFqem).

This PR fixes a problem that allows controlled icon menus to close on its own. Now, if the icon menu is controlled, it will just call onRequestChange and not automatically close the menu.

@oliviertassinari
Copy link
Member

That looks good to me. Thanks.
That could be considered a breaking change, but I would rather see this as a bug. That would have been better with a test.

@oliviertassinari
Copy link
Member

oliviertassinari commented Dec 2, 2016

@hai-cea Your last PR was some time ago. I'm glad to see you back 😄 .

@hai-cea
Copy link
Member Author

hai-cea commented Dec 2, 2016

Thanks @oliviertassinari - Trying to get back at it. I will do better next time with tests. :)

@hai-cea hai-cea deleted the feature-iconmenu-fix branch November 3, 2017 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change bug 🐛 Something doesn't work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants