geoblocks / ol-maplibre-layer

Use a MapBox map as an OpenLayers layer
https://geoblocks.github.io/ol-maplibre-layer/demo.html
BSD 3-Clause "New" or "Revised" License
35 stars 5 forks source link

Use a renderer instead of a custom render function #173

Closed oterral closed 2 months ago

oterral commented 2 months ago

On the demo, open the console then click on the map you will see the features clicked.

The attribution value come also directly from the maplibre style sources.

@ger-benjamin It's a first shot. If you are still interested please review, then I will add tests and fixes ts issues.

Fix #154

ger-benjamin commented 2 months ago

Hi @oterral , and thanks for your contribution. We want to accept it, that looks promising.

Could you please address my (future) comments ?

Side note: We have to says that we don't have a lot of direct implementation of this project, we use it more as reference (for some reasons, more or less relevant). That's why it's not in the best shape currently. On my side, I'll take time to update it a bit. It deserves it :-)

I'll do that after your merge, but before the next release (will be a minor one: 0.2.0)

oterral commented 2 months ago

thanks @ger-benjamin for the review. I make the fixes asap

oterral commented 2 months ago

@ger-benjamin ready for a 2nd review

ger-benjamin commented 2 months ago

Thanks @oterral , that works well, and the code looks nice now. Can you please rebase (remove, install and re-push the package-lock.json) and ev. squash your commits ? Then I'll merge it :-)

oterral commented 2 months ago

@ger-benjamin master is merged . If you can squash the commit when you merge, it will be nice.

ger-benjamin commented 2 months ago

Mayn thanks, I'll do a release after some fix. In a near future.