The Wayback Machine - https://web.archive.org/web/20220309004236/https://github.com/mapbox/mapbox-gl-js/issues/11277
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

Jump to bottom

map.setProjection should return a map object for function call chaining #11277

Open
karimnaaji opened this issue Nov 12, 2021 · 4 comments
Open

map.setProjection should return a map object for function call chaining #11277

karimnaaji opened this issue Nov 12, 2021 · 4 comments
Labels
api 📝 good first issue
Milestone
projections

Comments

@karimnaaji
Copy link
Member

@karimnaaji karimnaaji commented Nov 12, 2021

Most our set* APIs return a map object to allow for function call chaining, e.g.:

map.setProjection().setZoom().set...

Currently, map.setProjection doesn't follow that pattern. We should update that for API consistency.

Refer https://github.com/mapbox/mapbox-gl-js/blob/main/src/ui/map.js#L1020_L1027.

cc @ryanhamley
@karimnaaji karimnaaji added this to the projections milestone Nov 12, 2021
@lpizzinidev
Copy link
Contributor

@lpizzinidev lpizzinidev commented Nov 13, 2021

Hi @karimnaaji. Can I work on this issue?
@karimnaaji
Copy link
Member Author

@karimnaaji karimnaaji commented Nov 13, 2021

Hello @lpizzinidev , yes!
lpizzinidev added a commit to lpizzinidev/mapbox-gl-js that referenced this issue Nov 15, 2021
@lpizzinidev
Added function call chaining to function
d92163b 
Addresses mapbox#11277
@lpizzinidev lpizzinidev mentioned this issue Nov 15, 2021
Merged
3 tasks
@ryanhamley
Copy link
Contributor

@ryanhamley ryanhamley commented Nov 15, 2021

Closed by e534c87 Thanks @lpizzinidev!
@ryanhamley ryanhamley closed this Nov 15, 2021
@karimnaaji karimnaaji reopened this Jan 19, 2022
@karimnaaji
Copy link
Member Author

@karimnaaji karimnaaji commented Jan 19, 2022

Reopening as it was noticed that map.setProjection should also trigger an update, refer to @rreusser and @tristen examples in https://observablehq.com/d/c3799d41f58a5858 vs https://observablehq.com/compare/c3799d41f58a5858...f4c80005a5598a3e, where the first doesn't trigger a new repaint when editing the projection properties.

I think the fix should be as simple as using return this._update() instead of this, but we'd just have to add a test case covering this issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api 📝 good first issue
3 participants
@ryanhamley @karimnaaji @lpizzinidev