Skip to content
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

Merged
merged 6 commits into from
Nov 4, 2024
Merged

GSLUX-753: Legend panel #167

merged 6 commits into from
Nov 4, 2024

Conversation

AlitaBernachot
Copy link
Contributor

@AlitaBernachot AlitaBernachot commented Oct 22, 2024

JIRA issue

https://jira.camptocamp.com/browse/GSLUX-753

Description

  • New Legends panel
  • refactor getMetadata and getLegend to be used in metadata opup and in legends panel
  • add cache for getMetadata() (cache is cleared when language is changed)
  • update chore dep i18next-vue to last release
  • refactor app store => setters have been moved to new 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

Copy link
Contributor

GitHub Pages links: * Luxembourg-geoportail: https://geoportail-luxembourg.github.io/luxembourg-geoportail/GSLUX-753-legends/

@AlitaBernachot AlitaBernachot force-pushed the GSLUX-753-legends branch 8 times, most recently from f1b192e to 53e5f01 Compare October 23, 2024 16:45
Copy link
Contributor

@tkohr tkohr left a 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 :-)

cypress/e2e/legends/legends.cy.ts Outdated Show resolved Hide resolved
Comment on lines +14 to +18
layersOpen,
legendsOpen,
myMapsOpen,
styleEditorOpen,
themeGridOpen,
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor

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...

Copy link
Contributor Author

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.

@AlitaBernachot AlitaBernachot merged commit 55e4941 into main Nov 4, 2024
2 checks passed
@AlitaBernachot AlitaBernachot deleted the GSLUX-753-legends branch November 4, 2024 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants