1119 scale dependency indicator

[MatthewMuehlhauserNRCan]

Description

Added a new state for checking layer visibility based on zoom levels set either at the service level or at the config/layer level. Legend and Layers list have styles to show when layer is not visible at current zoom level.

Fixes # 1119

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.

Layer Zoom Levels

Vector Tile Changes

Checklist:

• I have build (rush build) and deploy (rush host) my PR
• I have connected the issues(s) to this PR
• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• My changes generate no new warnings
• I have created new issue(s) related to the outcome of this PR is needed
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

---

This change is 

[jolevesq]

Reviewed 9 of 26 files at r1, 13 of 17 files at r2, all commit messages.
Reviewable status: 21 of 26 files reviewed, 20 unresolved discussions

---

packages/geoview-core/src/api/event-processors/event-processor-children/map-event-processor.ts line 438 at r2 (raw file):

    // Set ordered layer info for layers if they are inVisibleRange
    const { orderedLayerInfo } = this.getMapStateProtected(mapId);

Should we have this function in legendEventProcessor as it manipulates layers?

---

packages/geoview-core/src/api/event-processors/event-processor-children/map-event-processor.ts line 633 at r2 (raw file):

  }

  static getMapInVisibleRangeFromOrderedLayerInfo(mapId: string, layerPath: string): boolean {

These kind of function has been renamed with prefix find instead of get when they have layerPath as parameter. It will reuse with useSelector for the particular layerPath info to reduce re rendering

---

packages/geoview-core/src/geo/layer/geoview-layers/raster/xyz-tiles.ts line 146 at r2 (raw file):

      }

      // ESRI MapServer Implementation

What is this?

---

packages/geoview-core/src/geo/layer/geoview-layers/raster/xyz-tiles.ts line 244 at r2 (raw file):

      }

      // For ESRI MapServer XYZ Tiles

If esri tile layer does not work the same way as other xyz, do we need its own file? Not now as geoview-layer will be deprecated at some point

---

packages/geoview-core/public/configs/navigator/24-vector-tile.json line 21 at r2 (raw file):

          {
            "layerId": "CBMT_CBCT_3978_V_OSM",
            "styleUrl": "https://nrcan-rncan.maps.arcgis.com/sharing/rest/content/items/88ad9e2ef6e040a19472985e6606a2f9/resources/styles/root.json",

Sample is not working, url not found

---

packages/geoview-core/src/geo/layer/geoview-layers/esri-layer-common.ts line 369 at r2 (raw file):

  // Set zoom limits for max / min zooms
  // ! Note: minScale is actually the maxZoom and maxScale is actually the minZoom

For important information it is better ti GV instead of !. The ! comment is for mostly breaking stuff

---

packages/geoview-core/src/core/components/layers/left-panel/single-layer.tsx line 61 at r2 (raw file):

  const theme = useTheme();
  const classes = getSxClasses(theme);

use the useMemo hook to get the sxClasses, rename sxClasses

---

packages/geoview-core/src/core/components/layers/left-panel/single-layer.tsx line 444 at r2 (raw file):

    }
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, [displayState]);

Why have you removed a dependency

---

packages/geoview-core/src/core/components/layers/left-panel/single-layer.tsx line 455 at r2 (raw file):

            selected={layerIsSelected || (layerChildIsSelected && !legendExpanded)}
            tabIndex={-1}
            sx={{ minHeight: '4.51rem', ...(!inVisibleRange && classes.outOfRange) }}

Should we need the class name, the sxClasses does not contain proper style already

---

packages/geoview-core/src/geo/layer/layer-sets/feature-info-layer-set.ts line 141 at r2 (raw file):

        // If state is not in visible range
        if (!AbstractLayerSet.isInVisibleRange(this.getMapId(), layer)) return;

Does not seems to work, still in details

---

packages/geoview-core/src/core/stores/store-interface-and-intial-values/map-state.ts line 917 at r2 (raw file):

  const orderedLayerInfo = useStore(geoviewStore, (state) => state.mapState.orderedLayerInfo);
  // Redirect
  return MapEventProcessor.findMapLayerFromOrderedInfo(geoviewStore.getState().mapId, layerPath, orderedLayerInfo)?.inVisibleRange || false;

Look at comment above for state trigger

---

packages/geoview-core/src/core/utils/utilities.ts line 568 at r2 (raw file):

  const dpi = 25.4 / 0.28; // OpenLayers default DPI

  // Calculate resolution from scale

Does resolution affected by latitude for LCC like this function getMetersPerPixel?

---

packages/geoview-core/src/geo/layer/geoview-layers/raster/wms.ts line 640 at r2 (raw file):

        // TODO Since web map runs mostly in zoom levels, may not need Scale limits
        // Set Min/Max Scale Limits
        if (

All modification apply in geoview-layers file must be done in the corresponding api/config file... or at least a TODO because when we will migrate from these files we will loose function

---

packages/geoview-core/src/geo/layer/layer-sets/hover-feature-info-layer-set.ts line 103 at r2 (raw file):

  #getOrderedLayerPaths(): string[] {
    // Get the map layer order
    const mapLayerOrder = this.layerApi.mapViewer.getMapLayerOrderInfo().filter((layer) => layer.inVisibleRange);

Does not seems to work, still hoverable. There is already a check for visibility, ca we check this condition at the same time?

---

packages/geoview-core/src/core/utils/config/validation-classes/config-base-class.ts line 49 at r2 (raw file):

  /** The min scale that can be reach by the layer. */
  minScale?: number;

New config item needs to be added in the api/config section as well

---

packages/geoview-core/src/core/utils/config/validation-classes/raster-validation-classes/vector-tiles-layer-entry-config.ts line 10 at r2 (raw file):

  tileGrid!: TypeTileGrid;

  styleUrl?: string;

Same as previous

---

packages/geoview-core/src/core/components/legend/legend-layer.tsx line 77 at r2 (raw file):

  const { initLightBox, LightBoxComponent } = useLightBox();
  const { setLegendCollapsed } = useMapStoreActions();
  const isVisible = useSelectorLayerVisibility(layer.layerPath);

The new useSelector are not ready yet to be use. They do not trigger when needed an may the reason of your problem with legend control

I modify this one in layer-state to force re render when array items change

export const useSelectorLayerItems = (layerPath: string): TypeLegendItem[] => {
  // Hook
  const legendLayers = useStore(useGeoViewStore(), (state) => state.layerState.legendLayers);

  // Find the items
  const items = LegendEventProcessor.findLayerByPath(legendLayers, layerPath)?.items;

  // Compute a dependency string based on the items values
  const itemsKey = (items || []).map((item) => `${item.name}|${item.isVisible}|${item.icon}`).join('|||');

  // Return a new array reference when items change
  return useMemo(() => {
    // Log
    logger.logTraceUseMemo('LAYER-STATE - useSelectorLayerItems', itemsKey); // Using itemsKey in log to satisfy linter

    // Create a new array with spread operator to force new reference
    return items ? [...items] : [];
  }, [items, itemsKey]);
};

---

packages/geoview-core/src/core/components/legend/legend-layer.tsx line 113 at r2 (raw file):

          tooltip={t('layers.toggleCollapse') as string}
          onExpandClick={handleExpandGroupClick}
        />

We still wan the collapse item visible. Only change the text style for now

---

packages/geoview-core/src/geo/layer/layer.ts line 1004 at r2 (raw file):

  #createGVGroupLayer(mapId: string, olLayerGroup: LayerGroup, layerConfig: GroupLayerEntryConfig): GVGroupLayer | undefined {
    // Set extreme zoom settings to group layer so sub layers can load
    olLayerGroup.setMaxZoom(50);

This should be set in geoview-layers and config files so it pre-set by the config

---

packages/geoview-core/src/geo/layer/gv-layers/abstract-gv-layer.ts line 235 at r2 (raw file):

    }

    if (layerConfig.initialSettings.minZoom && layerConfig.initialSettings.minZoom !== 0) {

Why code is moved from were there is the other initial setting?

[MatthewMuehlhauserNRCan]

Reviewable status: 15 of 29 files reviewed, 20 unresolved discussions (waiting on @jolevesq)

---

packages/geoview-core/public/configs/navigator/24-vector-tile.json line 21 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Sample is not working, url not found

Ah, I'm going to look into this more. It seems like it's not the style url, but the metadata url is getting extra appended to the end?

---

packages/geoview-core/src/api/event-processors/event-processor-children/map-event-processor.ts line 438 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Should we have this function in legendEventProcessor as it manipulates layers?

It updates the orderedLayerInfo, which is a mapState. Unlike the visibility, there is no inVisibleRange property on the layer itself that needs to be updated. It does use the legendState to check what the max/minZoom of each layer is, but it doesn't modify it, so I thought it belongs here? But I could be wrong.

---

packages/geoview-core/src/api/event-processors/event-processor-children/map-event-processor.ts line 633 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

These kind of function has been renamed with prefix find instead of get when they have layerPath as parameter. It will reuse with useSelector for the particular layerPath info to reduce re rendering

I was mostly just copying the structure of the "visible" property which has a function with a similar name right above it. In the useSelector hook, it doesn't use either of these functions though and I did copy that structure as well for the inVisibleRange. I think the point of this function though is to be used in the layer.ts, although it's not currently being used anywhere.

---

packages/geoview-core/src/core/components/layers/left-panel/single-layer.tsx line 61 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

use the useMemo hook to get the sxClasses, rename sxClasses

Done

---

packages/geoview-core/src/core/components/layers/left-panel/single-layer.tsx line 444 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Why have you removed a dependency

Good question. I think that happened during the rebase. I've added it back in.

---

packages/geoview-core/src/core/components/layers/left-panel/single-layer.tsx line 455 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Should we need the class name, the sxClasses does not contain proper style already

I'm not very familiar with MUI / sxClasses, so I could use some help with the styling / structuring of the classes for sure.

---

packages/geoview-core/src/core/components/legend/legend-layer.tsx line 77 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

The new useSelector are not ready yet to be use. They do not trigger when needed an may the reason of your problem with legend control

I modify this one in layer-state to force re render when array items change

export const useSelectorLayerItems = (layerPath: string): TypeLegendItem[] => {
  // Hook
  const legendLayers = useStore(useGeoViewStore(), (state) => state.layerState.legendLayers);

  // Find the items
  const items = LegendEventProcessor.findLayerByPath(legendLayers, layerPath)?.items;

  // Compute a dependency string based on the items values
  const itemsKey = (items || []).map((item) => `${item.name}|${item.isVisible}|${item.icon}`).join('|||');

  // Return a new array reference when items change
  return useMemo(() => {
    // Log
    logger.logTraceUseMemo('LAYER-STATE - useSelectorLayerItems', itemsKey); // Using itemsKey in log to satisfy linter

    // Create a new array with spread operator to force new reference
    return items ? [...items] : [];
  }, [items, itemsKey]);
};

This was after the re-base, but it seems to be working well now, it wasn't working before the rebase though when I was using a different hook. The initial state for the layers was being caused though because I was using the config to set the inVisibleRange state, but the maxZoom / minZoom values hadn't been updated by the validator yet, so if it was being set by either the scale or the service, then it could end up setting the wrong initial state. I've dealt with that now though.

---

packages/geoview-core/src/core/components/legend/legend-layer.tsx line 113 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

We still wan the collapse item visible. Only change the text style for now

Done

---

packages/geoview-core/src/core/stores/store-interface-and-intial-values/map-state.ts line 917 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Look at comment above for state trigger

Appears to be working

---

packages/geoview-core/src/core/utils/utilities.ts line 568 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Does resolution affected by latitude for LCC like this function getMetersPerPixel?

It seems to work fine this way at the moment since it's just being used to convert the scale from the service or the config into a zoom level. Unless we wanted to be able to take the extent from the service to then get the center latitude and calculate the zoom from there?

---

packages/geoview-core/src/core/utils/config/validation-classes/config-base-class.ts line 49 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

New config item needs to be added in the api/config section as well

These were already in the api/config, but were missing from this config.

---

packages/geoview-core/src/core/utils/config/validation-classes/raster-validation-classes/vector-tiles-layer-entry-config.ts line 10 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Same as previous

This is specific to Vector Tiles, but it doesn't seem the vector tiles have been implemented yet in the api/config?

---

packages/geoview-core/src/geo/layer/layer.ts line 1004 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

This should be set in geoview-layers and config files so it pre-set by the config

I've moved it into the gv-group-layer. The reason I didn't set it in the config is because I still wanted the geoview-layer to have the proper max and minZoom so they displayed correctly in the UI, but then underneath, in the map, the group layer would always be visible, otherwise the sub layers would be stuck in a loading state until you zoomed into the visible range of the group layer.

---

packages/geoview-core/src/geo/layer/geoview-layers/esri-layer-common.ts line 369 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

For important information it is better ti GV instead of !. The ! comment is for mostly breaking stuff

DONE

---

packages/geoview-core/src/geo/layer/geoview-layers/raster/wms.ts line 640 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

All modification apply in geoview-layers file must be done in the corresponding api/config file... or at least a TODO because when we will migrate from these files we will loose function

Added a TODO

---

packages/geoview-core/src/geo/layer/geoview-layers/raster/xyz-tiles.ts line 146 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

What is this?

Good question. I don't think it was fully implemented? The current solution / example navigator page didn't work properly and results in a invalid geojson error. The layer would load, but the config wasn't processed properly. This made it work properly, but is definitely not the best solution

---

packages/geoview-core/src/geo/layer/geoview-layers/raster/xyz-tiles.ts line 244 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

If esri tile layer does not work the same way as other xyz, do we need its own file? Not now as geoview-layer will be deprecated at some point

That could be a possibility.

---

packages/geoview-core/src/geo/layer/gv-layers/abstract-gv-layer.ts line 235 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Why code is moved from were there is the other initial setting?

This is setting the minZoom and maxZoom on the OL layer itself, similarly to the visibility just above this. I guess this could be set elsewhere, but it just seemed like the most straightforward place.

---

packages/geoview-core/src/geo/layer/layer-sets/feature-info-layer-set.ts line 141 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Does not seems to work, still in details

What is the desired result? This only prevents you from clicking and having a hidden feature from showing up in the details (if you've already clicked on a feature before zooming out of it's range, it's still there), but all the layers still show on the details tab.

Did we want to hide the layers that are not in the visible range? Remove their results from the tab?

---

packages/geoview-core/src/geo/layer/layer-sets/hover-feature-info-layer-set.ts line 103 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Does not seems to work, still hoverable. There is already a check for visibility, ca we check this condition at the same time?

This seems to be working for me, but maybe it's hard to tell with the current config. I'll change the config so it's easier to test. THe only thing that maybe could be improved is that the pop-up remains when you zoom out, but as soon as you move your mouse it disappears?

[Alex-NRCan]

Reviewed 7 of 26 files at r1, 9 of 17 files at r2, 15 of 15 files at r3, all commit messages.
Reviewable status: all files reviewed, 23 unresolved discussions (waiting on @jolevesq)

---

packages/geoview-core/src/api/event-processors/event-processor-children/map-event-processor.ts line 633 at r2 (raw file):

Previously, MatthewMuehlhauserNRCan wrote…

I was mostly just copying the structure of the "visible" property which has a function with a similar name right above it. In the useSelector hook, it doesn't use either of these functions though and I did copy that structure as well for the inVisibleRange. I think the point of this function though is to be used in the layer.ts, although it's not currently being used anywhere.

Minor.
These 3 get functions, could be calling findMapLayerFromOrderedInfo instead of doing const info = and const pathInfo = , possibly saving some lines and favor reusing functions. I think the 3 gets would be re-implemented in a 1 liner return. Something like: return findMapLayerFromOrderedInfo(mapId, layerPath)?.legendCollapsed !== false;
Maybe there are other functions comparing layerPath instead of pointing to findMapLayerFromOrderedInfo.

---

packages/geoview-core/src/geo/layer/gv-layers/gv-group-layer.ts line 24 at r3 (raw file):

    // Set extreme zoom settings to group layer so sub layers can load
    this.olLayer.setMaxZoom(50);

Suggestion:
Create 2 static constants inside gv-group-layer class to hold these 2 numbers, so that they are easy to pick up. Instead of having to search the code for hardcoded values.

In class header somewhere:
MAX_ZOOM = 50;
MIN_ZOOM = 0;

In here:
this.olLayer.setMinZoom(GVGroupLayer.MIN_ZOOM);
this.olLayer.setMaxZoom(GVGroupLayer.MAX_ZOOM);

We'll more easily be able to configure/adjust those values if needed too.

---

packages/geoview-core/src/geo/layer/layer-sets/abstract-layer-set.ts line 446 at r3 (raw file):

  protected static isInVisibleRange(mapId: string, layer: AbstractBaseLayer): boolean {
    // Return false when false or undefined
    return MapEventProcessor.findMapLayerFromOrderedInfo(mapId, layer.getLayerPath())?.inVisibleRange ?? false;

Concern:
Not sure how I feel about a LayerSet, a framework-level class, performing EventProcessor operations to check a state.
Could inVisibleRange be a property on a Layer object itself? For example, isQueryableType, isStateQueryable, isSourceQueryable above, they rely on the layer and its configuration to determine the result. I'm just afraid of having 2 different patterns to check for states, especially in 'framework-level classes' if you will. Mainly, I think the framework classes should work with framework properties and application/ui level should read store states.

---

packages/geoview-core/src/core/utils/utilities.ts line 9 at r3 (raw file):

import i18n from '@/core/translation/i18n';
import { TypeGuideObject } from '@/core/stores/store-interface-and-intial-values/app-state';
import { MapEventProcessor } from '@/api/event-processors/event-processor-children/map-event-processor';

Concern:
Here, similar concern as my other comment. I don't think an utilities class, very low-level, should have a dependency on a store/state map-event-processor. I'd suggest removing this import and refactor the code / split it or move it.

[MatthewMuehlhauserNRCan]

Reviewable status: all files reviewed, 22 unresolved discussions (waiting on @Alex-NRCan and @jolevesq)

---

packages/geoview-core/src/api/event-processors/event-processor-children/map-event-processor.ts line 633 at r2 (raw file):

Previously, Alex-NRCan (Alex) wrote…

Minor.
These 3 get functions, could be calling findMapLayerFromOrderedInfo instead of doing const info = and const pathInfo = , possibly saving some lines and favor reusing functions. I think the 3 gets would be re-implemented in a 1 liner return. Something like: return findMapLayerFromOrderedInfo(mapId, layerPath)?.legendCollapsed !== false;
Maybe there are other functions comparing layerPath instead of pointing to findMapLayerFromOrderedInfo.

That seems like a great one liner. I also went ahead and updated the other two functions as you said.

---

packages/geoview-core/src/core/utils/utilities.ts line 9 at r3 (raw file):

Previously, Alex-NRCan (Alex) wrote…

Concern:
Here, similar concern as my other comment. I don't think an utilities class, very low-level, should have a dependency on a store/state map-event-processor. I'd suggest removing this import and refactor the code / split it or move it.

I've removed the need for that dependency, although, it now depends on a viewer instead being passed into the scale/zoom functions. Will that work?

---

packages/geoview-core/src/geo/layer/gv-layers/gv-group-layer.ts line 24 at r3 (raw file):

Previously, Alex-NRCan (Alex) wrote…

Suggestion:
Create 2 static constants inside gv-group-layer class to hold these 2 numbers, so that they are easy to pick up. Instead of having to search the code for hardcoded values.

In class header somewhere:
MAX_ZOOM = 50;
MIN_ZOOM = 0;

In here:
this.olLayer.setMinZoom(GVGroupLayer.MIN_ZOOM);
this.olLayer.setMaxZoom(GVGroupLayer.MAX_ZOOM);

We'll more easily be able to configure/adjust those values if needed too.

DONE

---

packages/geoview-core/src/geo/layer/layer-sets/abstract-layer-set.ts line 446 at r3 (raw file):

Previously, Alex-NRCan (Alex) wrote…

Concern:
Not sure how I feel about a LayerSet, a framework-level class, performing EventProcessor operations to check a state.
Could inVisibleRange be a property on a Layer object itself? For example, isQueryableType, isStateQueryable, isSourceQueryable above, they rely on the layer and its configuration to determine the result. I'm just afraid of having 2 different patterns to check for states, especially in 'framework-level classes' if you will. Mainly, I think the framework classes should work with framework properties and application/ui level should read store states.

Ok, I've created a quick way to check on the layer itself if it's in it's visible range as well and used that instead.

[MatthewMuehlhauserNRCan]

Reviewable status: 19 of 32 files reviewed, 22 unresolved discussions (waiting on @Alex-NRCan and @jolevesq)

---

packages/geoview-core/src/api/event-processors/event-processor-children/map-event-processor.ts line 438 at r2 (raw file):

Previously, MatthewMuehlhauserNRCan wrote…

It updates the orderedLayerInfo, which is a mapState. Unlike the visibility, there is no inVisibleRange property on the layer itself that needs to be updated. It does use the legendState to check what the max/minZoom of each layer is, but it doesn't modify it, so I thought it belongs here? But I could be wrong.

I've updated this to be handled differently. In the map-viewer, the entry configs are used to check for the inVisibleRange and then the updated orderedLayerConfigs are passed here to be updated.

[jolevesq]

Reviewed 1 of 17 files at r2, 7 of 15 files at r3, 14 of 15 files at r4, all commit messages.
Reviewable status: 31 of 32 files reviewed, 7 unresolved discussions (waiting on @Alex-NRCan and @MatthewMuehlhauserNRCan)

---

packages/geoview-core/src/core/utils/config/validation-classes/raster-validation-classes/vector-tiles-layer-entry-config.ts line 10 at r2 (raw file):

Previously, MatthewMuehlhauserNRCan wrote…

This is specific to Vector Tiles, but it doesn't seem the vector tiles have been implemented yet in the api/config?

You are right, we miss some item in the new config API

---

packages/geoview-core/src/geo/layer/layer-sets/feature-info-layer-set.ts line 141 at r2 (raw file):

Previously, MatthewMuehlhauserNRCan wrote…

What is the desired result? This only prevents you from clicking and having a hidden feature from showing up in the details (if you've already clicked on a feature before zooming out of it's range, it's still there), but all the layers still show on the details tab.

Did we want to hide the layers that are not in the visible range? Remove their results from the tab?

The way it works now is we hide layer not visible from details. I would say if not in range visible, it should be removed as it shuld never trigger a result for this layer.

---

packages/geoview-core/src/geo/layer/layer-sets/hover-feature-info-layer-set.ts line 103 at r2 (raw file):

Previously, MatthewMuehlhauserNRCan wrote…

This seems to be working for me, but maybe it's hard to tell with the current config. I'll change the config so it's easier to test. THe only thing that maybe could be improved is that the pop-up remains when you zoom out, but as soon as you move your mouse it disappears?

Works now

---

packages/geoview-core/src/core/stores/store-interface-and-intial-values/map-state.ts line 356 at r4 (raw file):

       * @param {boolean} [newValue] - The new value of visibility.
       */
      setLayerInVisibleRange: (layerPath: string, newValue: boolean): void => {

Should this be in the layerAPI (layer.ts) file?

---

packages/geoview-core/src/geo/layer/gv-layers/vector/gv-vector-tiles.ts line 38 at r2 (raw file):

    this.olLayer = new VectorTileLayer({ ...tileLayerOptions, declutter });

    if (layerConfig.styleUrl) {

This is not working? I think we still have this param available from config?

---

packages/geoview-core/src/geo/layer/gv-layers/gv-group-layer.ts line 14 at r4 (raw file):

export class GVGroupLayer extends AbstractBaseLayer {
  /** Max zoom constant */
  static readonly MAX_ZOOM = 50;

I have changed it to be 20 by default. We should have at s0me point a constant to hold this value... we will need a check of all our constant... Just set it 20 for now

[DamonU2]

Reviewed 6 of 26 files at r1, 4 of 17 files at r2, 7 of 15 files at r3, 15 of 15 files at r4, all commit messages.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @Alex-NRCan, @jolevesq, and @MatthewMuehlhauserNRCan)

---

packages/geoview-core/src/geo/layer/layer-sets/feature-info-layer-set.ts line 141 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

The way it works now is we hide layer not visible from details. I would say if not in range visible, it should be removed as it shuld never trigger a result for this layer.

Noticed this with the data table too. Do we want to handle it the same way - don't show in data table if it is not visible?

---

packages/geoview-core/public/configs/navigator/05-layer-zoom-levels.json line 32 at r4 (raw file):

          {
            "layerId": "pub:WHSE_IMAGERY_AND_BASE_MAPS.MOT_CULVERTS_SP",
            "layerName": "Limited by Service - BC Minitstry of Transportation Culverts"

Minitstry > Ministry

---

packages/geoview-core/src/core/stores/store-interface-and-intial-values/map-state.ts line 352 at r4 (raw file):

      /**
       * Sets or toggles the visibility of a layer.

Description needs updating, it is from the above function.

[Alex-NRCan]

Reviewed 1 of 17 files at r2, 7 of 15 files at r4, all commit messages.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @jolevesq and @MatthewMuehlhauserNRCan)

---

packages/geoview-core/src/core/utils/utilities.ts line 9 at r3 (raw file):

Previously, MatthewMuehlhauserNRCan wrote…

I've removed the need for that dependency, although, it now depends on a viewer instead being passed into the scale/zoom functions. Will that work?

Hmm it's an improvement yes, but I'm unsure.
At this point, I'll leave it to Johann to determine if 'utils' was meant for very low-level code or having a depenency on OpenLayer is valid here.
2 ideas maybe?:
1- Maybe getMapScale, getZoomFromScale and getScaleFromZoom functions should simply be on the MapViewer class instead and internally use something like this.getView()?
2- Maybe we could have a 'map-utils' file that explicitely offers 'OpenLayers-related' utility functions (acting as a step-between the application level and utils level) or something in that vein?

---

packages/geoview-core/src/geo/layer/layer-sets/abstract-layer-set.ts line 446 at r3 (raw file):

Previously, MatthewMuehlhauserNRCan wrote…

Ok, I've created a quick way to check on the layer itself if it's in it's visible range as well and used that instead.

Ok, I see, and inside it's using this.getMapViewer() which is already a kind of a temporary 'hack', but at least now we're standardizing the code flow which is good. That works better for me.

[Alex-NRCan]

Reviewed 1 of 15 files at r4.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @jolevesq and @MatthewMuehlhauserNRCan)

[Alex-NRCan]

Reviewed 1 of 15 files at r4.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @MatthewMuehlhauserNRCan)

[Alex-NRCan]

Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @jolevesq and @MatthewMuehlhauserNRCan)

---

packages/geoview-core/src/geo/layer/gv-layers/abstract-gv-layer.ts line 235 at r2 (raw file):

Previously, MatthewMuehlhauserNRCan wrote…

This is setting the minZoom and maxZoom on the OL layer itself, similarly to the visibility just above this. I guess this could be set elsewhere, but it just seemed like the most straightforward place.

Question:
Shouldn't the code still remain in the initOptionsWithInitialSettings as well though? It seems initOptionsWithInitialSettings sets layerOptions which is addressing another matter than setting the parameter on the layer itself (which is fine to do here like you did I believe). So, keep your modifications as you've done here and put back the code in initOptionsWithInitialSettings or is that going to cause a contradiction that I'm not picking up?