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 issue 299 MutationObserver for markers in Shadow DOM #306

Merged
merged 2 commits into from
Jun 19, 2016

Conversation

mlisook
Copy link
Contributor

@mlisook mlisook commented Jun 18, 2016

In issue #299 it is observed that changes to the google-map-marker collection are not picked up by the google-map under Shadow dom as they are under Shady dom. The code uses a MutationObserver to look for changes, but the content is inside an iron-selector:

<iron-selector id="selector" multi="[[!singleInfoWindow]]" selected-attribute="open" activate-event="google-map-marker-open" on-google-map-marker-close="_deselectMarker">
      <content id="markers" select="google-map-marker"></content>
    </iron-selector>
    <content id="objects" select="*"></content>

this._mutationObserver = new MutationObserver(this._updateMarkers.bind(this));
this._mutationObserver.observe(this.$.selector, {
  childList: true
});

That makes content#markers light dom to the iron-selector, but shadow/shady dom to google-map. Mutation events do not bubble to the outside from within shadow dom. There is even a test for this in the Chromium project - LayoutTests/fast/dom/MutationObserver/shadow-dom.html.

In any case, iron-selector is already observing changes to content#markers. In this PR the google-map is modified to listen for changes to the items property of iron-selector#selector instead of using a MutationObserver.

The existing test, markers-add-remove.html, illustrates both the issue and this fix. With the fix we can add the dom=shadow variety of this test to the list of suites to run.

this._mutationObserver.observe(this.$.selector, {
childList: true
});
this._mutationObserver = this.listen(this.$.selector, 'items-changed', '_updateMarkers');
Copy link
Contributor

Choose a reason for hiding this comment

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

let's rename this property to something more appropriate: this._markersChildrenListener

@ebidel
Copy link
Contributor

ebidel commented Jun 18, 2016

Nice change. Just the name suggestion.

@mlisook
Copy link
Contributor Author

mlisook commented Jun 18, 2016

I made the name change as suggested to _markersChildrenListener.

@ebidel
Copy link
Contributor

ebidel commented Jun 19, 2016

LGTM

@ebidel ebidel merged commit 37f7203 into GoogleWebComponents:master Jun 19, 2016
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.

3 participants