Prevent duplication of color steps when reopening the symbology panel #697
Conversation
|
|
|
Integration tests report: appsharing.space |
| output: color[key][i + 1] | ||
| }); | ||
| const pairKey = `${color[key][i]}-${color[key][i + 1]}`; | ||
| if (!seenPairs.has(pairKey)) { |
There was a problem hiding this comment.
I'm a bit worried about this approach, looks a lot like you're fixing the symptom and not the problem itself. Are the pair/value duplicated in layer.parameters?.color?
There was a problem hiding this comment.
Are the pair/value duplicated in layer.parameters?.color?
No, they were duplicating in this method only as in #672, I changed from switch case using a condition to a for loop using a list iterating for both ['fill-color', 'circle-fill-color'] resulting in duplication
There was a problem hiding this comment.
In which case do we have that pairKey already there? It's when you have a duplication in layer.parameters?.color, right? That's what I understand from the code at least.
There was a problem hiding this comment.
We have that pairKey already there during the second iteration of for loop for 'circle-fill-color' after the first iteration of for loop for 'fill-color'
I cannot see the duplication in layer.parameters?.color
There was a problem hiding this comment.
Ok got it!
I'm wondering how much we could simplify the schema itself to only have color steps and radius steps, not dealing with open-layers implementation details about circle-fill-color and fill-color at all in the schema.
Not part of this PR though. Thanks!
There was a problem hiding this comment.
I'm wondering how much we could simplify the schema itself to only have color steps and radius steps, not dealing with open-layers implementation details about circle-fill-color and fill-color at all in the schema.
If we do this, the schema would be closer to what we see in the Symbology panel, and this code would be simplified quite a lot and more robust.
Description
Fix #693
sym.mp4
Checklist
Resolves #XXX.Failing lint checks can be resolved with:
pre-commit run --all-filesjlpm run lint📚 Documentation preview: https://jupytergis--697.org.readthedocs.build/en/697/
💡 JupyterLite preview: https://jupytergis--697.org.readthedocs.build/en/697/lite