From 547dd1bfae2d1dfde995336f85be7db2757493aa Mon Sep 17 00:00:00 2001 From: Corey Krewson Date: Mon, 17 Aug 2026 15:29:11 -0700 Subject: [PATCH 1/3] Raise a fill-viewport map so its legend and layer control clear other tiles Opening a map's legend or layer control drew it behind any grid item overlapping the map, but only when the map filled the viewport, and only outside edit mode. fillViewportActive makes the item position:fixed, which creates a stacking context even at z-index:auto. That seals the item's subtree in: the legend, layer control, error alert and coordinate readout all set z-index:1000, but inside a stacking context that only orders them against each other, never against another grid item. The item paints as one unit in gridItems order, so any tile ordered after the map covered the map's own controls. Edit mode was unaffected because fillViewportActive is gated on !isEditing, leaving the item position:relative and so not a stacking context -- which is why the controls behaved there. No descendant z-index can escape a stacking context, so the fix raises the item itself while a control is open, lifting the whole subtree. Each control flags its own container with data-map-control-open while expanded, and a :has() rule scoped to the fill-viewport branch raises the item to 1029. Two consequences worth naming. A tile ordered after the map is hidden while a control is open, reverting on close -- accepted deliberately, since the alternative is portalling each control out of the map and positioning it from a measured bounding rect. And 1029 clears dropdowns (1000) and sticky (1020) but stays under the fixed header (1030) and all modal chrome, so a modal still covers the map. LayerControlContainer also gains the aria-label it was missing, matching the legend's, which is how the test reaches it without direct DOM access. Co-Authored-By: Claude Opus 5 --- .../dashboard/DashboardItem.test.js | 92 +++++++++++++++++++ .../components/map/LayersControl.test.js | 21 +++++ .../__tests__/components/map/Legend.test.js | 18 ++++ .../components/dashboard/DashboardItem.js | 23 +++++ reactapp/components/map/LayersControl.js | 6 +- reactapp/components/map/LegendControl.js | 1 + 6 files changed, 160 insertions(+), 1 deletion(-) diff --git a/reactapp/__tests__/components/dashboard/DashboardItem.test.js b/reactapp/__tests__/components/dashboard/DashboardItem.test.js index 721a45f2..3c2ad5f0 100644 --- a/reactapp/__tests__/components/dashboard/DashboardItem.test.js +++ b/reactapp/__tests__/components/dashboard/DashboardItem.test.js @@ -5,6 +5,7 @@ import { within, fireEvent, waitFor, + cleanup, } from "@testing-library/react"; import DashboardItem, { handleGridItemExport, @@ -1655,6 +1656,97 @@ test("Dashboard attribution and not show", async () => { ).not.toBeInTheDocument(); }); +// jsdom's computed style does not resolve :has(), so the raise cannot be read +// back through getComputedStyle. Inspecting the injected rule is the next best +// thing: it pins that the rule ships, what it raises to, and that it is scoped to +// the fill-viewport branch rather than applied to every grid item. +const injectedCss = () => + Array.from(document.styleSheets) + .flatMap((sheet) => { + try { + return Array.from(sheet.cssRules).map((rule) => rule.cssText); + } catch { + return []; + } + }) + .join("\n"); + +const raiseRule = () => + injectedCss() + .split("\n") + .find((rule) => rule.includes('data-map-control-open="true"')); + +// The class styled-components generated for the fill-viewport block. Asserting +// class membership on the element is order-independent, unlike asserting the rule +// is absent from the stylesheet -- styled-components keeps injected rules for the +// whole test file, so a rule from an earlier test is still present. +const raiseRuleClass = () => { + const rule = raiseRule(); + const match = rule && rule.match(/^\.([\w-]+)/); + return match ? match[1] : null; +}; + +const renderGridItem = ({ fillViewport }) => { + const mockedDashboard = JSON.parse(JSON.stringify(userDashboard)); + const gridItem = mockedDashboard.tabs[0].gridItems[0]; + gridItem.metadata_string = JSON.stringify( + fillViewport ? { fillViewport: true } : {}, + ); + + return render( + createLoadedComponent({ + children: ( + + + + ), + options: { initialDashboard: mockedDashboard }, + }), + ); +}; + +test("Dashboard Item fill viewport raises the tile for an open map control", async () => { + // position:fixed seals the item into its own stacking context, so a map's + // legend or layer control cannot paint above a later grid item on its own. + renderGridItem({ fillViewport: true }); + const item = await screen.findByLabelText("gridItemDiv"); + + const rule = raiseRule(); + expect(rule).toBeDefined(); + expect(rule).toMatch(/z-index:\s*1029/); + // Below the fixed header and every modal layer, so a modal still covers the map. + expect(rule).not.toMatch(/z-index:\s*10[4-9]\d/); + // And the rule actually applies to this item. + expect(item.classList.contains(raiseRuleClass())).toBe(true); +}); + +test("Dashboard Item without fill viewport is not covered by the raise rule", async () => { + // A non-fill item is position:relative / z-index:auto, so it is not a stacking + // context and the control's own z-index already escapes. Scoping the rule to + // the fill branch keeps that path untouched. + renderGridItem({ fillViewport: true }); + await screen.findByLabelText("gridItemDiv"); + const fillClass = raiseRuleClass(); + expect(fillClass).not.toBeNull(); + cleanup(); + + renderGridItem({ fillViewport: false }); + const item = await screen.findByLabelText("gridItemDiv"); + expect(window.getComputedStyle(item).getPropertyValue("position")).toBe( + "relative", + ); + expect(item.classList.contains(fillClass)).toBe(false); +}); + test("Dashboard Item fill viewport fills the content area in view mode", async () => { const mockedDashboard = JSON.parse(JSON.stringify(userDashboard)); const gridItem = mockedDashboard.tabs[0].gridItems[0]; diff --git a/reactapp/__tests__/components/map/LayersControl.test.js b/reactapp/__tests__/components/map/LayersControl.test.js index 02fe2b27..daffb54c 100644 --- a/reactapp/__tests__/components/map/LayersControl.test.js +++ b/reactapp/__tests__/components/map/LayersControl.test.js @@ -251,3 +251,24 @@ describe("parseProgress", () => { expect(parseProgress(message)).toBeNull(); }); }); + +test("LayersControl flags itself only while expanded", async () => { + render( + , + ); + + expect(screen.getByLabelText("Layers Control")).not.toHaveAttribute( + "data-map-control-open", + ); + + fireEvent.click(await screen.findByLabelText("Show Layers Control")); + expect(screen.getByLabelText("Layers Control")).toHaveAttribute( + "data-map-control-open", + "true", + ); + + fireEvent.click(await screen.findByLabelText("Close Layers Control")); + expect(screen.getByLabelText("Layers Control")).not.toHaveAttribute( + "data-map-control-open", + ); +}); diff --git a/reactapp/__tests__/components/map/Legend.test.js b/reactapp/__tests__/components/map/Legend.test.js index 42f04a0c..a0af313d 100644 --- a/reactapp/__tests__/components/map/Legend.test.js +++ b/reactapp/__tests__/components/map/Legend.test.js @@ -44,3 +44,21 @@ test("LegendControl", async () => { fireEvent.click(closeLegendButton); expect(screen.queryByText("Some New Title")).not.toBeInTheDocument(); }); + +test("LegendControl flags itself only while expanded", async () => { + // The flag is what DashboardItem's fill-viewport rule keys off to raise the + // whole tile. Raising the tile is the only lever available: position:fixed + // makes it a stacking context, so no z-index on the control can escape it. + render(); + + fireEvent.click(await screen.findByLabelText("Show Legend Control")); + expect(await screen.findByLabelText("Legend Control")).toHaveAttribute( + "data-map-control-open", + "true", + ); + + fireEvent.click(await screen.findByLabelText("Close Legend Control")); + expect(screen.getByLabelText("Legend Control")).not.toHaveAttribute( + "data-map-control-open", + ); +}); diff --git a/reactapp/components/dashboard/DashboardItem.js b/reactapp/components/dashboard/DashboardItem.js index d87a8586..99a56c18 100644 --- a/reactapp/components/dashboard/DashboardItem.js +++ b/reactapp/components/dashboard/DashboardItem.js @@ -78,6 +78,29 @@ const StyledDiv = styled.div` left: 0; width: 100vw; height: calc(100vh - (${props.$fillOffset})); + + /* position:fixed creates a stacking context even at z-index:auto, which + seals everything inside this item in. A map's legend, layer control, + error alert and coordinate readout all set z-index:1000, but that only + orders them against each other, never against another grid item -- the + item paints as one unit in gridItems order, so a tile ordered after this + one covered the map's own controls. In edit mode the bug disappears + because fillViewportActive is gated on !isEditing, leaving the item + position:relative and therefore not a stacking context. + + Raising the item while a control is open is the only way out: no + descendant z-index can escape a stacking context, so lifting the whole + subtree is the lever available. The trade is that an overlapping tile + ordered after this one is hidden for as long as the control is open. It + reverts on close, so the DOM-order layering described above still holds + the rest of the time. + + 1029 clears dropdowns (1000) and sticky (1020) but stays below the fixed + header (1030) and all modal chrome (backdrop 1040, modal 1050, popover + 1070, tooltip 1080, app alerts 1081), so a modal still covers the map. */ + &:has([data-map-control-open="true"]) { + z-index: 1029; + } `} `; diff --git a/reactapp/components/map/LayersControl.js b/reactapp/components/map/LayersControl.js index 64c24858..af60d4ad 100644 --- a/reactapp/components/map/LayersControl.js +++ b/reactapp/components/map/LayersControl.js @@ -160,7 +160,11 @@ const LayersControl = ({ updater, visualizationRef, runtimeLayerState }) => { return ( - + {isexpanded ? ( <> Map Layers diff --git a/reactapp/components/map/LegendControl.js b/reactapp/components/map/LegendControl.js index 52556ce2..e72907ce 100644 --- a/reactapp/components/map/LegendControl.js +++ b/reactapp/components/map/LegendControl.js @@ -55,6 +55,7 @@ const LegendControl = ({ legendItems }) => { {isexpanded ? ( From 3133b15d2a58fafb6eb9a1f0a58598b516d48476 Mon Sep 17 00:00:00 2001 From: Corey Krewson Date: Mon, 17 Aug 2026 16:19:06 -0700 Subject: [PATCH 2/3] Omit a table's heading when the plugin returns no title title is optional in the `table` return shape, but DataTable rendered it unguarded while guarding the subtitle immediately below it. A plugin that returned only `data` therefore got an empty

-- a heading's worth of blank space above the table -- rather than no heading. Guarded the same way the subtitle already was. Co-Authored-By: Claude Opus 5 --- .../visualizations/DataTable.test.js | 22 +++++++++++++++++++ .../components/visualizations/DataTable.js | 5 ++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/reactapp/__tests__/components/visualizations/DataTable.test.js b/reactapp/__tests__/components/visualizations/DataTable.test.js index 20efa37d..3d27da85 100644 --- a/reactapp/__tests__/components/visualizations/DataTable.test.js +++ b/reactapp/__tests__/components/visualizations/DataTable.test.js @@ -68,6 +68,28 @@ it("Creates a Data Table with the provided data", () => { expect(occupationData3).toBeInTheDocument(); }); +it("Omits the heading entirely when a plugin returns no title", () => { + // title is optional in the `table` return shape. Rendering it unguarded left + // an empty

-- a heading's worth of blank space above the table -- for any + // plugin that returns only `data`. + const { title, ...withoutTitle } = mockedTableData; + initAndRender(withoutTitle); + + expect(screen.queryByRole("heading")).not.toBeInTheDocument(); + // The table itself still renders. + expect(screen.getByText("Name")).toBeInTheDocument(); + expect(screen.getByText("Alice Johnson")).toBeInTheDocument(); +}); + +it("Still renders the heading when a title is provided", () => { + const { subtitle, ...withTitle } = mockedTableData; + initAndRender({ ...withTitle, title: "User Information" }); + + expect( + screen.getByRole("heading", { name: "User Information" }), + ).toBeInTheDocument(); +}); + it("Creates a Data Table with subtitle with the provided data", () => { mockedTableData.subtitle = "some subtitle"; initAndRender(mockedTableData); diff --git a/reactapp/components/visualizations/DataTable.js b/reactapp/components/visualizations/DataTable.js index ac3b8835..b505fb4b 100644 --- a/reactapp/components/visualizations/DataTable.js +++ b/reactapp/components/visualizations/DataTable.js @@ -47,7 +47,10 @@ const DataTable = ({ data, title, subtitle, visualizationRef }) => { return ( -

{title}

+ {/* Guarded like the subtitle below it: title is optional in the `table` + return shape, and rendering an empty

left a heading's worth of + blank space above the table for any plugin that omits one. */} + {title &&

{title}

} {subtitle &&

{subtitle}

} From 0ec875b44a6311a223844a240cfea713b90864d2 Mon Sep 17 00:00:00 2001 From: Corey Krewson Date: Mon, 17 Aug 2026 16:26:12 -0700 Subject: [PATCH 3/3] Omit a card's header when the plugin returns no title or description Both are optional in the `card` return shape, but Card rendered each unguarded inside a Header that always rendered. A plugin returning only `data` therefore got an empty

, an empty

, and Header's own 1.5rem margin above the stats. Guarded each field, and the wrapper too -- dropping only the fields would still leave the margin behind. Header carries a testid so its absence is observable; an empty wrapper renders no text, so no query could otherwise tell it apart from no wrapper at all. Same fix as the preceding commit for DataTable's title, which had the narrower version of this: guarded subtitle, unguarded title. Co-Authored-By: Claude Opus 5 --- .../components/visualizations/Card.test.js | 34 +++++++++++++++++++ reactapp/components/visualizations/Card.js | 13 ++++--- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/reactapp/__tests__/components/visualizations/Card.test.js b/reactapp/__tests__/components/visualizations/Card.test.js index c52b5384..fc827938 100644 --- a/reactapp/__tests__/components/visualizations/Card.test.js +++ b/reactapp/__tests__/components/visualizations/Card.test.js @@ -48,6 +48,40 @@ it("Creates a Card with a Title and Description", () => { expect(screen.getByText("Fake Description")).toBeInTheDocument(); }); +it("Omits the header entirely when a plugin returns neither title nor description", async () => { + // Both are optional in the `card` return shape. Rendering them unguarded left + // an empty heading, an empty paragraph and Header's 1.5rem margin above the + // stats for any plugin that returns only `data`. + const { data } = mockedCardData; + initAndRender({ data }); + + // The stats render behind Suspense, so wait for them before concluding the + // header is absent rather than merely not painted yet. + expect(await screen.findByText("Total Sales")).toBeInTheDocument(); + expect(screen.getByText("1,500")).toBeInTheDocument(); + expect(screen.queryByRole("heading")).not.toBeInTheDocument(); + // Not merely empty: the wrapper itself must go, or its 1.5rem margin stays. + expect(screen.queryByTestId("card-header")).not.toBeInTheDocument(); +}); + +it("Renders only the title when no description is given", () => { + initAndRender({ title: "Fake Title", data: [] }); + + expect( + screen.getByRole("heading", { name: "Fake Title" }), + ).toBeInTheDocument(); + expect(screen.queryByText("Fake Description")).not.toBeInTheDocument(); +}); + +it("Renders only the description when no title is given", () => { + // The case the inner title guard exists for: the header is rendered because a + // description is present, so without the guard an empty

comes with it. + initAndRender({ description: "Fake Description", data: [] }); + + expect(screen.getByText("Fake Description")).toBeInTheDocument(); + expect(screen.queryByRole("heading")).not.toBeInTheDocument(); +}); + it("Creates a Card with actual data", async () => { const { title, data } = mockedCardData; initAndRender({ diff --git a/reactapp/components/visualizations/Card.js b/reactapp/components/visualizations/Card.js index ae6752e0..d69672a7 100644 --- a/reactapp/components/visualizations/Card.js +++ b/reactapp/components/visualizations/Card.js @@ -89,10 +89,15 @@ const StatItemGroup = ({ item, index }) => { const Card = ({ title, description, data, visualizationRef }) => { return ( -
-

{title}

-

{description}

-
+ {/* Both are optional in the `card` return shape. Rendering them unguarded + left an empty heading and paragraph, and Header's own 1.5rem margin, + above the stats for any plugin that returns only `data`. */} + {(title || description) && ( +
+ {title &&

{title}

} + {description &&

{description}

} +
+ )} {data.length === 0 ? ( ) : (