-
Notifications
You must be signed in to change notification settings - Fork 34
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
adding global style overrides #1815
adding global style overrides #1815
Conversation
d63b1f5
to
e766c1d
Compare
…eoview into adding-global-style-overrides
…eoview into adding-global-style-overrides
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.
Reviewable status: 0 of 24 files reviewed, 4 unresolved discussions (waiting on @cphelefu and @jolevesq)
packages/geoview-core/src/core/components/layers/left-panel/single-layer.tsx
line 252 at r5 (raw file):
if (layerChildIsSelected && !layerIsSelected && !isGroupOpen) { result.push('selectedLayer'); result.push('bordered-primary');
this can be looks
Also i know selectedLayer
as name done before, but can we follow same pattern like dash bordered-primary
, so that we dnt have mismatch.
Code snippet:
result.push('bordered-primary selectLayer');
packages/geoview-core/src/core/components/layers/left-panel/single-layer.tsx
line 257 at r5 (raw file):
if (layerIsSelected) { result.push('selectedLayer'); result.push('bordered-primary');
same here above
packages/geoview-core/src/core/components/nav-bar/nav-bar.tsx
line 179 at r5 (raw file):
{navBarComponents.includes('location') && <Location />} {navBarComponents.includes('home') && <Home />} {navBarComponents.includes('export') && <ExportButton sxDetails={sxClasses.navButton} className="" />}
Do we need className?
packages/geoview-core/src/ui/style/themeOptionsGenerator.ts
line 212 at r5 (raw file):
styleOverrides: { root: { '&.style1': {
R we still gonna use style1
, style2
, style3
naming convention?
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.
Reviewed 9 of 9 files at r1, 4 of 10 files at r2, 2 of 5 files at r3, 3 of 6 files at r4, 8 of 12 files at r5, all commit messages.
Reviewable status: 17 of 24 files reviewed, 6 unresolved discussions (waiting on @cphelefu and @kaminderpal)
packages/geoview-core/src/ui/style/global-style-overrides.ts
line 17 at r1 (raw file):
}, '.subtitle': {
Was this use for layer left panel tooltip?
packages/geoview-core/src/ui/style/themeOptionsGenerator.ts
line 216 at r4 (raw file):
color: `${geoViewColors.primary.main} !important`, '&:hover, &:active, &.active': { backgroundColor: `${geoViewColors.primary.light[400]} !important`,
Kaminder's comment about important in this file, does this apply here? I see a lot of !important
packages/geoview-core/src/ui/style/themeOptionsGenerator.ts
line 212 at r5 (raw file):
Previously, kaminderpal (Kamy) wrote…
R we still gonna use
style1
,style2
,style3
naming convention?
Agree, should have a better name to reflect state or type of use?
Previously, jolevesq (Johann Levesque) wrote…
Its used for the links in the details table. |
Previously, jolevesq (Johann Levesque) wrote…
Yes, will be renaming those later as I use them. Not sure what to name them yet. |
Previously, jolevesq (Johann Levesque) wrote…
Yes, it was applying to this file. Thats why I removed all the important statements |
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.
Reviewable status: 14 of 25 files reviewed, 6 unresolved discussions (waiting on @jolevesq and @kaminderpal)
packages/geoview-core/src/core/components/layers/left-panel/single-layer.tsx
line 252 at r5 (raw file):
Previously, kaminderpal (Kamy) wrote…
this can be looks
Also i know
selectedLayer
as name done before, but can we follow same pattern like dashbordered-primary
, so that we dnt have mismatch.
Done.
packages/geoview-core/src/core/components/layers/left-panel/single-layer.tsx
line 257 at r5 (raw file):
Previously, kaminderpal (Kamy) wrote…
same here above
Done.
packages/geoview-core/src/core/components/nav-bar/nav-bar.tsx
line 179 at r5 (raw file):
Previously, kaminderpal (Kamy) wrote…
Do we need className?
Done.
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.
Reviewed 2 of 6 files at r4, 2 of 12 files at r5, 5 of 6 files at r6, 2 of 2 files at r7, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @cphelefu)
a59709b
into
Canadian-Geospatial-Platform:develop
Description
This PR fixes the layer icons in the map tooltip and details tab.
Fixes #1827
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.
https://cphelefu.github.io/geoview/
Checklist:
I have made corresponding changes to the documentationI have added tests that prove my fix is effective or that my feature worksNew and existing unit tests pass locally with my changesThis change is