Skip to content

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

Merged
felixpalmer merged 6 commits intomasterfrom
felix/maplibre-popup
Jun 20, 2024
Merged

docs: Use Popup in Maplibre/Mapbox getting started example#8956
felixpalmer merged 6 commits intomasterfrom
felix/maplibre-popup

Conversation

@felixpalmer
Copy link
Copy Markdown
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
Copy Markdown

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
17 tasks
Copy link
Copy Markdown
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?

Comment thread examples/get-started/react/maplibre/app.jsx Outdated
Comment thread examples/get-started/react/maplibre/app.jsx Outdated
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown

Coverage Status

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

@coveralls
Copy link
Copy Markdown

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
Copy Markdown

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
@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