-
Notifications
You must be signed in to change notification settings - Fork 0
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
GSLUX-753: Legend panel #167
Conversation
GitHub Pages links: * Luxembourg-geoportail: https://geoportail-luxembourg.github.io/luxembourg-geoportail/GSLUX-753-legends/ |
f1b192e
to
53e5f01
Compare
53e5f01
to
31b301f
Compare
9a3a2a6
to
c05dac4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the various refactorings @AlitaBernachot ! Works as expected :-)
layersOpen, | ||
legendsOpen, | ||
myMapsOpen, | ||
styleEditorOpen, | ||
themeGridOpen, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the refactoring! Maybe we could even put all these into one sidePanelOpen
variable with predefined values defining its state in the future (not within this PR). WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean something like :
const sidePanelOpen = ref({
layersOpen: true,
legendsOpen: true,
myMapsOpen: true,
...
})
I am not a big fan of this solution because it means when a component is linked to themeGridOpen property only (for example), it will also trigger a refresh when the other properties are changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, good point. I was rather thinking of something like:
type SidePanelOpenState = 'layersOpen' | 'legendsOpen' | ...
const sidePanelOpenState = ref<SidePanelOpenState>('layersOpen')
or maybe using an enum, but just as an idea for future refactoring...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh! right! this is much better than what I thought. Well, why not :). I am merging this PR, feel free to improve this side panel state.
JIRA issue
https://jira.camptocamp.com/browse/GSLUX-753
Description
side-panel.vue
component, now using watchers, allowing v3 to have its own custom behavior in MainController.js with both angular and vuejs' watchers, as this new side-panel is not yet in v3