CLIM-1291: S2D Disable low skill masking in Climatology mode by renoirb · Pull Request #666 · CanadianClimateDataPortal/climatedata-wp-theme

renoirb · 2026-03-16T17:46:01Z

Description

Disable and visually mute the "Mask Low Skill" checkbox and hide the low skill map layer when Forecast Display is set to Climatology. The "low skill" concept (forecast confidence) is only meaningful for forecast data — it has no
relevance to historical climatology.

Checkbox (MaskLowSkillField): Disabled via Radix disabled prop when forecastDisplay !== 'forecast'. Label muted via Tailwind peer-disabled:opacity-50. Checked+disabled state swaps red to grey. State in Redux store
and URL (&maskLowSkill=) is never mutated — preserved automatically.
Map layer (LowSkillLayer): Early-return null from useMemo when not Forecast. WMS tiles are not fetched, Leaflet pane stays empty.

No changes to Redux store, URL sync, map-container.tsx, sidebar-inner-s2d.tsx, or checkbox.tsx.

Changes

File	What
`apps/src/components/fields/skill.tsx`	`useClimateVariable()` for `isForecast` via `climateVariable?.getForecastDisplay()`; `disabled` on checkbox; `peer-disabled:` on label; `disabled:data-[state=checked]:` grey override
`apps/src/components/map-layers/low-skill-layer.tsx`	`climateVariable?.getForecastDisplay()` for `isForecast`; `shouldHideLayer`/`hasLayerData` named booleans; `isForecast` guard in `useMemo` early-return
`apps/src/features/map/map-slice.ts`	JSDoc on `selectLowSkillVisibility` — clarifies "mask" = vector overlay semantics
`apps/src/types/types.ts`	JSDoc on `MapState.isLowSkillVisible` — clarifies "mask" = vector overlay, not "hide" we often jump to as native French speakers
`apps/src/lib/s2d.ts`	JSDoc on `buildSkillLayerName` — meteorology context

Test plan

Open map with S2D variable (?var=s2d_air_temp&dataset=260)
Initial load: checkbox should be enabled (not disabled) when Forecast Display defaults to Forecast
Set Forecast Display to Climatology → checkbox is disabled/muted (grey, not red), no diagonal line overlay on map
Set Forecast Display to Forecast → checkbox is active, diagonal line overlay appears (if checkbox checked)
Toggle between modes — checkbox state preserved (no store/URL mutation)
Verify &maskLowSkill= URL param unchanged when switching Forecast Display

Related ticket

CLIM-1291

renoirb · 2026-03-16T18:57:06Z

2026-03-16 Screenshots

2026-03-16 Screenshot Forecasts

Notice the “Forecast Display” and the check box red we can click and when we change to Climatology.

2026-03-16 Screenshot Climatology

It becomes:

Unavailable (we can’t change its value)
Toned down in color

ChaamC

Seems like when I open the Map and an S2D variable, the Mask Low Skill button is disabled at first, even if Forecast is selected in the Forecast Display.

If I change to Climatology and then Forecast, the button seems to be enabled then and to work as intended for the feature of this PR.

Can you check if you also have that behavior and see if it can be fixed, so that the Mask Low Skill is not disabled when opening the Map on a Forecast variable?

renoirb · 2026-03-17T18:26:31Z

I was thinking that the button is disabled in any situation when it isn't exactly ForecastDisplay === 'forecast'.

I was thinking to have that button disabled by default, yes, but not to be in a loaded state, be with forecast (as the initial state), and remain disabled.

This feels like it's some race condition

undefined is a transient React re-render state, not a valid forecastDisplay value. Only positively match ForecastDisplays.FORECAST.

Swap brand-red background/border to neutral-grey-medium when disabled + checked, so the checkbox is visually obviously inactive.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@see

The word "mask" in "Mask Low Skill" refers to a vector graphics overlay (diagonal lines over low-confidence forecast regions), not "to hide" (French: masquer). This distinction caused confusion during development and risked incorrect conditional logic. Added JSDoc with @see links to MaskLowSkillField at the three authoritative locations: MapState type definition, selectLowSkillVisibility selector, and buildSkillLayerName. Co-Authored-By: Claude Opus 4.6

At initial load, state.climateVariable.data.forecastDisplay is undefined because URL-sync only sets it when the fcastDisp param is explicitly present. When absent (the default Forecast case), the Redux field stays undefined while ClimateVariableBase.getForecastDisplay() silently falls back to ForecastDisplays.FORECAST. This caused the Mask Low Skill checkbox to render disabled even when Forecast Display is Forecast — a race condition where the component saw undefined before data loaded and never recovered because the value was never written to the store. selectForecastDisplay mirrors the fallback from ClimateVariableInterface.getForecastDisplay so both access paths agree. Co-located in map-slice alongside selectLowSkillVisibility because its consumers already import from there.

renoirb · 2026-03-19T18:48:52Z

Follow-up from @ChaamC earlier review comment.

Race condition fix

The initial implementation read forecastDisplay from Redux via direct selector (state.climateVariable.data?.forecastDisplay). At page load, this value is undefined — not because the data hasn't loaded yet, but because
URL-sync only writes forecastDisplay to the store when the URL param fcastDisp= is explicitly present. For the default Forecast case, the param is omitted (it equals the default config, so addClimateVariableParamsToUrl
skips it). Meanwhile, ClimateVariableBase.getForecastDisplay() silently falls back to ForecastDisplays.FORECAST, so other components behave correctly.

The result: the checkbox rendered as disabled on initial load even when Forecast Display was Forecast, and never recovered because the Redux value was never written.

Fix: centralized selectForecastDisplay() selector in map-slice.ts that mirrors the ?? ForecastDisplays.FORECAST fallback from the class method. Both skill.tsx and low-skill-layer.tsx now use it.

xfrenette-crim · 2026-03-19T19:57:36Z

@ChaamC Before approving, I just want to validate the race condition issue, see if there is a simpler solution

…Selector

ChaamC

Looks all good to me and works.
Left a minor comment, but I approve the PR.

ChaamC · 2026-03-20T16:02:39Z

apps/src/lib/s2d.ts

 /**
 * Create and return the GeoServer layer name for the Skill layer.
 *
+ * "Skill" in meteorology refers to forecast accuracy relative to a baseline


I think I wouldn't include this first sentence of the description. Feels like overexplaining the concept of skill, which is already documented for example in our Confluence.

Well. I was keeping bumping about what "skill" actually mean. That's why I wanted to clarify. The code has "forecast" and "climatology". With explanation about a "baseline" and that that base is... climatology. It makes sense.

renoirb · 2026-03-20T18:36:25Z

Follow-up from my own earlier response.

Race condition fix

The initial implementation read forecastDisplay from Redux via direct selector (state.climateVariable.data?.forecastDisplay). At page load, this value is undefined — not because the data hasn't loaded yet, but because URL-sync only writes forecastDisplay (...)

The "race condition" was caused by using a pattern that could have been because I wanted to avoid using climateVariable.getForecastDisplay() but that attempt to avoid "load more code" caused that race condition.

renoirb · 2026-03-20T19:05:33Z

apps/src/components/map-layers/low-skill-layer.tsx

+	const isForecast = forecastDisplay === ForecastDisplays.FORECAST;
 	const { releaseDate } = useS2D();
-	const isLowSkillMasked = !useAppSelector(selectLowSkillVisibility());
+	const isLowSkillMaskHidden = !useAppSelector(selectLowSkillVisibility());


~~If it's the opposite of the value, let's make sure we mean "hidden" or "not visible"~~

Suggestion, if the outcome of selectLowSkillVisibility is actually the value of isLowSkillVisible from the type MapState. And that we're flipping it to the opposite. We could instead call that variable what it is... the opposite of isLowSkillVisible ... something like notIsLowSkillVisible

…LowSkillVisible and flip later where needed.

renoirb marked this pull request as draft March 16, 2026 17:46

renoirb self-assigned this Mar 16, 2026

renoirb requested review from ChaamC and xfrenette-crim March 16, 2026 18:29

renoirb marked this pull request as ready for review March 16, 2026 18:30

ChaamC reviewed Mar 17, 2026

View reviewed changes

renoirb and others added 10 commits March 19, 2026 14:24

CLIM-1291: BEGIN

04dfd31

CLIM-1291: Disable checkbox when not Forecast

4c2c388

CLIM-1291: Hide low skill layer when not Forecast

215c571

CLIM-1291: Remove undefined fallback from isForecast

8d8669a

undefined is a transient React re-render state, not a valid forecastDisplay value. Only positively match ForecastDisplays.FORECAST.

CLIM-1291: One-per-line for conditional and deps array

9d15407

CLIM-1291: ordering

0600e49

CLIM-1291: Mute checkbox red when disabled

aa3b01d

Swap brand-red background/border to neutral-grey-medium when disabled + checked, so the checkbox is visually obviously inactive.

CLIM-1291: Extract named booleans for layer guard

44e8bda

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

renoirb force-pushed the CLIM-1291_S2D-Map-When-Climatology-Hide-Skill-Layer branch from e0267ed to ea48021 Compare March 19, 2026 18:24

CLIM-1291: Use climateVariable.getForecastDisplay() instead of useApp…

f496f99

…Selector

ChaamC approved these changes Mar 20, 2026

View reviewed changes

renoirb commented Mar 20, 2026

View reviewed changes

CLIM-1291: Rename confusing variable name from isLowSkillMasked to is…

d61643a

…LowSkillVisible and flip later where needed.

renoirb force-pushed the CLIM-1291_S2D-Map-When-Climatology-Hide-Skill-Layer branch from 533b3ea to d61643a Compare March 20, 2026 20:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLIM-1291: S2D Disable low skill masking in Climatology mode#666

CLIM-1291: S2D Disable low skill masking in Climatology mode#666
renoirb wants to merge 12 commits intomainfrom
CLIM-1291_S2D-Map-When-Climatology-Hide-Skill-Layer

renoirb commented Mar 16, 2026 •

edited

Loading

Uh oh!

renoirb commented Mar 16, 2026

Uh oh!

ChaamC left a comment

Uh oh!

renoirb commented Mar 17, 2026 •

edited

Loading

Uh oh!

renoirb commented Mar 19, 2026

Uh oh!

xfrenette-crim commented Mar 19, 2026

Uh oh!

ChaamC left a comment

Uh oh!

ChaamC Mar 20, 2026

Uh oh!

renoirb Mar 20, 2026 •

edited

Loading

Uh oh!

renoirb commented Mar 20, 2026 •

edited

Loading

Race condition fix

Uh oh!

renoirb Mar 20, 2026 •

edited

Loading

Uh oh!

renoirb Mar 20, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

renoirb commented Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Test plan

Related ticket

Uh oh!

renoirb commented Mar 16, 2026

2026-03-16 Screenshots

2026-03-16 Screenshot Forecasts

2026-03-16 Screenshot Climatology

Uh oh!

ChaamC left a comment

Choose a reason for hiding this comment

Uh oh!

renoirb commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

renoirb commented Mar 19, 2026

Race condition fix

Uh oh!

xfrenette-crim commented Mar 19, 2026

Uh oh!

ChaamC left a comment

Choose a reason for hiding this comment

Uh oh!

ChaamC Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

renoirb Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

renoirb commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Race condition fix

Uh oh!

renoirb Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

renoirb Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

renoirb commented Mar 16, 2026 •

edited

Loading

renoirb commented Mar 17, 2026 •

edited

Loading

renoirb Mar 20, 2026 •

edited

Loading

renoirb commented Mar 20, 2026 •

edited

Loading

renoirb Mar 20, 2026 •

edited

Loading

renoirb Mar 20, 2026 •

edited

Loading