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

Fixes for Geocoder when hosted via web components #8425

Merged
merged 1 commit into from
Dec 3, 2019

Conversation

mramato
Copy link
Contributor

@mramato mramato commented Dec 1, 2019

The Geocoder suggestion box does not currently work if hosted inside of a web component. I tracked it down to the use of container.contains(e.target) in _onInputBegin. Because of the shadow DOM, this check always evaluates to false inside of web components and search suggestions are never shown.

I'm not sure it's possible to make container.contains(e.target) work in a web components the way we were relying on it, but the entire thing way we were handling this seemed a little convoluted to begin with so I removed the use of document pointer/mouse events and made it use standard blur/focus events instead. As far as I can tell behavior is identical but now the code should be more portable.

@cesium-concierge
Copy link

Thanks for the pull request @mramato!

  • ✔️ Signed CLA found.
  • CHANGES.md was not updated.
    • If this change updates the public API in any way, please add a bullet point to CHANGES.md.
  • ❔ Unit tests were not updated.
    • Make sure you've updated tests to reflect your changes, added tests for any new code, and ran the code coverage tool.

Reviewers, don't forget to make sure that:

  • Cesium Viewer works.
  • Works in 2D/CV.
  • Works (or fails gracefully) in IE11.

@mramato mramato requested a review from hpinkos December 1, 2019 18:31
@hpinkos
Copy link
Contributor

hpinkos commented Dec 1, 2019

@mramato clicking the autocomplete results appears to be broken?

@mramato
Copy link
Contributor Author

mramato commented Dec 2, 2019

@mramato clicking the autocomplete results appears to be broken?

Damn it, I thought I fixed that. The problem is that blur is processed before click so clicking on the suggestion hides the box and then the click doesn't happen. I had a settimeout in there to fix it which worked for me but I'm guessing it's not a good enough fix. I'll see if I can find another way.

…nts.

Geocoder was relying on `Event.target` which does not work with the
Shadow DOM. This prevented suggestions from appearing inside of web
components. Detect if shadow DOM is being used and access the composedPath
to retrieve the real target if so.

Also remove unnecessary contains check for onInputEnd because we can
just listen subscribe to the container itself rather than the entire
document.
@mramato
Copy link
Contributor Author

mramato commented Dec 3, 2019

@hpinkos I had to bail on my initial approach because of the impossibility of handling a click after blur for the same element. Instead, I figured out a way to make e.target and node.contains work in the shadow DOM. Here's the new approach:

Geocoder was relying on Event.target which does not work with the Shadow DOM. This prevented suggestions from appearing inside of web components. Detect if shadow DOM is being used and access the composedPath to retrieve the real target if so.

Also remove unnecessary contains check for onInputEnd because we can just listen subscribe to the container itself rather than the entire document.

@hpinkos
Copy link
Contributor

hpinkos commented Dec 3, 2019

👍 works great, thanks @mramato!

@hpinkos hpinkos merged commit 0dbc863 into master Dec 3, 2019
@hpinkos hpinkos deleted the geocoder-web-components branch December 3, 2019 15:05
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 this pull request may close these issues.

4 participants