Repository navigation
fix(map): lay MapLibre popups above the map's panels so a listing popup's pager and close button can be tapped - #504
Conversation
orangecoding
left a comment
There was a problem hiding this comment.
Nice write-up, the measurements made this easy to follow. One thing before merge: the new z-index leaks out of the map and puts popups above the listing detail's action bar and the job form's save bar. Details inline, the fix is one line.
| * controls anyway is better covering them for as long as it is open than being half dead; it always closes. | ||
| */ | ||
| .maplibregl-popup { | ||
| z-index: 20; |
There was a problem hiding this comment.
This z-index isn't scoped to the map. Outside fullscreen .map-shell is position: relative without a z-index, so it doesn't form a stacking context, and nothing above it does either. The 20 competes with the whole page:
- Listing detail:
.listing-actionbarisz-index: 20too (sticky at the top on desktop, fixed at the bottom on a phone). It comes before the map in the DOM, so the popup wins the tie. Open a popup on the location map (transit is on by default there, a departure board is enough) and scroll: the popup is drawn over the action bar and takes its clicks. On a phone that's the bar with the primary action. - Job form:
.settingsSaveBaris sticky at the bottom withz-index: 5. Turn on transit in the area map, open a departure board, scroll the map behind the save bar, same thing.
Before this change the popup was z-index: auto and both bars stayed on top. I checked it with a small headless page copying the rules: elementFromPoint on the bar returns the popup with this rule, and the bar again once the shell is isolated.
Fix is one line:
.map-shell {
position: relative;
isolation: isolate;The popup stays above the panels, but only inside the map. isolation doesn't make the shell a containing block for position: fixed, so fullscreen is unaffected. As a bonus it fixes the panel column (z-index 10) being drawn over the job form's save bar, which already happens on master.
| * it takes effect: that needs a browser (verified by hand in a viewport 390 wide and 809 high, where a real click | ||
| * on the pager arrow reaches "2 / 2" only with the rule). | ||
| */ | ||
| describe('map popup layering', () => { |
There was a problem hiding this comment.
With the isolation in place, please pin it here as well, e.g. a third case that .map-shell declares isolation: isolate. Otherwise the next z-index bump in Map.less leaks into the page again and this test stays green.
…up's pager and close button can be tapped On a short phone screen a listing popup is nearly as large as the map, and keepPopupInView only keeps it inside the map, not clear of the search box and the panel column laid over it. The popup's first row then sat under them: drawn, but not clickable. Depending on the map's height that was the pager arrows, the close button or both, so the second listing at an address could not be reached. The popup cannot be kept clear of the controls on a 360px phone (it does not fit beside the zoom buttons), so it now lies above them for as long as it is open. It always closes, and the panels are reachable again afterwards. The rule is in the shared map stylesheet because it applies to every MapLibre popup. test/ui/mapPopupLayering.test.js pins that the rule is declared at top level and ordered above the panels.
|
Good catch, thanks. You're right: |
237fe5d to
c416a33
Compare
What does this PR do?
Fixes the map popup of a group of listings at one address ("1 / 2") being impossible to page through on a phone.
On a phone a listing popup with a photo is nearly as large as the map, and
keepPopupInView()only keeps a popup inside the map, not clear of the controls laid over it (the address search, and the style/filter panel column,z-index: 10). The popup's first row then ends up under them: it is drawn, but the panels take the taps. Depending on the map's height that is the next arrow, the close button, or both.Reported from a Vivo X300 Pro, German UI. Reproduced locally with two listings at one address (a card with a photo, about 600px tall), a viewport 390 CSS px wide as in the reporter's screenshot, real mouse clicks, and
document.elementFromPointon the controls. Whether it happens depends on the map's height, because that decides how high the popup is pinned:Between those heights the covered control changes (next arrow and close button, only the next arrow, only the close button), and the same holds at 360 and 430 px wide (checked at a height of 809px: next and close covered before, everything reachable after). At 360px the open popup also lies over the zoom buttons.
It is not tied to one phone. The search box and the panel pill occupy a band at the top of the map, and the popup is pinned inside the map, so its first row meets that band when the map is not much taller than the popup. In these measurements a map 107px taller than the popup was fine and one 81px taller already had a covered control, so roughly 80-100px. Anything that makes the popup taller or the map shorter moves a phone into that range: a photo, more nearby stops, a longer title, a header that wraps (the German one does), larger system text, a browser bar.
The fix is one rule in the shared map stylesheet:
.maplibregl-popup { z-index: 20 }, above the panels (10). Keeping the popup clear of the controls instead does not work on a narrow phone: the popup (262px) does not fit beside the zoom buttons, and is about as tall as the map. The popup now covers the search box and panel chip while it is open; it always closes, and the panels are reachable again afterwards. Checked that the style dropdown (Semi portal) still opens above the popup, and that close and next stay reachable on the expanded (fullscreen) map.The test reads the stylesheet like other tests in
test/ui/and pins that the rule is declared at top level and ordered above the panels (it fails if the rule is removed, nested in a media query, or lowered). It cannot prove the popup takes taps; that needs a browser, which the repo does not have in its tests.Related issue
none
AI disclosure (required)
Fredy accepts AI assisted contributions, but they have to be declared. An
undeclared AI PR will be closed. Tick exactly one box:
ai:none- I wrote this myself. No AI generated code, text or commit messages.ai:assisted- AI helped with parts of it (autocomplete, refactoring, tests, docs). I reviewed and understand every line.ai:generated- AI produced most or all of this PR. I reviewed it, but it is largely machine written.Which AI: Claude Code, Sonnet 5.5
How much: the investigation in a local Fredy, the one-line fix and its test were drafted with Claude Code; I reported the bug from my phone (with the screenshot the geometry above comes from), reviewed every line and ran the checks.
Why: I hit the bug on my phone and could not reach the second listing at an address.
Checklist
yarn test:offlinepasses (oryarn testif the change touches a live provider)yarn lintandyarn format:checkpass