LayerSwitcher: use editor-layer-index for adding Custom layer 🎉
#298
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Wow, another great PR! Thanks 🙂 I planned to use editor-layer-index as well in future, but i think i wouldn't have enough priority for that 😅
I did only some preliminary code review as this is only a draft. Once you notify me, i will do a proper one. If you needed some help with the polygons i would be happy to (eg.over videocall or whatever you like) 🙂
|
Actually the idea for the layer index is yours. (See issue #232) |
|
@Dlurak is attempting to deploy a commit to the OsmAPP Team Team on Vercel. A member of the Team first needs to authorize it. |
|
While playing around with different layers I noticed this bug: The |
|
@zbycz I think it should be fine now |
|
|
||
| const { url, text } = attribution; | ||
| const innerHtml = createAttributionInnerHtml(attribution); |
There was a problem hiding this comment.
It seems attribution is ripe for refactoring, this is introducing technical debt.
I suggest always storing a react node in the attribution, if this is possible. Then we can construct here a rich object like below, we would make uniq array by keys, and then just output the React nodes. What do you think?
{ key: 'maptiler', node: <MaptilerAttribution/>}{ key: 'URL/KEY-FROM-LAYER-INDEX', node: <a href="">...</a>}{ key: 'URL/KEY-FROM-LAYER-INDEX', node: <>...</>}
We can do it in followup, let me know, if you want to do it yourself or should I. Also if this is a followup it is usually cool to leave a comment saying sth like "TODO this will be refactored in a followup, link-to-issue". So the tech debt doesn't become unmaintainable 🙈
On the other hand if you want to do it here, merge master first, as this file was refactored and moved.
There was a problem hiding this comment.
That's the case now, right?
@Dlurak I think we should show warning in such cases and not allow the user to save the layer. |
| const onClick = () => { | ||
| const TMS_EXAMPLE = TMS_EXAMPLES[counter % TMS_EXAMPLES.length]; | ||
| counter += 1; | ||
| const url = prompt(t('layerswitcher.add_layer_prompt'), TMS_EXAMPLE); // eslint-disable-line no-alert |
There was a problem hiding this comment.
As this PR removes the ability to submit custom URL, can we perhaps add Custom URL as a first option in the Autocomplete dropdown and display the input below?
Followup is ok. Let me know :)
|
Another problem that just occurred to me: |
|
@zbycz I think it should be fine now |
|
Yo! Thats a pretty good job on everything. 👍 🥇 💯 Merging right now! I am so happy to have this finished. I already heard some complains about outdated layers in OsmAPP on the OSM Discord, this will probably make many people happy. 🙂 |
editor-layer-index for adding Custom layer
editor-layer-index for adding Custom layereditor-layer-index for adding Custom layer 🎉
Integrate the layer index.
When adding a new layer a dialog opens where the user can select a layer:
In the next step the user is presented with some details like the description of the layer, here they also need to specify some parts of the url:
Todo:
bestattribute to a layer to indicate if it is the best source for an area, maybe show these with a star icon in the selector