Fix/map controls above overlapping tiles - #173
Merged
Merged
Conversation
… 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 <noreply@anthropic.com>
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 <h2> -- 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 <noreply@anthropic.com>
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 <h3>, an empty <p>, 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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Raise fill-viewport map controls above overlapping tiles; stop rendering empty headings
Three commits, 10 files, +229 / −6. Two unrelated concerns — the branch name only describes the first. They are separate commits and cherry-pick cleanly if you would rather split them.
1. A fill-viewport map's legend and layer control were hidden by overlapping tiles
547dd1bfSymptom
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.
Cause
fillViewportActivemakes the itemposition: fixed, and a fixed element creates a stacking context even atz-index: auto. That seals the item's subtree in. The legend, layer control, error alert and coordinate readout all setz-index: 1000, but inside a stacking context that only orders them against each other — never against another grid item. The tile paints as one unit ingridItemsorder, so any tile ordered after the map covered the map's own controls.Edit mode was unaffected because
fillViewportActiveis gated on!isEditing, leaving the itemposition: relativeand therefore not a stacking context — which is exactly why the controls behaved there. That observation is what identified the bug; an earlier theory about z-index ties with Bootstrap dropdowns was wrong precisely because it would have behaved identically in both modes.Fix
No descendant
z-indexcan escape a stacking context, so the item itself is raised while a control is open, lifting the whole subtree. Each control flags its own container withdata-map-control-openwhile expanded, and a:has()rule scoped to the fill-viewport branch raises the item to1029.Two consequences, both deliberate:
1029clears dropdowns (1000) and sticky (1020) but stays under 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.LayerControlContaineralso gains thearia-labelit was missing, matching the legend's — which is how the test reaches it without the direct DOM accesstesting-library/no-node-accessforbids.2 & 3. Optional headings rendered as empty elements
3133b15d,0ec875b4titleis optional in thetablereturn shape;titleanddescriptionare both optional incard. Both renderers ignored that:A plugin returning only
datatherefore got an empty<h2>, or an empty<h3>+ empty<p>+Header's own1.5remmargin, as blank space above its content. Guarded now, the waysubtitlealready was. The Card wrapper is guarded too — dropping only the fields would leave the margin behind.Headercarries adata-testidso its absence is observable: an empty wrapper renders no text, so no query could otherwise tell it apart from no wrapper at all.Verification
131 suites, 2503 tests. Lint and Prettier clean.
Each new test was mutation-checked against the committed code — the mutation must fail the suite and the baseline must restore clean:
DataTable's titleCard's titleCard's Header wrapperCard's descriptionLimitations, stated plainly
:has()in computed styles, so the raise cannot be read back throughgetComputedStyle. The tests assert the injected rule instead: that it ships, what it raises to, that it stays below modal chrome, and that the element actually carries the generated class. They cannot prove paint order — this wants a quick look in a browser with a legend open over an overlapping tile before merging.Card's description is not caught. It only produces an empty<p>when a title exists without a description, and Testing Library has no query that distinguishes an empty paragraph from no paragraph without direct DOM access. The guard is correct; it is simply untested.Card's title (masked by the outer wrapper guard until a description-without-title case was added) and always rendering the Header (invisible to text queries until the testid was added).Notes
npm run build) to appear in the Django-served app.:has()is baseline-supported (Chrome 105+, Safari 15.4+, Firefox 121+).card-type plugin drop its title cleanly, which is the immediate motivation — but they apply to every table and card plugin.