feat(Text Layer): Separate text from geometry layer and add UI to toggle it's visibility by MatthewMuehlhauserNRCan · Pull Request #3352 · Canadian-Geospatial-Platform/geoview

MatthewMuehlhauserNRCan · 2026-03-16T19:32:13Z

Description

Separated the text layer from the geometry layer when labels are configured so that the declutter mode will work properly. Also allowed to create a button to toggle the labels in the UI.

Fixes #3227

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?

Host Updated 2026-03-16 @ 3:55PM

Tried toggling the text in the UI and different combinations of the layer being turned on/off and text on/off. Also, the different layers have different declutterModes, so the results of decluttering can be seen.

Layer Text Demo Navigator Page

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

@jolevesq reviewed 11 files and all commit messages, and made 4 comments.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on MatthewMuehlhauserNRCan).

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

  /**
   * Gets the OpenLayers text layer if one exists.
   * @returns {VectorLayer<VectorSource> | undefined} The text layer or undefined if no text layer exists.

Blank line and no need to set param or return type (for other functions below as well)

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

  /**
   * Emits an event to all handlers.
   * @param {TextVisibleChangedEvent} event - The event to emit

No need for private

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

  /**
   * Calculates a text-only style (no geometry) for the given feature.

Blank, param type and static

packages/geoview-core/src/geo/utils/renderer/geoview-text-renderer.ts line 3 at r1 (raw file):

import { Stroke, Fill, Text } from 'ol/style';
import type { FeatureLike } from 'ol/Feature';
import type Feature from 'ol/Feature';

Nice split to remove complexity inside geoview-renderer!

MatthewMuehlhauserNRCan

@MatthewMuehlhauserNRCan made 3 comments and resolved 1 discussion.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on jolevesq).

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

Previously, jolevesq (Johann Levesque) wrote…

Blank line and no need to set param or return type (for other functions below as well)

Done.

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

Previously, jolevesq (Johann Levesque) wrote…

No need for private

Done.

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

Previously, jolevesq (Johann Levesque) wrote…

Blank, param type and static

Done.

MatthewMuehlhauserNRCan

@MatthewMuehlhauserNRCan reviewed 7 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on jolevesq).

packages/geoview-core/public/locales/fr/translation.json line 119 at r2 (raw file):

    "subLayersCount": "{count} sous-couches",
    "itemsCount": "{count} sur {totalCount} classes",
    "legendInstructions": "Instructions de la légende",

Seemed like it wasn't translated. Did a quick search for context to see where it was used and didn't see any hits ...

Alex-NRCan

@Alex-NRCan reviewed 13 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on jolevesq and MatthewMuehlhauserNRCan).

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

        // Make sure text layer z-index above other vector layers
        const gvLayer = this.getMapViewerLayerAPI(mapId).getGeoviewLayerIfExists(orderedLayerInfo.layerPath);

Instead of re-getting the gvLayer again with the orderedLayerInfo.layerPath, you could get it originally instead of getting the olLayer above. You wouldn't have to call get..LayerIfExists() twice.

Suggestion: Also, more importantly, similar to how you've done it with setVisible, we should probably have a setZIndex on our AbstractGVLayer base class which would call an overridable method onSetZIndex() which would do this.getOLLayer().setZIndex() in the base mother class. And then, in AbstractGVVector we would override onSetZIndex() to call the parent and then do the this.getTextOLLayer()?.setZIndex();

That way the logic of the text layer following the zIndex of the geometry layer is handled internally inside AbstractGVVector instead of through the map-event-processor only.

(Note, setVisible should call a onSetVisible and it's that function that should be overridable to make it even more clean, but it's also ok to override setVisible directly for now)

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

    // Sync text layer visibility if it exists
    // Text layer visibility = layerVisibility && textVisible
    if (this.#textOLLayer) {

Very minor: what you have works, and this works too on a 1-liner: this.#textOLLayer?.setVisible(...); here and couple places

Alex-NRCan · 2026-03-17T14:57:56Z

Nicely done overall! ! 👍

MatthewMuehlhauserNRCan

@MatthewMuehlhauserNRCan made 2 comments.
Reviewable status: 11 of 14 files reviewed, 6 unresolved discussions (waiting on Alex-NRCan, DamonU2, and jolevesq).

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

Previously, Alex-NRCan (Alex) wrote…

Instead of re-getting the gvLayer again with the orderedLayerInfo.layerPath, you could get it originally instead of getting the olLayer above. You wouldn't have to call get..LayerIfExists() twice.

Suggestion: Also, more importantly, similar to how you've done it with setVisible, we should probably have a setZIndex on our AbstractGVLayer base class which would call an overridable method onSetZIndex() which would do this.getOLLayer().setZIndex() in the base mother class. And then, in AbstractGVVector we would override onSetZIndex() to call the parent and then do the this.getTextOLLayer()?.setZIndex();

That way the logic of the text layer following the zIndex of the geometry layer is handled internally inside AbstractGVVector instead of through the map-event-processor only.

(Note, setVisible should call a onSetVisible and it's that function that should be overridable to make it even more clean, but it's also ok to override setVisible directly for now)

Done I think I got everything.

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

Previously, Alex-NRCan (Alex) wrote…

Very minor: what you have works, and this works too on a 1-liner: this.#textOLLayer?.setVisible(...); here and couple places

Nice, I've added those changes.

DamonU2

@DamonU2 reviewed 14 files and all commit messages, and made 8 comments.
Reviewable status: all files reviewed, 14 unresolved discussions (waiting on Alex-NRCan, jolevesq, and MatthewMuehlhauserNRCan).

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

   * Sets the visibility of the layer (true or false).
   *
   * @param layerVisibility The visibility of the layer.

Needs a dash between layerVisibility and the description

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

   *
   * @param layerVisibility The visibility of the layer.
   * @param emitVisibleChanged Optional, whether to emit a visible changed event after updating the visibility. Defaults to true.

same here

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

   *
   * @param zIndex The z-index of the layer.
   * @param emitZIndexChanged Optional, whether to emit a z-index changed event after updating the z-index. Defaults to true.

Both of these too

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

  /**
   * Emits a z-index changed event.

Blank line after

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

  /**
   * Emits a z-index changed event.
   * @param {LayerZIndexChangedEvent} event - The event to emit

No type needed anymore

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

  /**
   * Registers a z-index changed event handler.
   * @param {LayerZIndexChangedDelegate} callback - The callback to be executed whenever the event is emitted

Same here

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

  /**
   * Unregisters a z-index changed event handler.
   * @param {LayerZIndexChangedDelegate} callback - The callback to stop being called whenever the event is emitted

And here

packages/geoview-core/src/geo/utils/renderer/geoview-text-renderer.ts line 20 at r3 (raw file):

   * This method returns true if a style config has a text configuration
   * @param {TypeLayerStyleConfig} layerStyle - The layer style
   * @returns {boolean} If the style has a text config

All functions in this file are still the old style of JSDocs

MatthewMuehlhauserNRCan

@MatthewMuehlhauserNRCan made 8 comments.
Reviewable status: 12 of 14 files reviewed, 14 unresolved discussions (waiting on Alex-NRCan, DamonU2, and jolevesq).

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