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/__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/__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/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 ? ( 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 ? ( ) : ( 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}

}