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

docs: Use Popup in Maplibre/Mapbox getting started example #8956

Merged
merged 6 commits into from
Jun 20, 2024

Conversation

felixpalmer
Copy link
Collaborator

@felixpalmer felixpalmer commented Jun 18, 2024

Closes #8530

Background

This is a common use case and it isn't trivial as it isn't obvious that it isn't possible to do this with the "reverse-controlled" React pattern.

maplibre-popup

Change List

  • Use Popup component instead of alert()

@coveralls
Copy link

Coverage Status

coverage: 89.563%. remained the same
when pulling e79734a on felix/maplibre-popup
into f1a163a on master.

@felixpalmer felixpalmer mentioned this pull request Jun 18, 2024
Copy link
Collaborator

@Pessimistress Pessimistress left a comment

Choose a reason for hiding this comment

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

Should the mapbox example be updated as well?

examples/get-started/react/maplibre/app.jsx Outdated Show resolved Hide resolved
examples/get-started/react/maplibre/app.jsx Outdated Show resolved Hide resolved
Copy link
Collaborator

@chrisgervang chrisgervang left a comment

Choose a reason for hiding this comment

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

Definitely a more useful example this way. Though, I'm not sure I understand your comment about reverse-control since this isn't that. Those examples are all in website whereas this is considered to be using the overlay pattern.

@felixpalmer
Copy link
Collaborator Author

@chrisgervang what I was getting at regarding "reverse-controlled" is that newcomers will look at our examples (get-started or website) and pick whatever looks most appropriate, not realizing that they've chosen an integration mode. Then when it comes to adding a Popup, they might get stuck if they "accidentally" picked a reverse-controlled pattern.

@coveralls
Copy link

Coverage Status

coverage: 89.404% (-0.2%) from 89.563%
when pulling 3d1ee38 on felix/maplibre-popup
into f1a163a on master.

@coveralls
Copy link

Coverage Status

coverage: 89.404%. remained the same
when pulling 3188b94 on felix/maplibre-popup
into 0af27a1 on master.

@felixpalmer felixpalmer changed the title docs: Use Popup in Maplibre getting started example docs: Use Popup in Maplibre/Mapbox getting started example Jun 20, 2024
@coveralls
Copy link

Coverage Status

coverage: 89.404%. remained the same
when pulling 3fddcf7 on felix/maplibre-popup
into 0af27a1 on master.

@felixpalmer felixpalmer merged commit 537f80b into master Jun 20, 2024
4 checks passed
@felixpalmer felixpalmer deleted the felix/maplibre-popup branch June 20, 2024 09:06
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.

[Bug] maplibre / mapbox Popup shows below the deck.gl features in non-interleaved MapboxOverlay
4 participants