-
Notifications
You must be signed in to change notification settings - Fork 36
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
2608 geo core initial settings #2651
2608 geo core initial settings #2651
Conversation
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 4 files at r1, all commit messages.
Reviewable status: 2 of 4 files reviewed, 1 unresolved discussion (waiting on @Alex-NRCan, @DamonU2, and @MatthewMuehlhauserNRCan)
packages/geoview-core/src/geo/layer/other/geocore.ts
line 60 at r1 (raw file):
for (let i = 0; i < response.layers.length; i++) { response.layers[i].initialSettings = { ...response.layers[i].initialSettings, ...layerConfig.initialSettings }; // Need to set the initial settings of the LayerConfigs. Only applies to top layers.
We should be able to apply to any layers?
2f9cea0
to
751fa7e
Compare
751fa7e
to
2fe333c
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.
Reviewed 4 of 5 files at r2, 5 of 5 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Alex-NRCan, @DamonU2, and @jolevesq)
packages/geoview-core/src/geo/layer/other/geocore.ts
line 60 at r1 (raw file):
Previously, jolevesq (Johann Levesque) wrote…
We should be able to apply to any layers?
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 3 of 5 files at r2, 5 of 5 files at r3, all commit messages.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @Alex-NRCan, @DamonU2, and @MatthewMuehlhauserNRCan)
packages/geoview-core/src/geo/layer/other/geocore.ts
line 4 at r3 (raw file):
import { UUIDmapConfigReader } from '@/core/utils/config/reader/uuid-config-reader'; // import { UUIDmapConfigReader } from '@/api/config/uuid-config-reader';
Do you need dead import
packages/geoview-core/public/configs/navigator/28-geocore-custom-inline-config.json
line 46 at r3 (raw file):
"hoverable": true, "legendCollapsed": false, "opacity": 0.7,
Can you put .3, .7 is hard to notice
packages/geoview-core/public/configs/navigator/28-geocore-custom-inline-config.json
line 57 at r3 (raw file):
"geoviewLayerType": "geoCore", "geoviewLayerId": "03ccfb5c-a06e-43e3-80fd-09d4f8f69703", "initialSettings": {
Is this section suppose to do something?
packages/geoview-core/public/configs/navigator/28-geocore-custom-inline-config.json
line 71 at r3 (raw file):
}, "controls": { "highlight": false,
The control is disable on layers but not on legend. If it is a quick fix you can do. If not, please create an issue
packages/geoview-core/src/core/components/layers/left-panel/add-new-layer/add-new-layer.tsx
line 887 at r3 (raw file):
(layerEntries as TypeGeoviewLayerConfig[]).forEach((geoviewLayerConfig) => { if (layerName !== geoviewLayerConfig.geoviewLayerName) { const tempConfig = geoviewLayerConfig;
I tested this uuid cf21aa0c-b76c-4f1f-96c1-4ef4b085b647 and set a name. Instead of giving the name to the group it gave it to the first layer
packages/geoview-core/src/core/utils/config/config.ts
line 58 at r3 (raw file):
if (mapConfigLayerEntryIsGeoCore(geoviewLayerEntry)) { // Skip it, because we don't validate the GeoCore configuration anymore. Not the same way as typical GeoView Layer Types at least. // ? Why not do GeoCore request here? Then could easily replace listOfLayerEntries and validate / process along with other layers?
Put TODO: refactor - so we will track it in the refactor
packages/geoview-core/src/geo/map/map-schema-types.ts
line 412 at r3 (raw file):
/** The layer entries to use from the GeoCore layer. */ listOfLayerEntryConfig?: TypeLayerEntryConfig[];
This file is depecrated, we should use ma-schema type in api config
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 3 of 5 files at r2, 5 of 5 files at r3, all commit messages.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @Alex-NRCan, @jolevesq, and @MatthewMuehlhauserNRCan)
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: all files reviewed, 8 unresolved discussions (waiting on @Alex-NRCan and @jolevesq)
packages/geoview-core/src/core/components/layers/left-panel/add-new-layer/add-new-layer.tsx
line 887 at r3 (raw file):
Previously, jolevesq (Johann Levesque) wrote…
I tested this uuid cf21aa0c-b76c-4f1f-96c1-4ef4b085b647 and set a name. Instead of giving the name to the group it gave it to the first layer
Looking at this, I think it's because of how the layers are handled. There is one layer with two indexes in the layerEntries returned from GeoCore, but there is no Group layer, so the name is applied to the first layer entry and not the created group entry. I'll look a bit closer to see if there's any way I can make the name apply to the group that is created later.
packages/geoview-core/src/core/utils/config/config.ts
line 58 at r3 (raw file):
Previously, jolevesq (Johann Levesque) wrote…
Put TODO: refactor - so we will track it in the refactor
Done.
packages/geoview-core/src/geo/layer/other/geocore.ts
line 4 at r3 (raw file):
Previously, jolevesq (Johann Levesque) wrote…
Do you need dead import
Done.
packages/geoview-core/src/geo/map/map-schema-types.ts
line 412 at r3 (raw file):
Previously, jolevesq (Johann Levesque) wrote…
This file is depecrated, we should use ma-schema type in api config
I don't think type GeoCoreLayerConfig exists in the new map-schema-types. I'm guessing it won't be necessary after the refactor? Including this use case.
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 5 files at r2, 4 of 5 files at r3, 2 of 2 files at r4, all commit messages.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @jolevesq and @MatthewMuehlhauserNRCan)
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: 6 of 8 files reviewed, 8 unresolved discussions (waiting on @Alex-NRCan, @DamonU2, and @jolevesq)
packages/geoview-core/public/configs/navigator/28-geocore-custom-inline-config.json
line 46 at r3 (raw file):
Previously, jolevesq (Johann Levesque) wrote…
Can you put .3, .7 is hard to notice
Done.
packages/geoview-core/public/configs/navigator/28-geocore-custom-inline-config.json
line 57 at r3 (raw file):
Previously, jolevesq (Johann Levesque) wrote…
Is this section suppose to do something?
I was trying to see if I could get it to highlight from the config, but it didn't seem to work.
packages/geoview-core/src/core/components/layers/left-panel/add-new-layer/add-new-layer.tsx
line 887 at r3 (raw file):
Previously, MatthewMuehlhauserNRCan wrote…
Looking at this, I think it's because of how the layers are handled. There is one layer with two indexes in the layerEntries returned from GeoCore, but there is no Group layer, so the name is applied to the first layer entry and not the created group entry. I'll look a bit closer to see if there's any way I can make the name apply to the group that is created later.
This should be handled now
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 2 files at r4, all commit messages.
Reviewable status: 6 of 8 files reviewed, 5 unresolved discussions (waiting on @Alex-NRCan, @DamonU2, and @MatthewMuehlhauserNRCan)
packages/geoview-core/public/configs/navigator/28-geocore-custom-inline-config.json
line 57 at r3 (raw file):
Previously, MatthewMuehlhauserNRCan wrote…
I was trying to see if I could get it to highlight from the config, but it didn't seem to work.
Can you remove from config if itis not working
packages/geoview-core/src/geo/map/map-schema-types.ts
line 409 at r5 (raw file):
/** Initial settings to apply to the GeoCore layer at creation time. */ initialSettings?: TypeLayerInitialSettings;
You said this is not working at the root level. Can we remove from the type file
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 1 of 2 files at r5.
Reviewable status: 7 of 8 files reviewed, 5 unresolved discussions (waiting on @Alex-NRCan, @DamonU2, and @jolevesq)
packages/geoview-core/public/configs/navigator/28-geocore-custom-inline-config.json
line 57 at r3 (raw file):
Previously, jolevesq (Johann Levesque) wrote…
Can you remove from config if itis not working
Done.
packages/geoview-core/public/configs/navigator/28-geocore-custom-inline-config.json
line 71 at r3 (raw file):
Previously, jolevesq (Johann Levesque) wrote…
The control is disable on layers but not on legend. If it is a quick fix you can do. If not, please create an issue
This happens with non-geocore layers too, so I'll create an issue.
Previously, jolevesq (Johann Levesque) wrote…
I just tested, it does work at the root level too, but I was testing the controls to see if maybe I needed to set it somewhere else to disable the highlight function, but it didn't seem to matter. I just tested the controls now too, and they seem to work there as well. So I guess it's just the highlight not behaving correctly in the legend. |
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 1 of 2 files at r5, 1 of 1 files at r6, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @MatthewMuehlhauserNRCan)
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: all files reviewed, 2 unresolved discussions (waiting on @MatthewMuehlhauserNRCan)
c382048
into
Canadian-Geospatial-Platform:develop
Description
Added the ability to apply initial settings to GeoCore layers. Only applies to the top most layers currently.
Fixes # 2608
Type of change
How Has This Been Tested?
GeoCore Custom Inline Config - Navigator Page
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