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

Add "Zoom to here" link in popup #831

Merged
merged 10 commits into from
Apr 28, 2023
Merged

Add "Zoom to here" link in popup #831

merged 10 commits into from
Apr 28, 2023

Conversation

yhy0217
Copy link
Contributor

@yhy0217 yhy0217 commented Apr 25, 2023

Closes #778

  • create shadowRoot for templated features and queries
  • add functional "zoom to here" links in popups of inline-static / templated / queried features
  • fix bugs in MapFeature, add tests for MapFeature.addEventListener
  • add tests for zoomTo link

@yhy0217 yhy0217 marked this pull request as ready for review April 26, 2023 15:49
@prushforth
Copy link
Member

prushforth commented Apr 26, 2023

@yhy0217 I put a build of this into GeoServer to test it against queryable layers, and I get the following on a query that should return a <map-feature> with a non-null <map-geometry>:

image

On the referenced line 401 of mapml.js,

 feature._extentEl.shadowRoot.firstChild?.remove();

the ._extentEl is null

The document returned by the query is:

-big, not relevant

@prushforth
Copy link
Member

the ._extentEl is null

was mistaken, it's the shadowRoot property of ._extentEl that is null

@prushforth
Copy link
Member

Closes #778

@prushforth
Copy link
Member

This is really working well now - the custom projection query is even working on the shrink/swell layer - reveals a "Zoom to here" link that works - thanks for working on this, too, @AliyanH. Also, I noticed that the extension's "Zoom to here" link is working correctly now, for example when rendering a GeoServer WFS layer, the zoom to here link is correct. Overall, this is excellent work. I'll keep reviewing, but I think it's ready to merge.

Copy link
Member

@prushforth prushforth left a comment

Choose a reason for hiding this comment

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

LGTM!

@prushforth
Copy link
Member

Seems like the popup is hard to get rid of after clicking on the "Zoom to here" link, check the restaurants experiment.

@yhy0217 yhy0217 merged commit 0f94342 into Maps4HTML:main Apr 28, 2023
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.

Add a link in the popup for a feature for "Zoom to here"
3 participants