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

MDCMenu can be closed by user, without changing state #70

Closed
gsteckman opened this issue Jan 9, 2022 · 10 comments · Fixed by #72
Closed

MDCMenu can be closed by user, without changing state #70

gsteckman opened this issue Jan 9, 2022 · 10 comments · Fixed by #72
Assignees
Labels
bug Something isn't working

Comments

@gsteckman
Copy link
Contributor

The MDCMenu can be opened/closed by setting MDCMenuOpts.open. However, when the menu is open the user can close it by clicking off the menu, which makes the menu's actual state inconsistent with the internal program state. This behavior can be observed in the sandbox - Anchored Dropdown example: Click in the textfield to show the menu. Then click off the menu to close it. Now clicking in the textfield again will not show the menu because the internal menuOpen state variable value does not change and so the MenuAnchored function doesn't recompose.

The desired behavior would be either to not have the menu close and fire an event that the user clicked off the menu, or allow the menu to close and fire an event to signal that the menu has closed. This would allow the internal state variable to be modified and cause a recomposition in the first case, or to reset the state variable in the second case to keep it synchronized with the menu's actual state.

@mpetuska
Copy link
Owner

mpetuska commented Jan 9, 2022

Hah, just when I merged in MDCMenu. You must be quickest gun in the west lol

Anyways, thanks for a quick report, I'll look into fixing it today.

@mpetuska mpetuska self-assigned this Jan 9, 2022
@mpetuska mpetuska added the bug Something isn't working label Jan 9, 2022
@mpetuska
Copy link
Owner

mpetuska commented Jan 9, 2022

Unless of course you would like to give it a stab yourself.

@mpetuska
Copy link
Owner

mpetuska commented Jan 9, 2022

For now you could make use of MDCMenuAttrsScope::onSelected event handler to sync-up your open state.

@mpetuska
Copy link
Owner

mpetuska commented Jan 9, 2022

I'll look into how to prevent the menu from closing to make it fully controlled.

@gsteckman
Copy link
Contributor Author

Wouldn't MDCMenuAttrsScope::onSelected only get fired when the user selects one of the MDCMenuItem's?

I was looking at how to maybe fix this, but the MDCMenu API doesn't include an event for closing the menu, nor when the user attempts to close the menu. Another option could be to add a listener to window.document for mousedown events, then check if the menu is visible and the target is not the menu, and if so fire an event to the MDCMenu client. That would be a state synchronization approach, but wouldn't prevent the menu from closing.

@mpetuska
Copy link
Owner

mpetuska commented Jan 9, 2022

Might be worth asking in mdc repo

@mpetuska
Copy link
Owner

mpetuska commented Jan 9, 2022

Also note that open opt is mainly meant for initial render if you want it open preemptivelly

@gsteckman
Copy link
Contributor Author

It looks like MDCMenuSurface has the events for closing, closed, and opened.

I would take a stab at it but I am having issues getting the project to compile on Windows. I was only able to run the sandbox from Windows Subsystem for Linux. But that's separate from this Issue.

@mpetuska
Copy link
Owner

Sandbox makes heavy use of symlinks to reuse shit from main kmdc project. Windows finally added symlink support fairly recently, so Git is likely to need explicit config to enable them.

@mpetuska
Copy link
Owner

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants