From f7f57688812f011a5c06dabc755af8d8b062aac5 Mon Sep 17 00:00:00 2001 From: Corey Krewson Date: Mon, 17 Aug 2026 09:48:26 -0700 Subject: [PATCH 1/5] Fix number variable inputs dropping decimals and zeros MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `number` variable input could not carry a fractional value, and any variable input legitimately set to 0 was indistinguishable from unset. Binding a plugin arg to a threshold like 0.15 therefore produced " variable is empty" on every dependent visualization. Four falsy/integer assumptions caused it: - VariableInput parsed the value with parseInt on both mount and change, so 0.15 became 0. Fixed with a shared toNumberOrEmpty helper, which also maps an unparseable entry to "" rather than NaN — NaN would otherwise read as a set value. - checkForEmptyVariableInputs tested truthiness, so a numeric 0 or a false checkbox reported as empty. - updateObjectWithVariableInputs used `|| ""` in the type-preserving branch, collapsing 0 and false to "" — inconsistent with the `?? ""` in the sibling branch, so plugins silently fell back to their own defaults instead of receiving the configured 0. - VariableInput's context-sync effect gated on truthiness, so a 0 arriving from the context never synced into local state. DashboardLoader now applies the same numeric coercion when it seeds the context. Without it the boot-time seed was the raw string ("0.3") while the mount effect published a number (0.3), leaving the context value's type dependent on which landed last. Co-Authored-By: Claude Opus 5 --- .../visualizations/VariableInput.test.js | 56 +++++++++++++++++ .../visualizations/utilities.test.js | 63 +++++++++++++++++++ reactapp/components/loader/DashboardLoader.js | 9 +++ .../visualizations/VariableInput.js | 14 +++-- .../components/visualizations/utilities.js | 37 ++++++++++- 5 files changed, 172 insertions(+), 7 deletions(-) diff --git a/reactapp/__tests__/components/visualizations/VariableInput.test.js b/reactapp/__tests__/components/visualizations/VariableInput.test.js index ba74887b..d97ac09a 100644 --- a/reactapp/__tests__/components/visualizations/VariableInput.test.js +++ b/reactapp/__tests__/components/visualizations/VariableInput.test.js @@ -860,6 +860,62 @@ it("Creates a Number Input for a Variable Input", async () => { ); }); +it("Keeps the decimals of a Number Input", async () => { + // parseInt truncated every fraction to its integer part, so a threshold of + // 0.15 silently became 0 and the box snapped back to "0" mid-typing. + const user = userEvent.setup(); + const dashboard = JSON.parse(JSON.stringify(userDashboard)); + // DashboardLoader seeds the context straight from the grid item's args, so the + // stored initial_value has to be fractional too -- not just the prop. + const varInputArgs = { + ...JSON.parse(mockedNumberVariable.args_string), + initial_value: "0.3", + }; + dashboard.tabs[0].gridItems = [ + { ...mockedNumberVariable, args_string: JSON.stringify(varInputArgs) }, + ]; + const handleChange = jest.fn(); + + render( + createLoadedComponent({ + children: ( + <> + + + + ), + options: { dashboards: { dashboards: [dashboard] } }, + }), + ); + + // A fractional initial_value survives mount and reaches the context. + expect(await screen.findByTestId("input-variables")).toHaveTextContent( + JSON.stringify({ "Test Variable": 0.3 }), + ); + + const variableInput = await screen.findByRole("textbox"); + + // Set the value outright rather than clear-then-type: NormalInput swallows an + // empty number input on purpose (allowEmpty is false), so user.clear() leaves + // the parent's value untouched and the resync effect can restore "0.3", + // appending the typed digits to it. + fireEvent.change(variableInput, { target: { value: "0.15" } }); + + // The fraction survives instead of being truncated to 0. + expect(variableInput).toHaveValue("0.15"); + expect(handleChange).toHaveBeenLastCalledWith(0.15); + + await user.click(screen.getByRole("button")); + expect(await screen.findByTestId("input-variables")).toHaveTextContent( + JSON.stringify({ "Test Variable": 0.15 }), + ); +}); + it("Creates a Checkbox Input for a Variable Input", async () => { const user = userEvent.setup(); const dashboard = JSON.parse(JSON.stringify(userDashboard)); diff --git a/reactapp/__tests__/components/visualizations/utilities.test.js b/reactapp/__tests__/components/visualizations/utilities.test.js index 7c165cc0..0c4ce199 100644 --- a/reactapp/__tests__/components/visualizations/utilities.test.js +++ b/reactapp/__tests__/components/visualizations/utilities.test.js @@ -1664,6 +1664,69 @@ test("checkForEmptyVariableInputs", async () => { expect(emptyVariableWarnings).toStrictEqual(null); }); +describe("checkForEmptyVariableInputs treats 0 and false as set", () => { + // A truthiness test used to report a numeric 0 or an unchecked checkbox as + // "empty", which made every fractional threshold unusable once parseFloat + // started preserving them. + // eslint-disable-next-line no-template-curly-in-string + const argsString = JSON.stringify({ threshold: "${Threshold}" }); + const metadataString = JSON.stringify({}); + const check = (variableInputValues) => + checkForEmptyVariableInputs({ + metadataString, + argsString, + variableInputValues, + }); + + test.each([ + ["numeric zero", 0], + ["a fraction", 0.15], + ["boolean false", false], + ["the string zero", "0"], + ])("%s is not empty", (_label, value) => { + expect(check({ Threshold: value })).toStrictEqual(null); + }); + + test.each([ + ["undefined", undefined], + ["null", null], + ["an empty string", ""], + ])("%s is empty", (_label, value) => { + expect(check({ Threshold: value })).toStrictEqual([ + "Threshold variable is empty", + ]); + }); + + test("a missing key is still empty", () => { + expect(check({})).toStrictEqual(["Threshold variable is empty"]); + }); +}); + +test("updateObjectWithVariableInputs preserves 0 and false", () => { + // The exact-match branch used `|| ""`, so a zero threshold reached the plugin + // as an empty string and silently fell back to the plugin's own default. + const result = updateObjectWithVariableInputs({ + args: { + // eslint-disable-next-line no-template-curly-in-string + zero: "${Zero}", + // eslint-disable-next-line no-template-curly-in-string + fraction: "${Fraction}", + // eslint-disable-next-line no-template-curly-in-string + off: "${Off}", + // eslint-disable-next-line no-template-curly-in-string + missing: "${Missing}", + }, + variableInputs: { Zero: 0, Fraction: 0.15, Off: false }, + }); + + expect(result).toStrictEqual({ + zero: 0, + fraction: 0.15, + off: false, + missing: "", + }); +}); + test("checkForEmptyVariableInputs skips feature.* keys", () => { // feature.* keys are scoped/unbinding-by-design; they should never // produce a warning regardless of whether the value is set. diff --git a/reactapp/components/loader/DashboardLoader.js b/reactapp/components/loader/DashboardLoader.js index eeb794a7..b125c0e7 100644 --- a/reactapp/components/loader/DashboardLoader.js +++ b/reactapp/components/loader/DashboardLoader.js @@ -19,6 +19,7 @@ import { AvailableDashboardsContext, TabContext, } from "components/contexts/Contexts"; +import { toNumberOrEmpty } from "components/visualizations/utilities"; import Error from "components/error/Error"; import errorImage from "assets/error404.png"; @@ -131,6 +132,14 @@ const DashboardLoader = ({ initialValue = false; } + // Seed the same type VariableInput itself publishes. Without this + // the boot-time seed is the raw string ("0.3") while the mount + // effect publishes a number (0.3), so the context value's type + // depended on which landed last. + if (args.variable_options_source === "number") { + initialValue = toNumberOrEmpty(initialValue); + } + if (args.variable_options_source.includes("date")) { dateFormat = args?.["variable_options_source.metadata"]?.format || ""; diff --git a/reactapp/components/visualizations/VariableInput.js b/reactapp/components/visualizations/VariableInput.js index a551beb9..249b6d6b 100644 --- a/reactapp/components/visualizations/VariableInput.js +++ b/reactapp/components/visualizations/VariableInput.js @@ -10,6 +10,8 @@ import { import { nonDropDownVariableInputTypes, findSelectOptionByValue, + hasVariableInputValue, + toNumberOrEmpty, updateObjectWithVariableInputs, } from "components/visualizations/utilities"; import TooltipButton from "components/buttons/TooltipButton"; @@ -160,8 +162,10 @@ const VariableInput = ({ } if (variable_options_source === "number") { - // If the variable_options_source is a number, it parses the int value from initial_value - initialVariableValue = parseInt(initial_value); + // parseFloat, not parseInt: a number input must accept decimals. + // parseInt turned every fractional initial value into its integer part + // (0.15 -> 0), and 0 then read as unset everywhere downstream. + initialVariableValue = toNumberOrEmpty(initial_value); variableValue = initialVariableValue; } else if ( variable_options_source === "checkbox" && @@ -186,7 +190,9 @@ const VariableInput = ({ if (Array.isArray(type) && type.length > 0) { newValue = findSelectOptionByValue(type, newValue); } - if (newValue && value !== newValue) { + // hasVariableInputValue, not truthiness, so a 0 or false arriving from the + // context still syncs into local state. + if (hasVariableInputValue(newValue) && value !== newValue) { setValue(newValue); } // eslint-disable-next-line react-hooks/exhaustive-deps @@ -196,7 +202,7 @@ const VariableInput = ({ (e) => { let inputValue = e; if (variable_options_source === "number") { - inputValue = parseInt(e); + inputValue = toNumberOrEmpty(e); } setValue(inputValue); onChange(inputValue); diff --git a/reactapp/components/visualizations/utilities.js b/reactapp/components/visualizations/utilities.js index e05eb957..0a95ccf7 100644 --- a/reactapp/components/visualizations/utilities.js +++ b/reactapp/components/visualizations/utilities.js @@ -74,6 +74,30 @@ export function clearImageVizCache() { imageVizCache.clear(); } +/** + * A number variable input's value as a number, or "" when it does not parse. + * + * parseFloat, not parseInt: a number input must accept decimals, and parseInt + * turned every fractional value into its integer part (0.15 -> 0). NaN would + * defeat hasVariableInputValue -- it is neither null nor "" -- so an unparseable + * entry becomes "" instead, letting the empty-variable check report it. + */ +export function toNumberOrEmpty(value) { + const parsed = parseFloat(value); + return Number.isNaN(parsed) ? "" : parsed; +} + +/** + * True when a variable input actually holds a value. + * + * Deliberately not a truthiness test. `0` and `false` are legitimate values for + * number and checkbox inputs, and treating them as unset made a threshold of 0 + * report as "variable is empty" while substituting "" into the args. + */ +export function hasVariableInputValue(value) { + return value !== undefined && value !== null && value !== ""; +} + /** * Returns an array of warning messages when any variable inputs referenced by a * visualization's args have no value, or null if all inputs are populated. @@ -102,9 +126,13 @@ export function checkForEmptyVariableInputs({ ); let warnings = []; - if (!dependentVariableInputs.every((key) => variableInputValues[key])) { + if ( + !dependentVariableInputs.every((key) => + hasVariableInputValue(variableInputValues[key]), + ) + ) { for (const dependentVariableInput of dependentVariableInputs) { - if (!variableInputValues[dependentVariableInput]) { + if (!hasVariableInputValue(variableInputValues[dependentVariableInput])) { warnings.push( metadata.customMessaging?.[dependentVariableInput] ?? `${dependentVariableInput} variable is empty`, @@ -496,7 +524,10 @@ export function updateObjectWithVariableInputs({ ) { updatedValuesWithVariableInputs = value; } else { - updatedValuesWithVariableInputs = variableInputsCopy[key] || ""; + // Nullish, not falsy: this branch exists to PRESERVE the value's type, + // so a numeric 0 or a false checkbox must survive rather than collapse + // to "". Matches the `?? ""` in the inline-replacement branch below. + updatedValuesWithVariableInputs = variableInputsCopy[key] ?? ""; } } else { // Value contains one or more inline ${variableName} placeholders mixed From 61a31dc8b925d423e3efe1510afc97f552a9f850 Mon Sep 17 00:00:00 2001 From: Corey Krewson Date: Mon, 17 Aug 2026 10:30:31 -0700 Subject: [PATCH 2/5] Stop a number input's resync from eating a decimal point mid-edit Backspacing "0.3" down to "0." deleted the decimal point too, so the next keystroke produced 5 instead of 0.5. The parent normalizes whatever the input publishes and feeds the number back as `value`. "0." parses to 0, so the resync effect echoed "0" and overwrote the entry in progress. Trailing zeros were lost the same way: typing "0.50" published 0.5, which came back as "0.5". The effect now skips the resync when the raw text already denotes the incoming number, so only a genuinely different value overwrites the box. An external update still applies, since its number differs from what is on screen. This also drops an accidental assertion in SettingsPane.test.js. It expected a rejected negative refresh rate to leave "0" in the box, which only held because the mount effect happened to flush after the change event and overwrite the entry. onRefreshRateChange rejects negatives rather than clamping them, so once mounted the box keeps what was typed while the setting stays unchanged -- unaltered by this commit, and the test now states it. Co-Authored-By: Claude Opus 5 --- .../components/inputs/NormalInput.test.js | 46 +++++++++++++++++++ .../modals/DataViewer/SettingsPane.test.js | 7 ++- reactapp/components/inputs/NormalInput.js | 12 ++++- 3 files changed, 62 insertions(+), 3 deletions(-) diff --git a/reactapp/__tests__/components/inputs/NormalInput.test.js b/reactapp/__tests__/components/inputs/NormalInput.test.js index 90a8854d..9b3c7383 100644 --- a/reactapp/__tests__/components/inputs/NormalInput.test.js +++ b/reactapp/__tests__/components/inputs/NormalInput.test.js @@ -1,4 +1,5 @@ import { render, screen, fireEvent } from "@testing-library/react"; +import { useState } from "react"; import NormalInput from "components/inputs/NormalInput"; /* eslint-disable no-template-curly-in-string */ @@ -427,4 +428,49 @@ describe("NormalInput Component", () => { ); expect(input).toHaveValue("20"); }); + + describe("editing a decimal under a normalizing parent", () => { + // Mirrors the real chain: the parent parses whatever the input publishes and + // feeds the number back as `value`. "0." parses to 0, so a naive resync + // echoes "0" and eats the decimal point mid-edit. + // Both cases start from 0.3, so no props are needed -- which also keeps the + // helper clear of prop-types lint. + const ControlledNumber = () => { + const [value, setValue] = useState(0.3); + return ( + { + const parsed = parseFloat(e.target.value); + setValue(Number.isNaN(parsed) ? "" : parsed); + }} + /> + ); + }; + + test("backspacing 0.3 to 0. keeps the decimal point", () => { + render(); + const input = screen.getByLabelText("Num Input"); + expect(input).toHaveValue("0.3"); + + fireEvent.change(input, { target: { value: "0." } }); + expect(input).toHaveValue("0."); + + // Typing the next digit therefore yields 0.5, not 5. + fireEvent.change(input, { target: { value: "0.5" } }); + expect(input).toHaveValue("0.5"); + }); + + test("a trailing zero survives while it is being typed", () => { + // Starts at 0.3 so the parent's value genuinely changes to 0.5 and the + // resync fires. String(0.5) is "0.5", which would drop the typed zero. + render(); + const input = screen.getByLabelText("Num Input"); + + fireEvent.change(input, { target: { value: "0.50" } }); + expect(input).toHaveValue("0.50"); + }); + }); }); diff --git a/reactapp/__tests__/components/modals/DataViewer/SettingsPane.test.js b/reactapp/__tests__/components/modals/DataViewer/SettingsPane.test.js index 2da16130..b72083ed 100644 --- a/reactapp/__tests__/components/modals/DataViewer/SettingsPane.test.js +++ b/reactapp/__tests__/components/modals/DataViewer/SettingsPane.test.js @@ -88,7 +88,12 @@ test("Settings Pane with visualizationRef Element", async () => { ); expect(refreshRateInput).toBeInTheDocument(); fireEvent.change(refreshRateInput, { target: { value: -2 } }); - expect(refreshRateInput.value).toBe("0"); + // onRefreshRateChange rejects negatives rather than clamping them, so the + // setting is left alone while the box keeps what was typed. This previously + // asserted "0", which only held because the mount effect happened to flush + // after this change and overwrite the entry -- a timing artifact of the test, + // not the behaviour a user sees once the component has mounted. + expect(refreshRateInput.value).toBe("-2"); await expectSettings(JSON.stringify({})); diff --git a/reactapp/components/inputs/NormalInput.js b/reactapp/components/inputs/NormalInput.js index 7d222c72..38d7fc41 100644 --- a/reactapp/components/inputs/NormalInput.js +++ b/reactapp/components/inputs/NormalInput.js @@ -28,9 +28,17 @@ const NormalInput = ({ useEffect(() => { const strValue = String(value ?? ""); - if (strValue !== "NaN") { - setRawValue(strValue); + if (strValue === "NaN") return; + // Don't overwrite an in-progress entry that already denotes this number. + // The parent normalizes what it receives, so backspacing "0.3" to "0." + // publishes 0 and echoes "0" back -- which would eat the decimal point the + // user deliberately left, making the next keystroke read 5 instead of 0.5. + // Trailing zeros ("0.50") are preserved for the same reason. + if (isNumber && rawValue !== "" && Number(rawValue) === Number(value)) { + return; } + setRawValue(strValue); + // eslint-disable-next-line react-hooks/exhaustive-deps }, [value]); const handleChange = (e) => { From 46e689a96188c960085ed6cb4a02ffa508430ee1 Mon Sep 17 00:00:00 2001 From: Corey Krewson Date: Mon, 17 Aug 2026 11:19:49 -0700 Subject: [PATCH 3/5] Fix dynamic map layers staying blank when the fetch wins the race A dynamic_map_layer could load with a valid, non-empty response and paint nothing. Changing one of its bound arguments made it appear, which is what made it look intermittent. Map.js builds its layers asynchronously, so on a dashboard load the feature fetch can resolve before the OpenLayers layer it belongs to exists. performFetch looked the layer up once, and silently discarded the payload when it was not there yet. It never recovered: the resolved args are recorded before the request goes out, so the reconciliation effect then saw argsUnchanged and never refetched. Only a genuine argument change produced another fetch -- hence the layer appearing after a threshold edit. The payload is now held and applied on the layer collection's "add" event, so it paints as soon as the layer exists without a second request. The listener is released when it fires, when a newer fetch supersedes it, when the layer is removed, and on unmount. A response arriving with no map at all now clears the recorded args so the next reconciliation retries rather than treating the fetch as done. Also fixes the requestId carrying a literal "undefined" for the grid item: the hook destructured gridItemUuid while its only caller passes gridItemUUID, the name GridItemContext uses. Per-layer progress messages were routed under that malformed id. The hook's own tests passed the lowercase spelling, so nothing caught it -- the new Map test wraps a real GridItemContext and asserts the id is well formed. Co-Authored-By: Claude Opus 5 --- .../components/visualizations/Map.test.js | 93 ++++++++- .../runtimeLayerFetcher.test.js | 178 +++++++++++++----- .../visualizations/runtimeLayerFetcher.js | 82 ++++++-- 3 files changed, 290 insertions(+), 63 deletions(-) diff --git a/reactapp/__tests__/components/visualizations/Map.test.js b/reactapp/__tests__/components/visualizations/Map.test.js index a47346e5..8ac9803d 100644 --- a/reactapp/__tests__/components/visualizations/Map.test.js +++ b/reactapp/__tests__/components/visualizations/Map.test.js @@ -14,12 +14,16 @@ import { Map } from "ol"; import ImageArcGISRest from "ol/source/ImageArcGISRest.js"; import VariableInput from "components/visualizations/VariableInput"; import { Vector as VectorSource } from "ol/source.js"; +import { GridItemContext } from "components/contexts/Contexts"; import appAPI from "services/api/app"; import { applyStyle } from "ol-mapbox-style"; import Point from "ol/geom/Point.js"; import LineString from "ol/geom/LineString.js"; import Feature from "ol/Feature.js"; -import { queryLayerFeatures } from "components/map/utilities"; +import { + queryLayerFeatures, + swapVectorLayerFeatures, +} from "components/map/utilities"; import { fetchLayerVectorFeatures } from "components/map/snapping"; import Overlay from "ol/Overlay"; import { @@ -77,6 +81,7 @@ jest.mock("components/map/snapping", () => { }; }); const mockedQueryLayerFeatures = jest.mocked(queryLayerFeatures); +const mockedSwapVectorLayerFeatures = jest.mocked(swapVectorLayerFeatures); const mockedFetchLayerVectorFeatures = jest.mocked(fetchLayerVectorFeatures); const exampleGeoJSON = { @@ -4222,6 +4227,92 @@ test("Map runtime layer swap dismisses popup overlay (no prior click)", async () expect(vectorClearSpy).not.toHaveBeenCalled(); }); +test("Map runtime layer hands features to the swap and identifies its grid item", async () => { + // End-to-end guard for two separate failures that each left a dynamic layer + // silently blank on dashboard load: + // 1. the fetch resolving before Map.js finished building the OL layer, and + // 2. the requestId carrying "undefined" for the grid item, because the hook + // destructured gridItemUuid while the caller passes gridItemUUID. + // Neither showed up in the hook's own tests: those pass the map and the prop + // name the hook expects, so the race and the casing mismatch were invisible. + const mockGetVisualizationFeatures = jest + .spyOn(appAPI, "getVisualizationFeatures") + .mockResolvedValue({ + success: true, + viz_type: "features", + data: { + type: "FeatureCollection", + features: [ + { + type: "Feature", + properties: { peligro: 3, nivel: "Alto" }, + geometry: { + type: "Polygon", + coordinates: [ + [ + [-90.54, 14.48], + [-90.53, 14.48], + [-90.53, 14.49], + [-90.54, 14.48], + ], + ], + }, + }, + ], + crs: { type: "name", properties: { name: "EPSG:4326" } }, + }, + }); + mockedSwapVectorLayerFeatures.mockClear(); + + const layers = [JSON.parse(JSON.stringify(dynamicMapLayer))]; + const layerId = layers[0].configuration.props.layerId; + + render( + createLoadedComponent({ + children: ( + + + + + + ), + }), + ); + + expect(await screen.findByLabelText("Map Div")).toBeInTheDocument(); + expect(await screen.findByText("Map Ready")).toBeInTheDocument(); + await waitFor(() => { + expect(mockGetVisualizationFeatures).toHaveBeenCalledTimes(1); + }); + + const { requestId } = mockGetVisualizationFeatures.mock.calls[0][0]; + expect(requestId).not.toContain("undefined"); + expect(requestId).toContain(":grid-uuid-1:"); + expect(requestId.endsWith(`:${layerId}`)).toBe(true); + + // The payload reaches the swap, aimed at the OL layer carrying this layerId. + // components/map/utilities is mocked in this file, so this asserts the + // hand-off rather than OpenLayers' own parsing; the deferred path -- when the + // fetch wins the race against layer construction -- is covered by + // runtimeLayerFetcher.test.js. + await waitFor(() => { + expect(mockedSwapVectorLayerFeatures).toHaveBeenCalledTimes(1); + }); + const [targetLayer, collection] = mockedSwapVectorLayerFeatures.mock.calls[0]; + expect(targetLayer.get("layerId")).toBe(layerId); + expect(collection.features).toHaveLength(1); + expect(collection.features[0].properties.nivel).toBe("Alto"); +}); + test("Map runtime layer swap dismisses popup and clears highlight after click", async () => { jest.spyOn(appAPI, "getVisualizationFeatures").mockResolvedValue({ success: true, diff --git a/reactapp/__tests__/components/visualizations/runtimeLayerFetcher.test.js b/reactapp/__tests__/components/visualizations/runtimeLayerFetcher.test.js index e05a1257..a0a33698 100644 --- a/reactapp/__tests__/components/visualizations/runtimeLayerFetcher.test.js +++ b/reactapp/__tests__/components/visualizations/runtimeLayerFetcher.test.js @@ -21,11 +21,32 @@ function fakeOlLayer(layerId) { } function fakeOlMap(olLayers) { + // The layer collection must be a stable object with working on/un, because a + // fetch that lands before its layer exists waits on the collection's "add" + // event rather than discarding the payload. + const addListeners = []; + const collection = { + getArray: () => olLayers, + on: (type, fn) => { + if (type === "add") addListeners.push(fn); + }, + un: (type, fn) => { + if (type !== "add") return; + const i = addListeners.indexOf(fn); + if (i !== -1) addListeners.splice(i, 1); + }, + }; return { - getLayers: () => ({ getArray: () => olLayers }), + getLayers: () => collection, getView: () => ({ getProjection: () => ({ getCode: () => "EPSG:3857" }), }), + // Mimics Map.js finishing its async layer construction. + addLayerLate: (layer) => { + olLayers.push(layer); + addListeners.slice().forEach((fn) => fn()); + }, + pendingAddListeners: () => addListeners.length, }; } @@ -99,7 +120,7 @@ describe("useRuntimeLayerFetcher", () => { renderHook(() => useRuntimeLayerFetcher({ layers, - gridItemUuid: "grid-a", + gridItemUUID: "grid-a", sessionNonce: "nonce", mapRef, variableInputValues: {}, @@ -140,7 +161,7 @@ describe("useRuntimeLayerFetcher", () => { ({ variableInputValues }) => useRuntimeLayerFetcher({ layers, - gridItemUuid: "g", + gridItemUUID: "g", sessionNonce: "n", mapRef, variableInputValues, @@ -174,7 +195,7 @@ describe("useRuntimeLayerFetcher", () => { ({ variableInputValues }) => useRuntimeLayerFetcher({ layers, - gridItemUuid: "g", + gridItemUUID: "g", sessionNonce: "n", mapRef, variableInputValues, @@ -212,7 +233,7 @@ describe("useRuntimeLayerFetcher", () => { renderHook(() => useRuntimeLayerFetcher({ layers, - gridItemUuid: "g", + gridItemUUID: "g", sessionNonce: "n", mapRef, variableInputValues: { X: 1 }, @@ -245,7 +266,7 @@ describe("useRuntimeLayerFetcher", () => { const { result } = renderHook(() => useRuntimeLayerFetcher({ layers, - gridItemUuid: "g", + gridItemUUID: "g", sessionNonce: "n", mapRef, variableInputValues: {}, @@ -288,7 +309,7 @@ describe("useRuntimeLayerFetcher", () => { const { result } = renderHook(() => useRuntimeLayerFetcher({ layers, - gridItemUuid: "g", + gridItemUUID: "g", sessionNonce: "n", mapRef, variableInputValues: {}, @@ -325,7 +346,7 @@ describe("useRuntimeLayerFetcher", () => { const { result } = renderHook(() => useRuntimeLayerFetcher({ layers, - gridItemUuid: "g", + gridItemUUID: "g", sessionNonce: "n", mapRef, variableInputValues: {}, @@ -380,7 +401,7 @@ describe("useRuntimeLayerFetcher", () => { const { result } = renderHook(() => useRuntimeLayerFetcher({ layers, - gridItemUuid: "g", + gridItemUUID: "g", sessionNonce: "n", mapRef, variableInputValues: {}, @@ -431,7 +452,7 @@ describe("useRuntimeLayerFetcher", () => { const { unmount } = renderHook(() => useRuntimeLayerFetcher({ layers, - gridItemUuid: "g", + gridItemUUID: "g", sessionNonce: "n", mapRef, variableInputValues: {}, @@ -472,7 +493,7 @@ describe("useRuntimeLayerFetcher", () => { renderHook(() => useRuntimeLayerFetcher({ layers, - gridItemUuid: "g", + gridItemUUID: "g", sessionNonce: "n", mapRef: { current: fakeOlMap([]) }, variableInputValues: {}, @@ -497,7 +518,7 @@ describe("useRuntimeLayerFetcher", () => { ({ refreshTick }) => useRuntimeLayerFetcher({ layers, - gridItemUuid: "g", + gridItemUUID: "g", sessionNonce: "n", mapRef, variableInputValues: {}, @@ -540,7 +561,7 @@ describe("useRuntimeLayerFetcher", () => { const { unmount } = renderHook(() => useRuntimeLayerFetcher({ layers, - gridItemUuid: "g", + gridItemUUID: "g", sessionNonce: "n", mapRef, variableInputValues: {}, @@ -572,7 +593,7 @@ describe("useRuntimeLayerFetcher", () => { renderHook(() => useRuntimeLayerFetcher({ layers, - gridItemUuid: "g", + gridItemUUID: "g", sessionNonce: "n", mapRef, variableInputValues: {}, @@ -607,7 +628,7 @@ describe("useRuntimeLayerFetcher", () => { const { result } = renderHook(() => useRuntimeLayerFetcher({ layers, - gridItemUuid: "g", + gridItemUUID: "g", sessionNonce: "n", mapRef, variableInputValues: {}, @@ -638,7 +659,7 @@ describe("useRuntimeLayerFetcher", () => { const { result } = renderHook(() => useRuntimeLayerFetcher({ layers, - gridItemUuid: "g", + gridItemUUID: "g", sessionNonce: "n", mapRef, variableInputValues: {}, @@ -676,7 +697,7 @@ describe("useRuntimeLayerFetcher", () => { ({ layers }) => useRuntimeLayerFetcher({ layers, - gridItemUuid: "g", + gridItemUUID: "g", sessionNonce: "n", mapRef, variableInputValues: {}, @@ -720,7 +741,7 @@ describe("useRuntimeLayerFetcher", () => { ({ layers }) => useRuntimeLayerFetcher({ layers, - gridItemUuid: "g", + gridItemUUID: "g", sessionNonce: "n", mapRef, variableInputValues: {}, @@ -751,7 +772,7 @@ describe("useRuntimeLayerFetcher", () => { const { result } = renderHook(() => useRuntimeLayerFetcher({ layers, - gridItemUuid: "g", + gridItemUUID: "g", sessionNonce: "n", mapRef: null, variableInputValues: {}, @@ -770,30 +791,87 @@ describe("useRuntimeLayerFetcher", () => { expect(result.current.errorsByLayerId).toEqual({}); }); - test("OL map without a matching layerId skips the swap", async () => { - const otherLayer = fakeOlLayer("other-layer"); - const mapRef = { current: fakeOlMap([otherLayer]) }; - const layers = [runtimeLayerConfig({ layerId: "layer-1" })]; + describe("when the fetch lands before its OL layer exists", () => { + // Map.js builds layers asynchronously, so on a dashboard load the first + // fetch can win the race. The payload must be held rather than dropped: + // performFetch records the resolved args before requesting, so a discarded + // payload was never refetched and the layer stayed blank until an argument + // actually changed. + const setup = () => { + const otherLayer = fakeOlLayer("other-layer"); + const map = fakeOlMap([otherLayer]); + const layers = [runtimeLayerConfig({ layerId: "layer-1" })]; + const view = renderHook(() => + useRuntimeLayerFetcher({ + layers, + gridItemUUID: "g", + sessionNonce: "n", + mapRef: { current: map }, + variableInputValues: {}, + variableInputDateFormats: {}, + }), + ); + return { map, layers, view }; + }; + + const settle = async () => { + await act(async () => { + jest.advanceTimersByTime(250); + await Promise.resolve(); + await Promise.resolve(); + }); + }; - renderHook(() => - useRuntimeLayerFetcher({ - layers, - gridItemUuid: "g", - sessionNonce: "n", - mapRef, - variableInputValues: {}, - variableInputDateFormats: {}, - }), - ); + test("paints as soon as the layer is added", async () => { + const { map } = setup(); + await settle(); - await act(async () => { - jest.advanceTimersByTime(250); - await Promise.resolve(); - await Promise.resolve(); + expect(getFeaturesMock).toHaveBeenCalledTimes(1); + expect(swapSpy).not.toHaveBeenCalled(); + expect(map.pendingAddListeners()).toBe(1); + + const late = fakeOlLayer("layer-1"); + await act(async () => { + map.addLayerLate(late); + }); + + expect(swapSpy).toHaveBeenCalledTimes(1); + expect(swapSpy.mock.calls[0][0]).toBe(late); + // No refetch was needed to get the features onto the map. + expect(getFeaturesMock).toHaveBeenCalledTimes(1); + // The listener is released once it has fired. + expect(map.pendingAddListeners()).toBe(0); }); - expect(getFeaturesMock).toHaveBeenCalledTimes(1); - expect(swapSpy).not.toHaveBeenCalled(); + test("an unrelated layer arriving does not consume the pending swap", async () => { + const { map } = setup(); + await settle(); + + await act(async () => { + map.addLayerLate(fakeOlLayer("someone-else")); + }); + expect(swapSpy).not.toHaveBeenCalled(); + expect(map.pendingAddListeners()).toBe(1); + + await act(async () => { + map.addLayerLate(fakeOlLayer("layer-1")); + }); + expect(swapSpy).toHaveBeenCalledTimes(1); + }); + + test("unmounting releases the pending listener", async () => { + const { map, view } = setup(); + await settle(); + expect(map.pendingAddListeners()).toBe(1); + + view.unmount(); + expect(map.pendingAddListeners()).toBe(0); + + await act(async () => { + map.addLayerLate(fakeOlLayer("layer-1")); + }); + expect(swapSpy).not.toHaveBeenCalled(); + }); }); test("success: false with empty data falls back to 'Unknown error'", async () => { @@ -806,7 +884,7 @@ describe("useRuntimeLayerFetcher", () => { const { result } = renderHook(() => useRuntimeLayerFetcher({ layers, - gridItemUuid: "g", + gridItemUUID: "g", sessionNonce: "n", mapRef, variableInputValues: {}, @@ -841,7 +919,7 @@ describe("useRuntimeLayerFetcher", () => { const { result } = renderHook(() => useRuntimeLayerFetcher({ layers, - gridItemUuid: "g", + gridItemUUID: "g", sessionNonce: "n", mapRef, variableInputValues: {}, @@ -870,7 +948,7 @@ describe("useRuntimeLayerFetcher", () => { const { result } = renderHook(() => useRuntimeLayerFetcher({ layers, - gridItemUuid: "g", + gridItemUUID: "g", sessionNonce: "n", mapRef, variableInputValues: {}, @@ -910,7 +988,7 @@ describe("useRuntimeLayerFetcher", () => { const { unmount } = renderHook(() => useRuntimeLayerFetcher({ layers, - gridItemUuid: "g", + gridItemUUID: "g", sessionNonce: "n", mapRef, variableInputValues: {}, @@ -955,7 +1033,7 @@ describe("useRuntimeLayerFetcher", () => { const { unmount } = renderHook(() => useRuntimeLayerFetcher({ layers, - gridItemUuid: "g", + gridItemUUID: "g", sessionNonce: "n", mapRef, variableInputValues: {}, @@ -989,7 +1067,7 @@ describe("useRuntimeLayerFetcher", () => { const { result } = renderHook(() => useRuntimeLayerFetcher({ layers, - gridItemUuid: "g", + gridItemUUID: "g", sessionNonce: "n", mapRef, variableInputValues: {}, @@ -1030,7 +1108,7 @@ describe("useRuntimeLayerFetcher", () => { const { result } = renderHook(() => useRuntimeLayerFetcher({ layers, - gridItemUuid: "g", + gridItemUUID: "g", sessionNonce: "n", mapRef: { current: fakeOlMap([]) }, variableInputValues: {}, @@ -1058,7 +1136,7 @@ describe("useRuntimeLayerFetcher", () => { const { result } = renderHook(() => useRuntimeLayerFetcher({ layers: undefined, - gridItemUuid: "g", + gridItemUUID: "g", sessionNonce: "n", mapRef: { current: fakeOlMap([]) }, variableInputValues: {}, @@ -1102,7 +1180,7 @@ describe("useRuntimeLayerFetcher", () => { renderHook(() => useRuntimeLayerFetcher({ layers, - gridItemUuid: "g", + gridItemUUID: "g", sessionNonce: "n", mapRef, // exercises `variableInputValues ?? {}`. @@ -1130,7 +1208,7 @@ describe("useRuntimeLayerFetcher", () => { renderHook(() => useRuntimeLayerFetcher({ layers, - gridItemUuid: "g", + gridItemUUID: "g", sessionNonce: "n", mapRef, variableInputValues: {}, @@ -1159,7 +1237,7 @@ describe("useRuntimeLayerFetcher", () => { ({ layers }) => useRuntimeLayerFetcher({ layers, - gridItemUuid: "g", + gridItemUUID: "g", sessionNonce: "n", mapRef, variableInputValues: {}, diff --git a/reactapp/components/visualizations/runtimeLayerFetcher.js b/reactapp/components/visualizations/runtimeLayerFetcher.js index 67b9fc45..6f16eaa4 100644 --- a/reactapp/components/visualizations/runtimeLayerFetcher.js +++ b/reactapp/components/visualizations/runtimeLayerFetcher.js @@ -5,9 +5,54 @@ import { swapVectorLayerFeatures } from "components/map/utilities"; import appAPI from "services/api/app"; import { valuesEqual } from "components/modals/utilities"; +/** Find the OL layer carrying a runtime layer's identity tag. */ +function findOlLayer(map, layerId) { + return map + .getLayers() + .getArray() + .find((l) => l.get("layerId") === layerId); +} + +/** Drop any listener left waiting for this layer to appear. */ +function cancelPendingSwap(state) { + if (state.pendingSwap) { + state.pendingSwap(); + state.pendingSwap = null; + } +} + +/** + * Hold a fetched FeatureCollection until its OL layer exists on the map. + * + * Map.js constructs layers asynchronously, so the first fetch of a dashboard can + * return before the layer it belongs to has been added. Discarding the payload + * left the layer permanently blank: performFetch records the resolved args + * before requesting, so the reconciliation effect then saw `argsUnchanged` and + * never refetched -- the features only appeared once an argument genuinely + * changed. + */ +function swapWhenLayerAppears( + state, + map, + layerId, + featureCollection, + mapProjection, +) { + const collection = map.getLayers(); + cancelPendingSwap(state); + const onAdd = () => { + const olLayer = findOlLayer(map, layerId); + if (!olLayer) return; + cancelPendingSwap(state); + swapVectorLayerFeatures(olLayer, featureCollection, mapProjection); + }; + state.pendingSwap = () => collection.un("add", onAdd); + collection.on("add", onAdd); +} + export default function useRuntimeLayerFetcher({ layers, - gridItemUuid, + gridItemUUID, sessionNonce, mapRef, variableInputValues, @@ -32,6 +77,7 @@ export default function useRuntimeLayerFetcher({ if (state.cancelTokenSource) { state.cancelTokenSource.cancel("unmount"); } + cancelPendingSwap(state); }); stateMap.clear(); }; @@ -76,11 +122,13 @@ export default function useRuntimeLayerFetcher({ if (state.cancelTokenSource) { state.cancelTokenSource.cancel("superseded"); } + // A newer fetch replaces whatever an older one was still waiting to paint. + cancelPendingSwap(state); const cancelTokenSource = axios.CancelToken.source(); state.cancelTokenSource = cancelTokenSource; state.lastResolvedArgs = resolvedArgs; - const requestId = `${sessionNonce}:${gridItemUuid}:${layerId}`; + const requestId = `${sessionNonce}:${gridItemUUID}:${layerId}`; return appAPI .getVisualizationFeatures({ @@ -106,16 +154,23 @@ export default function useRuntimeLayerFetcher({ onBeforeSwap(layerId); } const map = mapRef?.current; - if (!map) return; - const olLayer = map - .getLayers() - .getArray() - .find((l) => l.get("layerId") === layerId); + if (!map) { + // No map to paint into yet. Forget the args so the next + // reconciliation refetches rather than treating this as done. + state.lastResolvedArgs = undefined; + return; + } + const featureCollection = response?.data ?? null; + const mapProjection = map.getView().getProjection().getCode(); + const olLayer = findOlLayer(map, layerId); if (olLayer) { - const mapProjection = map.getView().getProjection().getCode(); - swapVectorLayerFeatures( - olLayer, - response?.data ?? null, + swapVectorLayerFeatures(olLayer, featureCollection, mapProjection); + } else { + swapWhenLayerAppears( + state, + map, + layerId, + featureCollection, mapProjection, ); } @@ -130,7 +185,7 @@ export default function useRuntimeLayerFetcher({ }); }); }, - [sessionNonce, gridItemUuid, mapRef, onBeforeSwap, setError, clearError], + [sessionNonce, gridItemUUID, mapRef, onBeforeSwap, setError, clearError], ); const scheduleFetch = useCallback( @@ -163,6 +218,7 @@ export default function useRuntimeLayerFetcher({ cancelTokenSource: null, debounceTimer: null, lastResolvedArgs: undefined, + pendingSwap: null, }); } const state = perLayerStateRef.current.get(layerId); @@ -196,6 +252,7 @@ export default function useRuntimeLayerFetcher({ if (!currentLayerIds.has(layerId)) { if (state.debounceTimer) clearTimeout(state.debounceTimer); if (state.cancelTokenSource) state.cancelTokenSource.cancel("removed"); + cancelPendingSwap(state); perLayerStateRef.current.delete(layerId); } }); @@ -209,6 +266,7 @@ export default function useRuntimeLayerFetcher({ cancelTokenSource: null, debounceTimer: null, lastResolvedArgs: undefined, + pendingSwap: null, }); // First appearance — always fetch (subject to debounce). scheduleFetch(layerId, pluginSource, resolvedArgs); From ac55069147ad523548dba7f05085b83676431486 Mon Sep 17 00:00:00 2001 From: Corey Krewson Date: Mon, 17 Aug 2026 11:32:08 -0700 Subject: [PATCH 4/5] Add ten continuous color ramps for GeoTIFF and Zarr layers Raster layers had four ramps: viridis, turbo, RdYlBu and grayscale. That left no single-hue sequential option for depth or precipitation, and only one diverging option for anomaly data. Adds the rest of the perceptually uniform family (magma, inferno, plasma, cividis), three ColorBrewer sequential maps (Blues, YlGnBu, YlOrRd), and three more diverging maps (RdBu, Spectral, BrBG) -- fourteen in total. Keystops were sampled from matplotlib 3.10 at twelve evenly spaced points, the convention viridis and turbo already used, rather than transcribed by hand. The default stays turbo, so existing layers are unaffected. Fourteen unlabelled swatches in a flat column would be worse than four, so the picker now groups them under Perceptually uniform / Sequential / Diverging, names each row, and scrolls within its own box instead of pushing the rest of the Style tab off-screen. RAMP_GROUPS is the single source of order and RAMP_NAMES derives from it, so a ramp cannot be added to the registry and left unreachable in the UI. RAMP_STOPS is unchanged at 32: it bounds the stops in one layer's shader expression, which does not grow with the number of ramps on offer. Tests now iterate the registry rather than a hardcoded list of four, and assert the properties that a mis-sampled table breaks: the perceptually uniform ramps rise monotonically in luminance, the diverging ramps are lightest at their midpoint, and the groups cover exactly the registered names with no duplicates. Co-Authored-By: Claude Opus 5 --- .../components/map/colorRamps.test.js | 74 ++++++- .../modals/MapLayer/RampPicker.test.js | 31 ++- reactapp/components/map/colorRamps.js | 196 +++++++++++++++++- .../components/modals/MapLayer/RampPicker.js | 78 ++++--- 4 files changed, 348 insertions(+), 31 deletions(-) diff --git a/reactapp/__tests__/components/map/colorRamps.test.js b/reactapp/__tests__/components/map/colorRamps.test.js index c26f2958..24cfe31c 100644 --- a/reactapp/__tests__/components/map/colorRamps.test.js +++ b/reactapp/__tests__/components/map/colorRamps.test.js @@ -1,12 +1,13 @@ import { COLOR_RAMPS, + RAMP_GROUPS, RAMP_NAMES, RAMP_STOPS, _internal, } from "components/map/colorRamps"; const HEX_RE = /^#[0-9a-fA-F]{6}$/; -const RAMP_KEYS = ["viridis", "turbo", "RdYlBu", "grayscale"]; +const RAMP_KEYS = RAMP_NAMES; describe("COLOR_RAMPS", () => { test.each(RAMP_KEYS)( @@ -36,10 +37,77 @@ describe("COLOR_RAMPS", () => { }, ); - test("RAMP_NAMES exposes the four canonical names in order", () => { - expect(RAMP_NAMES).toEqual(["viridis", "turbo", "RdYlBu", "grayscale"]); + test("RAMP_NAMES is the groups flattened, in display order", () => { + expect(RAMP_NAMES).toEqual([ + "viridis", + "magma", + "inferno", + "plasma", + "cividis", + "turbo", + "Blues", + "YlGnBu", + "YlOrRd", + "grayscale", + "RdYlBu", + "RdBu", + "Spectral", + "BrBG", + ]); + }); + + test("groups and COLOR_RAMPS cover exactly the same names", () => { + // A ramp defined but never grouped is unreachable in the picker; a grouped + // name with no ramp renders an empty gradient. + expect([...RAMP_NAMES].sort()).toEqual(Object.keys(COLOR_RAMPS).sort()); + }); + + test("no ramp is listed in two groups", () => { + expect(new Set(RAMP_NAMES).size).toBe(RAMP_NAMES.length); }); + test("every group has a label and at least one ramp", () => { + for (const group of RAMP_GROUPS) { + expect(typeof group.label).toBe("string"); + expect(group.label.length).toBeGreaterThan(0); + expect(group.names.length).toBeGreaterThan(0); + } + }); + + // Relative luminance (Rec. 709). Used below to assert the shape of each + // family, which is what a mis-sampled or mis-pasted colormap table breaks. + const luminance = (hex) => { + const n = parseInt(hex.slice(1), 16); + const r = ((n >> 16) & 0xff) / 255; + const g = ((n >> 8) & 0xff) / 255; + const b = (n & 0xff) / 255; + return 0.2126 * r + 0.7152 * g + 0.0722 * b; + }; + + test.each(["viridis", "magma", "inferno", "plasma", "cividis"])( + "%s increases in luminance from end to end", + (rampName) => { + // The defining property of this family: lightness rises monotonically, so + // the ramp reads as an ordered scale even in greyscale print. + const lums = COLOR_RAMPS[rampName].map(luminance); + for (let i = 1; i < lums.length; i++) { + expect(lums[i]).toBeGreaterThan(lums[i - 1]); + } + }, + ); + + test.each(["RdYlBu", "RdBu", "Spectral", "BrBG"])( + "%s is lightest at its midpoint", + (rampName) => { + // Diverging maps pivot through a pale neutral; both ends must be darker + // than the centre or the midpoint stops reading as the neutral value. + const ramp = COLOR_RAMPS[rampName]; + const mid = luminance(ramp[Math.floor(ramp.length / 2)]); + expect(mid).toBeGreaterThan(luminance(ramp[0])); + expect(mid).toBeGreaterThan(luminance(ramp[ramp.length - 1])); + }, + ); + test("grayscale starts black and ends white", () => { expect(COLOR_RAMPS.grayscale[0]).toBe("#000000"); expect(COLOR_RAMPS.grayscale[RAMP_STOPS - 1]).toBe("#ffffff"); diff --git a/reactapp/__tests__/components/modals/MapLayer/RampPicker.test.js b/reactapp/__tests__/components/modals/MapLayer/RampPicker.test.js index fde5b056..f9993549 100644 --- a/reactapp/__tests__/components/modals/MapLayer/RampPicker.test.js +++ b/reactapp/__tests__/components/modals/MapLayer/RampPicker.test.js @@ -2,10 +2,10 @@ import { render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import RampPicker from "components/modals/MapLayer/RampPicker"; -const RAMP_NAMES = ["viridis", "turbo", "RdYlBu", "grayscale"]; +import { RAMP_GROUPS, RAMP_NAMES } from "components/map/colorRamps"; describe("RampPicker", () => { - test("renders all four ramp options by name", () => { + test("renders every registered ramp option by name", () => { render( {}} />); for (const name of RAMP_NAMES) { @@ -15,6 +15,33 @@ describe("RampPicker", () => { } }); + test("shows each group's heading and its ramps' names as text", () => { + // With fourteen ramps the picker is no longer self-explanatory from + // swatches alone, so each row is labelled and grouped by family. + render( {}} />); + + for (const group of RAMP_GROUPS) { + expect(screen.getByText(group.label)).toBeInTheDocument(); + for (const name of group.names) { + expect(screen.getByTestId(`ramp-option-${name}`)).toHaveTextContent( + name, + ); + } + } + }); + + test("every ramp is reachable and selectable", async () => { + const onChange = jest.fn(); + render(); + + expect(screen.getAllByRole("radio")).toHaveLength(RAMP_NAMES.length); + for (const name of RAMP_NAMES) { + onChange.mockClear(); + await userEvent.click(screen.getByTestId(`ramp-option-${name}`)); + expect(onChange).toHaveBeenCalledWith(name); + } + }); + test("each option has a gradient swatch element", () => { render( {}} />); diff --git a/reactapp/components/map/colorRamps.js b/reactapp/components/map/colorRamps.js index 119c81c2..08329a9f 100644 --- a/reactapp/components/map/colorRamps.js +++ b/reactapp/components/map/colorRamps.js @@ -102,19 +102,211 @@ const GRAYSCALE_KEYSTOPS = toKeystops([ [1.0, [1.0, 1.0, 1.0]], ]); +// The ramps below were sampled from matplotlib 3.10 at 12 evenly spaced points, +// the same convention as viridis and turbo above, rather than transcribed by +// hand. matplotlib's Blues/YlGnBu/YlOrRd/RdBu/Spectral/BrBG are the ColorBrewer +// maps of those names. + +// magma, inferno, plasma, cividis — the perceptually uniform family that ships +// alongside viridis. cividis is additionally optimized for red-green color +// vision deficiency. +const MAGMA_KEYSTOPS = toKeystops([ + [0.0, [0.001462, 0.000466, 0.013866]], + [0.0909, [0.069764, 0.049726, 0.193735]], + [0.1818, [0.198177, 0.063862, 0.404009]], + [0.2727, [0.347636, 0.082946, 0.494121]], + [0.3636, [0.494258, 0.141462, 0.507988]], + [0.4545, [0.639216, 0.189921, 0.49415]], + [0.5455, [0.786212, 0.241514, 0.450184]], + [0.6364, [0.913354, 0.330052, 0.382563]], + [0.7273, [0.979645, 0.491014, 0.367783]], + [0.8182, [0.996341, 0.660969, 0.45116]], + [0.9091, [0.995131, 0.827052, 0.585701]], + [1.0, [0.987053, 0.991438, 0.749504]], +]); + +const INFERNO_KEYSTOPS = toKeystops([ + [0.0, [0.001462, 0.000466, 0.013866]], + [0.0909, [0.076637, 0.041905, 0.205799]], + [0.1818, [0.224763, 0.036405, 0.388129]], + [0.2727, [0.372768, 0.073915, 0.4324]], + [0.3636, [0.522206, 0.12815, 0.419549]], + [0.4545, [0.66454, 0.181539, 0.369846]], + [0.5455, [0.796607, 0.254728, 0.287264]], + [0.6364, [0.902003, 0.364492, 0.184116]], + [0.7273, [0.969163, 0.515946, 0.063488]], + [0.8182, [0.987714, 0.682807, 0.072489]], + [0.9091, [0.960626, 0.859069, 0.29801]], + [1.0, [0.988362, 0.998364, 0.644924]], +]); + +const PLASMA_KEYSTOPS = toKeystops([ + [0.0, [0.050383, 0.029803, 0.527975]], + [0.0909, [0.241396, 0.014979, 0.610259]], + [0.1818, [0.387183, 0.001434, 0.654177]], + [0.2727, [0.523633, 0.024532, 0.652901]], + [0.3636, [0.650746, 0.125309, 0.595617]], + [0.4545, [0.752312, 0.227133, 0.513149]], + [0.5455, [0.836801, 0.329105, 0.430905]], + [0.6364, [0.907365, 0.434524, 0.35297]], + [0.7273, [0.963203, 0.553865, 0.271909]], + [0.8182, [0.991985, 0.681179, 0.195295]], + [0.9091, [0.986509, 0.822401, 0.143557]], + [1.0, [0.940015, 0.975158, 0.131326]], +]); + +const CIVIDIS_KEYSTOPS = toKeystops([ + [0.0, [0.0, 0.135112, 0.304751]], + [0.0909, [0.003602, 0.195911, 0.441564]], + [0.1818, [0.185453, 0.258914, 0.426788]], + [0.2727, [0.28324, 0.32139, 0.423211]], + [0.3636, [0.37043, 0.38689, 0.433428]], + [0.4545, [0.448447, 0.451053, 0.456264]], + [0.5455, [0.529086, 0.517207, 0.472543]], + [0.6364, [0.616852, 0.585913, 0.462237]], + [0.7273, [0.712105, 0.66116, 0.434117]], + [0.8182, [0.806859, 0.737385, 0.387684]], + [0.9091, [0.905589, 0.818257, 0.312889]], + [1.0, [0.995737, 0.909344, 0.217772]], +]); + +// Single- and multi-hue sequential maps. Blues suits depth and water extent, +// YlGnBu precipitation, YlOrRd heat and risk. +const BLUES_KEYSTOPS = toKeystops([ + [0.0, [0.968627, 0.984314, 1.0]], + [0.0909, [0.897885, 0.939039, 0.977363]], + [0.1818, [0.828881, 0.893764, 0.954725]], + [0.2727, [0.750634, 0.847843, 0.928212]], + [0.3636, [0.632526, 0.797647, 0.886874]], + [0.4545, [0.491765, 0.721968, 0.854779]], + [0.5455, [0.361599, 0.642737, 0.816578]], + [0.6364, [0.248166, 0.561892, 0.77098]], + [0.7273, [0.150727, 0.464452, 0.720784]], + [0.8182, [0.074817, 0.373256, 0.65521]], + [0.9091, [0.031373, 0.281615, 0.558262]], + [1.0, [0.031373, 0.188235, 0.419608]], +]); + +const YL_GN_BU_KEYSTOPS = toKeystops([ + [0.0, [1.0, 1.0, 0.85098]], + [0.0909, [0.949066, 0.980192, 0.737793]], + [0.1818, [0.863376, 0.946482, 0.699331]], + [0.2727, [0.733887, 0.89564, 0.710404]], + [0.3636, [0.521292, 0.812964, 0.731073]], + [0.4545, [0.342622, 0.746267, 0.755894]], + [0.5455, [0.203968, 0.661376, 0.762968]], + [0.6364, [0.11534, 0.552157, 0.74519]], + [0.7273, [0.130104, 0.401569, 0.674325]], + [0.8182, [0.139885, 0.276909, 0.615148]], + [0.9091, [0.113433, 0.178808, 0.514879]], + [1.0, [0.031373, 0.113725, 0.345098]], +]); + +const YL_OR_RD_KEYSTOPS = toKeystops([ + [0.0, [1.0, 1.0, 0.8]], + [0.0909, [1.0, 0.949066, 0.675494]], + [0.1818, [0.998262, 0.894656, 0.554464]], + [0.2727, [0.996078, 0.82579, 0.435617]], + [0.3636, [0.996078, 0.710634, 0.311603]], + [0.4545, [0.993572, 0.60529, 0.257932]], + [0.5455, [0.990742, 0.463806, 0.209827]], + [0.6364, [0.980161, 0.289089, 0.160185]], + [0.7273, [0.906344, 0.135548, 0.118847]], + [0.8182, [0.807213, 0.045183, 0.131642]], + [0.9091, [0.674571, 0.0, 0.14902]], + [1.0, [0.501961, 0.0, 0.14902]], +]); + +// Diverging maps, for values read against a meaningful midpoint -- anomalies, +// differences, change between two dates. +const RD_BU_KEYSTOPS = toKeystops([ + [0.0, [0.403922, 0.0, 0.121569]], + [0.0909, [0.669204, 0.08489, 0.164014]], + [0.1818, [0.811534, 0.321107, 0.275817]], + [0.2727, [0.922261, 0.567474, 0.448674]], + [0.3636, [0.9797, 0.784083, 0.68489]], + [0.4545, [0.979239, 0.919108, 0.883737]], + [0.5455, [0.901423, 0.936794, 0.956248]], + [0.6364, [0.732411, 0.853749, 0.916263]], + [0.7273, [0.48143, 0.714879, 0.839446]], + [0.8182, [0.236601, 0.541869, 0.74702]], + [0.9091, [0.118647, 0.379239, 0.645675]], + [1.0, [0.019608, 0.188235, 0.380392]], +]); + +const SPECTRAL_KEYSTOPS = toKeystops([ + [0.0, [0.619608, 0.003922, 0.258824]], + [0.0909, [0.814148, 0.219685, 0.304806]], + [0.1818, [0.933026, 0.391311, 0.271972]], + [0.2727, [0.981776, 0.607382, 0.34579]], + [0.3636, [0.994694, 0.809227, 0.486967]], + [0.4545, [0.998231, 0.945175, 0.657055]], + [0.5455, [0.955786, 0.982314, 0.680046]], + [0.6364, [0.8203, 0.927566, 0.612687]], + [0.7273, [0.591003, 0.835525, 0.644291]], + [0.8182, [0.360015, 0.716186, 0.665513]], + [0.9091, [0.212995, 0.511419, 0.730796]], + [1.0, [0.368627, 0.309804, 0.635294]], +]); + +const BR_BG_KEYSTOPS = toKeystops([ + [0.0, [0.329412, 0.188235, 0.019608]], + [0.0909, [0.527489, 0.30496, 0.037293]], + [0.1818, [0.709804, 0.468973, 0.149558]], + [0.2727, [0.837601, 0.685813, 0.397924]], + [0.3636, [0.932872, 0.857209, 0.66782]], + [0.4545, [0.962553, 0.937793, 0.872357]], + [0.5455, [0.879431, 0.94133, 0.932488]], + [0.6364, [0.682122, 0.877509, 0.848212]], + [0.7273, [0.415456, 0.741638, 0.699193]], + [0.8182, [0.167859, 0.554479, 0.523106]], + [0.9091, [0.003537, 0.383852, 0.350942]], + [1.0, [0.0, 0.235294, 0.188235]], +]); + // Shader-friendly stop count — see file-level comment for the WebGL // fragment-shader instruction-limit constraint. export const RAMP_STOPS = 32; export const COLOR_RAMPS = { viridis: interpolateRamp(VIRIDIS_KEYSTOPS, RAMP_STOPS), + magma: interpolateRamp(MAGMA_KEYSTOPS, RAMP_STOPS), + inferno: interpolateRamp(INFERNO_KEYSTOPS, RAMP_STOPS), + plasma: interpolateRamp(PLASMA_KEYSTOPS, RAMP_STOPS), + cividis: interpolateRamp(CIVIDIS_KEYSTOPS, RAMP_STOPS), turbo: interpolateRamp(TURBO_KEYSTOPS, RAMP_STOPS), - RdYlBu: interpolateRamp(RD_YL_BU_KEYSTOPS, RAMP_STOPS), + Blues: interpolateRamp(BLUES_KEYSTOPS, RAMP_STOPS), + YlGnBu: interpolateRamp(YL_GN_BU_KEYSTOPS, RAMP_STOPS), + YlOrRd: interpolateRamp(YL_OR_RD_KEYSTOPS, RAMP_STOPS), grayscale: interpolateRamp(GRAYSCALE_KEYSTOPS, RAMP_STOPS), + RdYlBu: interpolateRamp(RD_YL_BU_KEYSTOPS, RAMP_STOPS), + RdBu: interpolateRamp(RD_BU_KEYSTOPS, RAMP_STOPS), + Spectral: interpolateRamp(SPECTRAL_KEYSTOPS, RAMP_STOPS), + BrBG: interpolateRamp(BR_BG_KEYSTOPS, RAMP_STOPS), }; +// Grouped for the picker, which would otherwise be an unlabelled column of +// swatches. Sequential maps read low-to-high; diverging maps read against a +// midpoint and are wrong for data that has no meaningful centre. +export const RAMP_GROUPS = [ + { + label: "Perceptually uniform", + names: ["viridis", "magma", "inferno", "plasma", "cividis"], + }, + { + // turbo is a rainbow rather than perceptually uniform -- high contrast and + // popular, but it invents edges that are not in the data. + label: "Sequential", + names: ["turbo", "Blues", "YlGnBu", "YlOrRd", "grayscale"], + }, + { + label: "Diverging", + names: ["RdYlBu", "RdBu", "Spectral", "BrBG"], + }, +]; + // Canonical display order in the picker UI. -export const RAMP_NAMES = ["viridis", "turbo", "RdYlBu", "grayscale"]; +export const RAMP_NAMES = RAMP_GROUPS.flatMap((group) => group.names); // Exported for unit testing. export const _internal = { interpolateRamp, rgbToHex, hexToRgb01 }; diff --git a/reactapp/components/modals/MapLayer/RampPicker.js b/reactapp/components/modals/MapLayer/RampPicker.js index 42f2651d..e8746daa 100644 --- a/reactapp/components/modals/MapLayer/RampPicker.js +++ b/reactapp/components/modals/MapLayer/RampPicker.js @@ -1,12 +1,36 @@ +import { Fragment } from "react"; import PropTypes from "prop-types"; import styled from "styled-components"; -import { COLOR_RAMPS, RAMP_NAMES } from "components/map/colorRamps"; +import { COLOR_RAMPS, RAMP_GROUPS } from "components/map/colorRamps"; const PickerList = styled.div` display: flex; flex-direction: column; gap: 6px; margin-bottom: 12px; + /* Enough ramps now that the list needs its own scroll rather than pushing the + rest of the Style tab off-screen. */ + max-height: 320px; + overflow-y: auto; +`; + +const GroupLabel = styled.div` + font-size: 0.75rem; + font-weight: 600; + text-transform: uppercase; + letter-spacing: 0.04em; + color: #6c757d; + margin: 4px 0 0 2px; + + &:first-child { + margin-top: 0; + } +`; + +const RampLabel = styled.span` + flex: 0 0 5.5rem; + font-size: 0.8rem; + color: #212529; `; const RampRow = styled.button` @@ -47,29 +71,35 @@ const buildGradient = (colors) => const RampPicker = ({ selectedRamp, onChange }) => { return ( - {RAMP_NAMES.map((name) => { - const colors = COLOR_RAMPS[name]; - const isSelected = selectedRamp === name; - return ( - onChange(name)} - > - - ); - })} + {RAMP_GROUPS.map((group) => ( + + + {group.names.map((name) => { + const colors = COLOR_RAMPS[name]; + const isSelected = selectedRamp === name; + return ( + onChange(name)} + > + {name} + + ); + })} + + ))} ); }; From 037226902319979edcbd56b932ebe54f3537620c Mon Sep 17 00:00:00 2001 From: Corey Krewson Date: Mon, 17 Aug 2026 11:45:42 -0700 Subject: [PATCH 5/5] Add a reverse option for raster color ramps, drop the picker row names Reversing a ramp is the difference between reading a Blues layer as depth and reading it as elevation, and it was previously only possible by authoring a style expression by hand. Adds a `rampReverse` flag to GeoTIFF and Zarr sources, toggled by a "Reverse ramp" checkbox under the picker. Reversal happens in one place -- resolveRamp in colorRamps.js -- so the five consumers cannot disagree about which end is which: the style expression, its render-time rebuild in ModuleLoader, the save path in MapLayer, the editor's swatches, and the map legend's gradient. The picker previews the reversed direction, so what is selected matches what the map draws. resolveRamp returns the shared array untouched when unreversed and a copy when reversed; an in-place reverse would corrupt the ramp for every other consumer. The flag is persisted only when true, so an unreversed layer's saved config is byte-identical to before this option existed, and it is hidden in categorical mode, where a discrete class list has no direction to flip. Also removes the per-row ramp names from the picker. They were added with the grouping in the previous commit and were not there before; the group headings carry the organisation on their own, and the names are still exposed to assistive tech through each row's aria-label. Co-Authored-By: Claude Opus 5 --- .../components/map/colorRamps.test.js | 34 ++++++++++ .../components/map/geoTIFFStyle.test.js | 65 +++++++++++++++++++ .../modals/MapLayer/RampPicker.test.js | 11 ++-- .../modals/MapLayer/StylePane.test.js | 39 +++++++++++ reactapp/components/map/ModuleLoader.js | 1 + reactapp/components/map/colorRamps.js | 15 +++++ reactapp/components/map/geoTIFFStyle.js | 7 +- .../components/modals/MapLayer/MapLayer.js | 18 ++++- .../components/modals/MapLayer/RampPicker.js | 16 ++--- .../components/modals/MapLayer/StylePane.js | 31 ++++++++- reactapp/components/visualizations/Map.js | 7 +- 11 files changed, 219 insertions(+), 25 deletions(-) diff --git a/reactapp/__tests__/components/map/colorRamps.test.js b/reactapp/__tests__/components/map/colorRamps.test.js index 24cfe31c..3fa467be 100644 --- a/reactapp/__tests__/components/map/colorRamps.test.js +++ b/reactapp/__tests__/components/map/colorRamps.test.js @@ -3,6 +3,7 @@ import { RAMP_GROUPS, RAMP_NAMES, RAMP_STOPS, + resolveRamp, _internal, } from "components/map/colorRamps"; @@ -108,6 +109,39 @@ describe("COLOR_RAMPS", () => { }, ); + describe("resolveRamp", () => { + test.each(RAMP_NAMES)("%s unreversed is the registered array", (name) => { + expect(resolveRamp(name, false)).toBe(COLOR_RAMPS[name]); + expect(resolveRamp(name)).toBe(COLOR_RAMPS[name]); + }); + + test.each(RAMP_NAMES)("%s reversed is end-to-end flipped", (name) => { + const forward = COLOR_RAMPS[name]; + const reversed = resolveRamp(name, true); + expect(reversed).toHaveLength(forward.length); + expect(reversed[0]).toBe(forward[forward.length - 1]); + expect(reversed[reversed.length - 1]).toBe(forward[0]); + }); + + test("reversing does not mutate the registered ramp", () => { + // resolveRamp returns the shared array when unreversed, so an in-place + // reverse would corrupt every other consumer of that ramp. + const before = [...COLOR_RAMPS.viridis]; + resolveRamp("viridis", true); + expect(COLOR_RAMPS.viridis).toEqual(before); + }); + + test("reversing twice returns to the original order", () => { + const once = resolveRamp("magma", true); + expect([...once].reverse()).toEqual(COLOR_RAMPS.magma); + }); + + test("an unknown ramp resolves to undefined either way", () => { + expect(resolveRamp("nope")).toBeUndefined(); + expect(resolveRamp("nope", true)).toBeUndefined(); + }); + }); + test("grayscale starts black and ends white", () => { expect(COLOR_RAMPS.grayscale[0]).toBe("#000000"); expect(COLOR_RAMPS.grayscale[RAMP_STOPS - 1]).toBe("#ffffff"); diff --git a/reactapp/__tests__/components/map/geoTIFFStyle.test.js b/reactapp/__tests__/components/map/geoTIFFStyle.test.js index 5fa475b8..f6f8cf3e 100644 --- a/reactapp/__tests__/components/map/geoTIFFStyle.test.js +++ b/reactapp/__tests__/components/map/geoTIFFStyle.test.js @@ -27,6 +27,71 @@ describe("buildGeoTIFFStyleColor", () => { expect(expr).toHaveLength(3 + RAMP_STOPS * 2); }); + describe("rampReverse", () => { + const stopsOf = (expr) => { + // Strip the 3-element operator header, then take every other entry. + const body = expr.slice(3); + return body.filter((_, i) => i % 2 === 1); + }; + + test("flips the colors while leaving the value stops in place", () => { + const forward = buildGeoTIFFStyleColor({ + rampName: "viridis", + rampMin: 0, + rampMax: 100, + }); + const reversed = buildGeoTIFFStyleColor({ + rampName: "viridis", + rampMin: 0, + rampMax: 100, + rampReverse: true, + }); + + // Same length and same numeric breakpoints -- only the palette turns around. + expect(reversed).toHaveLength(forward.length); + const values = (expr) => expr.slice(3).filter((_, i) => i % 2 === 0); + expect(values(reversed)).toEqual(values(forward)); + expect(stopsOf(reversed)).toEqual([...stopsOf(forward)].reverse()); + }); + + test("the low end of the range takes the ramp's last color", () => { + const reversed = buildGeoTIFFStyleColor({ + rampName: "viridis", + rampMin: 0, + rampMax: 100, + rampReverse: true, + }); + expect(reversed[3]).toBe(0); + expect(reversed[4]).toBe( + COLOR_RAMPS.viridis[COLOR_RAMPS.viridis.length - 1], + ); + expect(reversed[reversed.length - 1]).toBe(COLOR_RAMPS.viridis[0]); + }); + + test("omitting rampReverse matches passing false", () => { + const args = { rampName: "turbo", rampMin: -5, rampMax: 5 }; + expect(buildGeoTIFFStyleColor(args)).toEqual( + buildGeoTIFFStyleColor({ ...args, rampReverse: false }), + ); + }); + + test("reversing survives the transparency guards being prepended", () => { + const reversed = buildGeoTIFFStyleColor({ + rampName: "Blues", + rampMin: 0, + rampMax: 1, + rampReverse: true, + hasNodata: true, + }); + expect(reversed[0]).toBe("case"); + const interpolateExpr = reversed[reversed.length - 1]; + expect(interpolateExpr[0]).toBe("interpolate"); + expect(interpolateExpr[4]).toBe( + COLOR_RAMPS.Blues[COLOR_RAMPS.Blues.length - 1], + ); + }); + }); + test("starts with the first ramp color and ends with the last", () => { const expr = buildGeoTIFFStyleColor({ rampName: "viridis", diff --git a/reactapp/__tests__/components/modals/MapLayer/RampPicker.test.js b/reactapp/__tests__/components/modals/MapLayer/RampPicker.test.js index f9993549..b5988633 100644 --- a/reactapp/__tests__/components/modals/MapLayer/RampPicker.test.js +++ b/reactapp/__tests__/components/modals/MapLayer/RampPicker.test.js @@ -15,17 +15,16 @@ describe("RampPicker", () => { } }); - test("shows each group's heading and its ramps' names as text", () => { - // With fourteen ramps the picker is no longer self-explanatory from - // swatches alone, so each row is labelled and grouped by family. + test("shows each group's heading", () => { + // Fourteen swatches need family headings to stay navigable. The rows + // themselves carry no visible text -- the swatch is the label, with the + // ramp name exposed only to assistive tech. render( {}} />); for (const group of RAMP_GROUPS) { expect(screen.getByText(group.label)).toBeInTheDocument(); for (const name of group.names) { - expect(screen.getByTestId(`ramp-option-${name}`)).toHaveTextContent( - name, - ); + expect(screen.getByTestId(`ramp-option-${name}`)).toHaveTextContent(""); } } }); diff --git a/reactapp/__tests__/components/modals/MapLayer/StylePane.test.js b/reactapp/__tests__/components/modals/MapLayer/StylePane.test.js index 0bd58925..ffef912b 100644 --- a/reactapp/__tests__/components/modals/MapLayer/StylePane.test.js +++ b/reactapp/__tests__/components/modals/MapLayer/StylePane.test.js @@ -118,6 +118,9 @@ const GeoTIFFTestHarness = ({ initialSourceProps, sourcePropsSpy }) => {

{sourceProps.rampName ?? ""}

{sourceProps.rampMin ?? ""}

{sourceProps.rampMax ?? ""}

+

+ {String(sourceProps.rampReverse ?? false)} +

); @@ -157,6 +160,42 @@ test("StylePane GeoTIFF ramp/min/max handlers no-op when setSourceProps is missi expect(() => fireEvent.change(maxInput, { target: { value: "100" } }), ).not.toThrow(); + + // Reverse checkbox → handleReverseToggle short-circuits too. + const reverse = screen.getByLabelText("Reverse Color Ramp"); + expect(() => fireEvent.click(reverse)).not.toThrow(); +}); + +test("StylePane reverse checkbox toggles sourceProps.rampReverse", async () => { + render(); + + const reverse = await screen.findByLabelText("Reverse Color Ramp"); + expect(reverse).not.toBeChecked(); + expect(screen.getByTestId("rampReverse")).toHaveTextContent("false"); + + await userEvent.click(reverse); + expect(screen.getByTestId("rampReverse")).toHaveTextContent("true"); + expect(await screen.findByLabelText("Reverse Color Ramp")).toBeChecked(); + + await userEvent.click(screen.getByLabelText("Reverse Color Ramp")); + expect(screen.getByTestId("rampReverse")).toHaveTextContent("false"); +}); + +test("StylePane hides the reverse checkbox in categorical mode", async () => { + // A discrete class list has no ramp direction to flip. + render( + , + ); + + expect(await screen.findByText("Classes")).toBeInTheDocument(); + expect(screen.queryByLabelText("Reverse Color Ramp")).not.toBeInTheDocument(); }); test("StylePane json Input", async () => { diff --git a/reactapp/components/map/ModuleLoader.js b/reactapp/components/map/ModuleLoader.js index 1015f2ff..b34c5c14 100644 --- a/reactapp/components/map/ModuleLoader.js +++ b/reactapp/components/map/ModuleLoader.js @@ -224,6 +224,7 @@ export async function applyAutoRamp(layerConfig) { rampName, rampMin: rampMinValue, rampMax: rampMaxValue, + rampReverse: source.rampReverse === true, hasNodata: true, maskBelow: source.props?.mask_below, }), diff --git a/reactapp/components/map/colorRamps.js b/reactapp/components/map/colorRamps.js index 08329a9f..8fcf3398 100644 --- a/reactapp/components/map/colorRamps.js +++ b/reactapp/components/map/colorRamps.js @@ -308,5 +308,20 @@ export const RAMP_GROUPS = [ // Canonical display order in the picker UI. export const RAMP_NAMES = RAMP_GROUPS.flatMap((group) => group.names); +/** + * A ramp's colors, optionally reversed. + * + * The single place reversal happens, so the raster style, the editor preview and + * the map legend cannot disagree about which end is which. Returns a copy when + * reversed and the shared array otherwise, so callers must not mutate it. + * + * Returns undefined for an unknown name; callers decide whether that throws. + */ +export function resolveRamp(rampName, reverse = false) { + const colors = COLOR_RAMPS[rampName]; + if (!colors) return undefined; + return reverse ? [...colors].reverse() : colors; +} + // Exported for unit testing. export const _internal = { interpolateRamp, rgbToHex, hexToRgb01 }; diff --git a/reactapp/components/map/geoTIFFStyle.js b/reactapp/components/map/geoTIFFStyle.js index 4c959ea5..022320ca 100644 --- a/reactapp/components/map/geoTIFFStyle.js +++ b/reactapp/components/map/geoTIFFStyle.js @@ -1,4 +1,4 @@ -import { COLOR_RAMPS } from "./colorRamps"; +import { resolveRamp } from "./colorRamps"; import PropTypes from "prop-types"; const TRANSPARENT = [0, 0, 0, 0]; @@ -83,10 +83,11 @@ export function buildGeoTIFFStyleColor({ rampName, rampMin, rampMax, + rampReverse = false, hasNodata = false, maskBelow, }) { - const colors = COLOR_RAMPS[rampName]; + const colors = resolveRamp(rampName, rampReverse); if (!colors) { throw new Error(`Unknown color ramp: ${rampName}`); } @@ -134,6 +135,8 @@ buildGeoTIFFStyleColor.propTypes = { rampName: PropTypes.string.isRequired, rampMin: PropTypes.oneOfType([PropTypes.number, PropTypes.string]), rampMax: PropTypes.oneOfType([PropTypes.number, PropTypes.string]), + // Flip the ramp so its last color lands on the low end of the range. + rampReverse: PropTypes.bool, hasNodata: PropTypes.bool, // Cells at or below this raw value render transparent. maskBelow: PropTypes.oneOfType([PropTypes.number, PropTypes.string]), diff --git a/reactapp/components/modals/MapLayer/MapLayer.js b/reactapp/components/modals/MapLayer/MapLayer.js index d5fcb2af..54664b42 100644 --- a/reactapp/components/modals/MapLayer/MapLayer.js +++ b/reactapp/components/modals/MapLayer/MapLayer.js @@ -374,8 +374,15 @@ const MapLayerModal = ({ } if (sourceProps.type === "GeoTIFF" || sourceProps.type === "Zarr") { - const { rampName, rampMin, rampMax, styleMode, classes, fallbackColor } = - sourceProps; + const { + rampName, + rampMin, + rampMax, + rampReverse, + styleMode, + classes, + fallbackColor, + } = sourceProps; const hasRampName = typeof rampName === "string" && rampName.trim() !== ""; @@ -404,6 +411,7 @@ const MapLayerModal = ({ if (fallbackColor) savedSource.fallbackColor = fallbackColor; // Kept so switching back to a ramp does not lose the chosen palette. if (hasRampName) savedSource.rampName = rampName; + if (rampReverse === true) savedSource.rampReverse = true; } // Each bound is independent: a set one pins that end of the ramp, an // empty one is resolved from the file's statistics at render time. @@ -424,11 +432,17 @@ const MapLayerModal = ({ rampName, rampMin: hasRange ? rampMin : "", rampMax: hasRange ? rampMax : "", + rampReverse: rampReverse === true, hasNodata: true, maskBelow: validSourceProps.mask_below, }); mapConfiguration.configuration.style = { color }; mapConfiguration.configuration.props.source.rampName = rampName; + // Persisted only when set, so an unreversed layer's config is unchanged + // from before this option existed. + if (rampReverse === true) { + mapConfiguration.configuration.props.source.rampReverse = true; + } // Raw range styles raw band values; anything less than a full range // normalizes band 1 from stats until the render-time resolve lands. mapConfiguration.configuration.props.source.props.normalize = !hasRange; diff --git a/reactapp/components/modals/MapLayer/RampPicker.js b/reactapp/components/modals/MapLayer/RampPicker.js index e8746daa..587174db 100644 --- a/reactapp/components/modals/MapLayer/RampPicker.js +++ b/reactapp/components/modals/MapLayer/RampPicker.js @@ -1,7 +1,7 @@ import { Fragment } from "react"; import PropTypes from "prop-types"; import styled from "styled-components"; -import { COLOR_RAMPS, RAMP_GROUPS } from "components/map/colorRamps"; +import { RAMP_GROUPS, resolveRamp } from "components/map/colorRamps"; const PickerList = styled.div` display: flex; @@ -27,12 +27,6 @@ const GroupLabel = styled.div` } `; -const RampLabel = styled.span` - flex: 0 0 5.5rem; - font-size: 0.8rem; - color: #212529; -`; - const RampRow = styled.button` display: flex; align-items: center; @@ -68,14 +62,15 @@ const GradientSwatch = styled.span` const buildGradient = (colors) => `linear-gradient(to right, ${colors.join(", ")})`; -const RampPicker = ({ selectedRamp, onChange }) => { +const RampPicker = ({ selectedRamp, onChange, reversed }) => { return ( {RAMP_GROUPS.map((group) => ( {group.names.map((name) => { - const colors = COLOR_RAMPS[name]; + // Swatches preview the reversed direction, so the picker matches the map. + const colors = resolveRamp(name, reversed); const isSelected = selectedRamp === name; return ( { $selected={isSelected} onClick={() => onChange(name)} > - {name}