Conversation
src/App.svelte
Outdated
| // in MapStyleInputWrapper are coming from polled styles or from history | ||
| const nextMaps = JSON.parse(JSON.stringify(maps)).map(m => { | ||
| // cloneDeep copies over functions in case those are the values | ||
| const nextMaps = _.cloneDeep(maps).map(m => { |
There was a problem hiding this comment.
We have to replace all clones using JSON.parse(JSON.stringify(...)) with cloneDeep or something similar to prevent losing functions passed as values from the config.
There was a problem hiding this comment.
I'd lean toward es-toolkit when possible, it's more modern / has a smaller footprint than lodash.
src/components/GlMap.svelte
Outdated
| let isUrl = !style && typeof url === 'string'; | ||
| let stylesheet = style; | ||
| if (isUrl) { | ||
| stylesheet = await fetchUrl(url); |
There was a problem hiding this comment.
Do you think we should just do this if we need a precompile rather than always? Seems fairly quick, but maybe better to skip a fetch when possible.
There was a problem hiding this comment.
Yes I think letting the map library handle the fetch / cache / etc would be good if we're not modifying the style.
ebrelsford
left a comment
ebrelsford
left a comment
There was a problem hiding this comment.
Having a slightly hard time testing it, biggest thing is I think we should document it.
src/App.svelte
Outdated
| // in MapStyleInputWrapper are coming from polled styles or from history | ||
| const nextMaps = JSON.parse(JSON.stringify(maps)).map(m => { | ||
| // cloneDeep copies over functions in case those are the values | ||
| const nextMaps = _.cloneDeep(maps).map(m => { |
There was a problem hiding this comment.
I'd lean toward es-toolkit when possible, it's more modern / has a smaller footprint than lodash.
| - Shorten URLs by reducing map state kept in the hash to the minimum | ||
| - Don't set RTL text plugin if already in progress | ||
| - Add branch name to screenshots if looking at branch pattern | ||
| - Add precompile option |
There was a problem hiding this comment.
Can we also document this?
src/components/GlMap.svelte
Outdated
| let isUrl = !style && typeof url === 'string'; | ||
| let stylesheet = style; | ||
| if (isUrl) { | ||
| stylesheet = await fetchUrl(url); |
There was a problem hiding this comment.
Yes I think letting the map library handle the fetch / cache / etc would be good if we're not modifying the style.
|
@ebrelsford ok, I believe I've gotten this sorted out now. The issue was that I wasn't compiling on first render so it was trying to handle some sources that should be stripped off inappropriately. Now it's set to use a default value on first render if a selected option isn't available. |
ebrelsford
left a comment
ebrelsford
left a comment
There was a problem hiding this comment.
Looking good! When I click a label on the right map it seems to trigger the checkbox on the left map? Presumably because currently the ids for checkboxes are reused? They appear to be simply the index of the option, but should be globally unique.
|
Ok, I believe I have fixed everything now. I also found a bug where adding a map wasn't behaving as expected. Now when you add a map, it will default to the virtual variant of the previous map, but maintain default value on switching the map style in that pane. |
2 participants
Description
Alternative to #243
More likely to work more smoothly for our needs and cover future compiling use cases.
You need to input the compilation script, so this will work for various compiling use cases if users build their own spec.
QA steps
Author checklist
Create the PR
After approval
maininto your branch, resolve any conflictsmain