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

Move overlay wiring out from the ready block #138

Closed
tomivirkki opened this issue Jun 27, 2018 · 1 comment · Fixed by #157
Closed

Move overlay wiring out from the ready block #138

tomivirkki opened this issue Jun 27, 2018 · 1 comment · Fixed by #157
Assignees

Comments

@tomivirkki
Copy link
Member

tomivirkki commented Jun 27, 2018

All the code in this block is ignored if this._overlayElement.content doesn't happen to exist when ready() is invoked: https://github.com/vaadin/vaadin-dropdown-menu/blob/master/src/vaadin-dropdown-menu.html#L340

And since ready() is invoked only once, the component can remain broken for the rest of its lifespan.

Add a MutationObserver to the component instead and do the wiring only once a template or a renderer is received.

@tomivirkki tomivirkki added the needs triage Has to be triaged by the team label Jun 27, 2018
@tomivirkki tomivirkki added backlog and removed needs triage Has to be triaged by the team labels Aug 10, 2018
@web-padawan
Copy link
Member

Note: also breaks the renderer function when using in lit-element
https://glitch.com/edit/#!/zesty-bumper?path=app.js:35:11

@tomivirkki tomivirkki added needs triage Has to be triaged by the team and removed backlog labels Aug 27, 2018
@yuriy-fix yuriy-fix added in progress and removed needs triage Has to be triaged by the team labels Aug 27, 2018
@yuriy-fix yuriy-fix self-assigned this Aug 27, 2018
@web-padawan web-padawan assigned web-padawan and unassigned yuriy-fix Sep 3, 2018
@platosha platosha removed the in review label Sep 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants