fix(core): Fix many bug, add function for url validation, add a funct…

[jolevesq]

…ion to extract and apply color map from geotiff

Description

Fixes & Improvements

• Layer Management: Disabled "Continue" button if no service type is selected.
• Opacity Slider: * Rounded values to fix floating-point precision issues (e.g., 56.999...).
• Fixed bug where opacity reset to 1 on map zoom.
• Visual/CSS: * Fixed map background turning black in full-screen mode.
• Changed GeoJSON point style from star to square to reduce confusion.
• URL Validation: Added server ping utility to validate URLs during the "Add Layer" process.
• Slider Logic: Removed singleHandle restriction on step combo box to allow step-setting in dual-handle/non-discrete modes.
• Documentation: Improved abstract class exports for better TypeDoc/JSDoc visibility.
• COG Integration: * Added utility to extract color maps from Cloud Optimized GeoTIFFs.
• Implemented function to set pixel colors based on the extracted color map.

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes # (issue)

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.

Hosted Feb 27, 10am

https://jolevesq.github.io/geoview/layers-navigator.html

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 23 files and all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on Alex-NRCan).

[Alex-NRCan]

@Alex-NRCan partially reviewed 23 files and made 6 comments.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on jolevesq).

---

docs/programming/best-practices.md line 168 at r3 (raw file):

 * @param signal - Optional abort signal for request cancellation.
 * @returns Parsed layer metadata object.
 * @throws {LayerNotGeoJsonError} An error to 

'An error to' should rather be 'When ....' no?

---

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

    if (activeStep === 0) {
      // Validate URL for step 1
      const validateUrl = async (): Promise<void> => {

Minor: Maybe externalize this method as a useCallback outside of the useEffect, for clarity?

---

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

  };

  const helperFetchWithTimeout = async (url: string, options: RequestInit): Promise<Response | null> => {

Don't we already have a Fetch.fetchWithTimeout function? That fetchWithTimeout already throws errors like 'ResponseEmptyError'. I'd try using it instead of doing a whole new function and throwing new errors like throw new Error('No response') below.

---

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

      if (colorMap) {
        // eslint-disable-next-line no-param-reassign
        layerConfig.embeddedColorMap = colorMap;

Can we use a setter here and put embeddedColorMap private in the function?

---

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

   */
  static createGeoTIFFSource(layerConfig: GeoTIFFLayerEntryConfig): GeoTIFFSource {
    const hasColorMap = !!layerConfig.embeddedColorMap;

Here, use a getter? Idea, it could be a getter like 'hasEmbeddedColorMap' next to the getter 'getEmbeddedColorMap'?

---

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

   * @param palette - Array of RGBA color tuples from the GeoTIFF color map.
   */
  #applyColorMapStyle(palette: RGBA[]): void {

Move this private function down below in the class file. Not before the overrides.

[DamonU2]

@DamonU2 reviewed 23 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on jolevesq).

---

packages/geoview-core/src/core/components/layers/left-panel/add-new-layer/add-new-layer.tsx line 757 at r3 (raw file):

        // Validate and ping HTTPS URLs
        if (layerURL.startsWith('https://')) {

We need a visual indicator in the UI that something is happening while this runs, the continue button being disabled until it's not just seems broken

---

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

  }

  const size = colorMap.length / 3;

Could use a comment here to explain the structure of the colorMap and help understand the indexing below

[jolevesq]

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

---

docs/programming/best-practices.md line 168 at r3 (raw file):

Previously, Alex-NRCan (Alex) wrote…

'An error to' should rather be 'When ....' no?

Done.

---

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

Previously, Alex-NRCan (Alex) wrote…

Minor: Maybe externalize this method as a useCallback outside of the useEffect, for clarity?

Copilot told we should not

In this case, no — extracting validateUrl into a useCallback wouldn't improve performance and could reduce clarity:

1. No performance gain: validateUrl is only called inside the useEffect, not passed to a child component. There's no memoized child that would benefit from a stable reference. A useCallback wrapping it would still re-create whenever layerURL changes (its dependency), which is exactly when the effect re-runs anyway.

2. Added dependency overhead: You'd need to add the callback to the useEffect dependency array, and the callback itself would depend on layerURL — so you'd be tracking the same dependency in two places instead of one.

3. Co-location is clearer here: The function is a single-use async IIFE pattern (define + immediately call). Keeping it inside the effect makes its scope and lifecycle obvious — it only exists when activeStep === 0.

useCallback shines when a function is passed as a prop to memoized children (per the project's copilot instructions) or reused across multiple effects. For an effect-internal async helper like this, inline is the idiomatic React pattern.

---

packages/geoview-core/src/core/components/layers/left-panel/add-new-layer/add-new-layer.tsx line 757 at r3 (raw file):

Previously, DamonU2 (Damon Ulmi) wrote…

We need a visual indicator in the UI that something is happening while this runs, the continue button being disabled until it's not just seems broken

Done.

---

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

Previously, Alex-NRCan (Alex) wrote…

Don't we already have a Fetch.fetchWithTimeout function? That fetchWithTimeout already throws errors like 'ResponseEmptyError'. I'd try using it instead of doing a whole new function and throwing new errors like throw new Error('No response') below.

Done.

---

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

Previously, DamonU2 (Damon Ulmi) wrote…

Could use a comment here to explain the structure of the colorMap and help understand the indexing below

Done.

---

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

Previously, Alex-NRCan (Alex) wrote…

Can we use a setter here and put embeddedColorMap private in the function?

Done.

---

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

Previously, Alex-NRCan (Alex) wrote…

Here, use a getter? Idea, it could be a getter like 'hasEmbeddedColorMap' next to the getter 'getEmbeddedColorMap'?

Done.

---

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

Previously, Alex-NRCan (Alex) wrote…

Move this private function down below in the class file. Not before the overrides.

Done.

[Alex-NRCan]

@Alex-NRCan reviewed 7 files and all commit messages, made 1 comment, and resolved 6 discussions.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on DamonU2 and jolevesq).

---

packages/geoview-core/src/core/utils/fetch-helper.ts line 353 at r4 (raw file):

      return response;
    } catch {
      return null;

Can we simply let it throw rather than return a Promise<Response | null> ? Just remove the 'catch' from here. You'll need to add a catch in the caller, but at least the core Fetch would behave like the other fetch functions of this class.

[DamonU2]

@DamonU2 reviewed 7 files and all commit messages, and resolved 2 discussions.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on jolevesq).

[jolevesq]

@jolevesq reviewed 6 files and all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on jolevesq).

[Alex-NRCan]

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

---

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

  const separator = '?';
  const ogcCheckUrls = [
    `${targetUrlWithoutParams}${separator}service=WMS&request=GetCapabilities`,

Suggestion: Use GeoUtilities.ensureServiceRequestUrl(targetUrl, 'GetCapabilities', 'WMS', '') and GeoUtilities.ensureServiceRequestUrl(targetUrl, 'GetCapabilities', 'WFS', '') ?
That way, other parameters (such as 'version' or even others) will remain in the whole url ? I believe it could be important to keep the 'version' in the url request.

---

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

    );

    for (const settled of directChecks) {

Minor: Maybe write a comment explaining that if any promise has fulfilled, it's good enough for us, meaning that we try WMS and WFS on purpose and if either one work, it's 'reachable' for our usecase. I was a bit surprised to see a return statement right after any fulfilled in the loop.

[Alex-NRCan]

@Alex-NRCan reviewed 1 file and all commit messages, made 1 comment, and resolved 2 discussions.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on jolevesq).

---

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

Previously, Alex-NRCan (Alex) wrote…

Minor: Maybe write a comment explaining that if any promise has fulfilled, it's good enough for us, meaning that we try WMS and WFS on purpose and if either one work, it's 'reachable' for our usecase. I was a bit surprised to see a return statement right after any fulfilled in the loop.

Nice

[Alex-NRCan]

@Alex-NRCan made 1 comment and resolved 1 discussion.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on jolevesq).

---

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

Previously, Alex-NRCan (Alex) wrote…

Suggestion: Use GeoUtilities.ensureServiceRequestUrl(targetUrl, 'GetCapabilities', 'WMS', '') and GeoUtilities.ensureServiceRequestUrl(targetUrl, 'GetCapabilities', 'WFS', '') ?
That way, other parameters (such as 'version' or even others) will remain in the whole url ? I believe it could be important to keep the 'version' in the url request.

Nice