Skip to content

Keep the map Confirm button beneath the order options sheet - #1400

Merged
johnpooch merged 1 commit into
mainfrom
claude/mobile-confirm-orders-overlap-ral82h
Sep 25, 2026
Merged

johnpooch merged 1 commit into
mainfrom
claude/mobile-confirm-orders-overlap-ral82h

Conversation

@johnpooch

Copy link
Copy Markdown
Owner

What this PR does

A player reported this on mobile: after tapping a unit on the map screen, the floating Confirm (n/m) button showed on top of the Hold / Move / Support options sheet and hid part of it.

Cause: the Confirm button's wrapper in MapScreen.tsx used z-[1000]. On mobile, FloatingMenu shows the order options in a Sheet, and the sheet's overlay and content are both z-50 (components/ui/sheet.tsx). Nothing between the wrapper and the page root creates a stacking context, so 1000 beat 50 and the button was painted over the sheet.

The fill toggle next to it also uses z-[1000], but it stays under the sheet. It sits inside GameMapCanvas's isolation: isolate container, so its z-index only counts inside the map.

Fix: the Confirm button only needs to sit above that isolated map container, so its wrapper now uses z-10, the same value GameMap's province banner uses. It still floats over the map when no sheet is open, and the sheet's overlay now dims it and the sheet covers it.

Reproduction

I reproduced this in headless Chromium at 390×844 against the MSW mocks. The mock /options/ response is empty, so for the screenshots only I gave the active-movement fixture some order options for England's army in Liverpool. That change is not committed. I then tapped Liverpool on the map.

Screenshots

Before and after at 390×844, taken right after tapping A Liverpool:

Before After
screenshot to attach: Confirm (2/3) drawn over the Move/Support rows screenshot to attach: sheet covers the button

This session has no API for uploading GitHub attachments, so the images were captured but still need to be dragged in here. With the sheet closed, the button still shows above the map, bottom right, next to the fill toggle.

The Screenshot Diff workflow won't show this change: it only captures static screens, and the sheet opens only after tapping a unit.

Checklist

  • This PR does one thing — no unrelated fixes, refactors, or drive-by cleanups bundled in
  • For PRs of any significant complexity: I ran /review-pr against this PR in Claude Code and addressed (or responded to) its findings — one-class change
  • Tests cover the change — jsdom does no layout or stacking, so a unit test could only check the class string. I verified the fix in a real browser instead (above). MapScreen.test.tsx passes, and so do eslint and npx tsc -b --noEmit.
  • Screenshots embedded in the PR description for any visual changes (see CLAUDE.md) — captured, still to be attached (see above)

🤖 Generated with Claude Code

https://claude.ai/code/session_01Y9qBhhFYZaUPJKjPRoFKv6


Generated by Claude Code

The Confirm button's wrapper used z-[1000], which painted it above the
mobile order options bottom sheet (z-50). It only needs to sit above the
map canvas, which is its own isolated stacking context, so z-10 suffices.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y9qBhhFYZaUPJKjPRoFKv6
@github-actions

Copy link
Copy Markdown
Contributor

Screenshot diff

0 screens changed of 276 captured.

No visual differences found.

@johnpooch
johnpooch merged commit 95b75b5 into main Sep 25, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants