fix(mobile): prevent integrated action menu clipping on mode homes - #292
Merged
Conversation
This pull request has been ignored for the connected project Preview Branches by Supabase. |
BigSimmo
marked this pull request as ready for review
July 5, 2026 17:46
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
BigSimmo
enabled auto-merge (squash)
July 5, 2026 17:56
BigSimmoforce-pushed
the
cursor/fix-mobile-action-popup-whitespace-4dcc
branch
from
July 6, 2026 04:41
3a6b018 to
9201f7aCompareDefer focus restoration until after the scope popover unmounts, and use double requestAnimationFrame in dismissable layers so Escape reliably returns focus to the answer options trigger.
Uh oh!
There was an error while loading. Please reload this page.
BigSimmo pushed a commit
that referenced
this pull request
Aug 9, 2026
…check Implements the smallest correct fix for the two recommendations that had one, and corrects a recommendation that turned out to be wrong. #291 — corrected, not implemented as written. The row claimed that pairing `disabled` with an aria-describedby sr-only reason means the reason is never announced, and that the docs and eslint rule blessed the pairing so a repo-wide pass was needed. All three claims are refuted. A natively disabled button stays in the accessibility tree with its accessible description intact; tests/favourites-hub-unavailable-controls.dom.test.tsx pins toBeDisabled(), not.toHaveAttribute("aria-disabled") and toHaveAccessibleDescription() together under the title "keeps unavailable actions natively disabled and exposes their reasons" — a deliberate, tested decision. require-button-wiring already accepts both forms. An attempt to swap the attribute in favourites-hub and four other sites was reverted for contradicting that decision. The one real defect was documentation: docs/wiring-conventions.md named favourites-hub.tsx as "the reference markup" while showing aria-disabled="true", which is not what that file does. The canonical block now shows native `disabled`, explains why the description survives it, cites the pinning test, and says when aria-disabled + a no-op handler is correct instead. It also records that `disabled` + `aria-disabled` together is a third shape pinned two contradictory ways in one test file, which stays open as the residue of #291. #292 — mitigation implemented. The open-PR check before starting a queued item is now in newtask's "Before you start" (which already made that GitHub read for PR bundling, so it asks the same list a second question at no extra cost), in the issues skill after the read-only flow, and in the /issues section of AGENTS.md so Codex and Cursor get it too rather than Claude Code only. All three scan for the route/component/surface rather than the ledger id, and degrade to a warning when GitHub is unreachable. #290 — not implementable here: it needs two more same-SHA Playwright reproductions, and this container has no node_modules or browsers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Ex4A4xx3tNdZUADV9VaXK
BigSimmo pushed a commit
that referenced
this pull request
Aug 9, 2026
…r-status claim Two valid review findings on #1773, both verified against the repo before acting. Codex (P2, docs/outstanding-issues.md #290): the "Next" step added in 16b6119 told the reader to append each of the next two reproductions to tests/flake-ledger.json. Following it cannot produce a valid ledger state. docs/testing.md requires three same-SHA reproductions before an entry is added or retained, and scripts/flake-ledger.mjs:48 throws "exact title must include @quarantine" while tests/ui-smoke.spec.ts:4470 is untagged — so an entry written today fails validation outright, and appending per run treats the ledger as a per-run log. The step now says to record the two observations in the row itself, then after the third reproduction tag the test @quarantine and add one complete entry with owner, repro command, tracking ref, first/last-seen and expiry. CodeRabbit (docs/outstanding-issues.md #292): the claim that the ledger "has no in-progress state" is wrong — IN PROGRESS appears on two rows and IMPLEMENTED in PR #1766 on another. The accurate gap is narrower and worse: there is no structured status field and no atomic claim, so a marker is written by whoever did the work, usually after the fact, and nothing requires or checks one, which means the absence of a marker carries no information. Corrected in all four places that carried the overstatement, not only the flagged row — AGENTS.md, the issues and newtask skills, and #292 itself — since the same sentence had been copied into each. CI on the previous head was green; no check failures were outstanding. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Ex4A4xx3tNdZUADV9VaXK
BigSimmo added a commit
that referenced
this pull request
Aug 9, 2026
…ree items (#1773) * docs: pin PageSection ownership to the nav-header sibling, capture three items Record the in-page-nav decision in docs/search-chrome-behaviour.md under "Default in-page navigation template": a page's PageSection[] table is always owned and exported by a colocated "use client" nav-header sibling, never declared inline in the page and never in a separate per-route section-index module. The four Server Component pages need the sibling regardless, since neither onSelectSection nor a LucideIcon crosses the RSC boundary. Applying the same shape to Client Component pages gives one answer to where the section table lives and one import path for tests/in-page-nav-route-sections.dom.test.tsx. PR #1766 shipped both shapes; the closed PR #1767 proposed a third. States explicitly that existing pages are not being migrated — the rule binds new conversions only, and the grandfathered sites are listed so the inconsistency is not read as a bug. Ledger: - #291 (P2 issue) disabled placeholder buttons pair `disabled` with an aria-describedby sr-only reason; `disabled` removes the tab stop, so the reason is never announced. Needs one repo-wide pass across the sites, docs/wiring-conventions.md and eslint-rules/require-button-wiring.mjs. - #292 (P2 rec) two sessions built the same queued item four hours apart (PR #1766, closed PR #1767); check the open-PR list for the affected route before starting a queued item, not just the ledger row. - #290 extended with the quarantine arithmetic: tests/flake-ledger.json is empty, so two more same-SHA reproductions are needed before a quarantine entry is admissible. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Ex4A4xx3tNdZUADV9VaXK * docs: fix the placeholder-pattern reference, wire the duplicate-work check Implements the smallest correct fix for the two recommendations that had one, and corrects a recommendation that turned out to be wrong. #291 — corrected, not implemented as written. The row claimed that pairing `disabled` with an aria-describedby sr-only reason means the reason is never announced, and that the docs and eslint rule blessed the pairing so a repo-wide pass was needed. All three claims are refuted. A natively disabled button stays in the accessibility tree with its accessible description intact; tests/favourites-hub-unavailable-controls.dom.test.tsx pins toBeDisabled(), not.toHaveAttribute("aria-disabled") and toHaveAccessibleDescription() together under the title "keeps unavailable actions natively disabled and exposes their reasons" — a deliberate, tested decision. require-button-wiring already accepts both forms. An attempt to swap the attribute in favourites-hub and four other sites was reverted for contradicting that decision. The one real defect was documentation: docs/wiring-conventions.md named favourites-hub.tsx as "the reference markup" while showing aria-disabled="true", which is not what that file does. The canonical block now shows native `disabled`, explains why the description survives it, cites the pinning test, and says when aria-disabled + a no-op handler is correct instead. It also records that `disabled` + `aria-disabled` together is a third shape pinned two contradictory ways in one test file, which stays open as the residue of #291. #292 — mitigation implemented. The open-PR check before starting a queued item is now in newtask's "Before you start" (which already made that GitHub read for PR bundling, so it asks the same list a second question at no extra cost), in the issues skill after the read-only flow, and in the /issues section of AGENTS.md so Codex and Cursor get it too rather than Claude Code only. All three scan for the route/component/surface rather than the ledger id, and degrade to a warning when GitHub is unreachable. #290 — not implementable here: it needs two more same-SHA Playwright reproductions, and this container has no node_modules or browsers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Ex4A4xx3tNdZUADV9VaXK * docs: address PR review — fix the quarantine sequencing and the ledger-status claim Two valid review findings on #1773, both verified against the repo before acting. Codex (P2, docs/outstanding-issues.md #290): the "Next" step added in 16b6119 told the reader to append each of the next two reproductions to tests/flake-ledger.json. Following it cannot produce a valid ledger state. docs/testing.md requires three same-SHA reproductions before an entry is added or retained, and scripts/flake-ledger.mjs:48 throws "exact title must include @quarantine" while tests/ui-smoke.spec.ts:4470 is untagged — so an entry written today fails validation outright, and appending per run treats the ledger as a per-run log. The step now says to record the two observations in the row itself, then after the third reproduction tag the test @quarantine and add one complete entry with owner, repro command, tracking ref, first/last-seen and expiry. CodeRabbit (docs/outstanding-issues.md #292): the claim that the ledger "has no in-progress state" is wrong — IN PROGRESS appears on two rows and IMPLEMENTED in PR #1766 on another. The accurate gap is narrower and worse: there is no structured status field and no atomic claim, so a marker is written by whoever did the work, usually after the fact, and nothing requires or checks one, which means the absence of a marker carries no information. Corrected in all four places that carried the overstatement, not only the flagged row — AGENTS.md, the issues and newtask skills, and #292 itself — since the same sentence had been copied into each. CI on the previous head was green; no check failures were outstanding. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Ex4A4xx3tNdZUADV9VaXK --------- Co-authored-by: Claude <noreply@anthropic.com>
BigSimmo pushed a commit
that referenced
this pull request
Aug 12, 2026
Checked all 12 open PRs and mapped them to ledger rows. Twenty-four open rows are actively being fixed right now — #1835 (7 rows), #1840 (4), #1842 (6), #1841 (4), #1836 (2), #1837 (1), #1839 (1) — and none of those rows said so. This is the #292 failure mode with the safety off: a queued row that reads as unclaimed while a PR is open against it is how the same conversion shipped twice on 2026-08-09. Each row now names its PR and says not to start until that PR merges. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017paT42ZVMf8jaLtkjFxdy5
BigSimmo added a commit
that referenced
this pull request
Aug 13, 2026
… queue text (#1890) * docs(issues): retire 22 non-actionable ledger rows and correct the #231 queue text A yield review of all 114 open rows against current main. The queue had become roughly 60 tasks and 50 notes; this removes the notes and fixes two places where the ledger was actively misdirecting. The correction that matters most: the recommended-queue entry for #231, the top clinical P1, told every session to "measure and fix the fast-route budget / generation timeout" — an approach #231's own detail records as tested and rejected, because the decisive 40-second probe completed generation in 25.272s with route_deadline_exceeded=false and still failed quality. The session-start hook prints the queue, not the row, so the refuted text was the text agents read. Closed 22 rows: - #304 was already done on main (commit d182844 refreshed the ranking snapshot; generatedAt is 0 days old, not 2026-07-20), yet sat in the queue advertising a freshness fuse that is not armed. - #241#244#272#294#300#257 were standing cautions whose own text says "no action". Each one's knowledge now lives in the code it protects, so closing the row loses nothing. - #196-#200 are five steps of the disaster-recovery checklist that is canonical in docs/operator-backlog.md, with no trigger until a restore. - #86#188 were index rows over children that are individually findable. - #250#253#254 were superseded; #250 and #253 say so themselves. - #156#301#152#236#260 merged into #168, #292 and #169 respectively — each pair or group was one problem recorded two to four times. Demoted 20 rows with a stated reason (premature ops for a single-user prototype, upstream-blocked, measurement-gated, or design-system adoption competing with an open clinical P1). The Pri cell is unchanged because the writer has no --pri flag — which is now #313. Added three rows for mechanism gaps this sweep exposed: rows outliving their own completion (#312), the missing --pri flag (#313), and the queue being able to contradict the row it cites with no guard (#314). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DuYJz8hauCsCdx8r4fXiZU * docs(ledger): record the ledger yield review handoff Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DuYJz8hauCsCdx8r4fXiZU * Keep recovery work visible and pin forced colors --------- Co-authored-by: Claude <noreply@anthropic.com>
BigSimmo pushed a commit
that referenced
this pull request
Aug 14, 2026
Hand-written follow-up to the machine-generated transaction in the previous commit. Separate commit so the reconciliation stays independently revertible and remains purely generated. - newtask: the #292 open-PR duplicate check is DENIED, not merely unavailable, once the PR-handoff stop hook has armed in a session. The existing text only covers "GitHub is unreachable", which reads as a network problem and does not tell a session what to do when the tool itself is blocked. Records git ls-remote --heads as the fallback, and says explicitly not to unlock the hook for it. - handoff: --force-with-lease fails with "stale info" when restarting a branch whose PR merged, because GitHub deleted the remote branch and the local remote-tracking ref is stale. The failure looks like a lease violation inviting --force; it is not. git remote prune origin, then push normally — there is nothing to force. - issues: the visual-register refresh is unavailable off the operator's Windows machine, so a cloud session can reconcile but cannot refresh or even measure staleness. States the limit where the command is, and forbids improvising a substitute renderer: an artifact that looks refreshed but came from a different generator is worse than a visibly stale one. Queues one P3 request for the register drift itself, which is cumulative rather than a single-run failure. No predicted ledger id is cited anywhere — ids are allocated at reconcile, which is the #168 hazard. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Chrn9bTYFFYzrLZqtpVptW
BigSimmo pushed a commit
that referenced
this pull request
Aug 14, 2026
Four merge-safe inbox requests; docs/outstanding-issues.md is untouched and is reconciled separately after this PR lands. - update #316: Phase 0 closed, including the forced-dispatch proof (run 31813064485 -> auto-created issue #1963). Also supersedes the stale 2026-08-09 drift figures with measured ones: 10 RPC mismatches unchanged, 20 missing indexes, 2 unexpected, and the two trigram indexes confirmed restored. - done #331: its comparison-bug hypothesis is refuted; the staleness was real and wrapping-only, inherited from main by every branch. - update #333: the regeneration half is done here; its real question - the check runs in verify:pr-local but in no CI job - stays open. - update #292: records the #1938/#1939 Phase 0 duplicate against the existing duplicate-work row rather than opening a near-identical new one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BKNFogaYfCQBvFVqFQnfRt
BigSimmo pushed a commit
that referenced
this pull request
Aug 18, 2026
While this PR was open, a concurrent session landed an equivalent fix on main as 049760a ("fix(theme): clear and guard the theme-transition removal timer"), root-caused on PR #2052. Its implementation is functionally identical to the one here: same `typeof document === "undefined"` early return, same tracked handle cleared before scheduling. Keeping a second, cosmetically different version of the same fix would be pure churn and a conflict magnet, so use-theme.ts is reverted to main's version byte-for-byte. What main does NOT have is any regression test for this behaviour, so that is all this PR now carries. The test was re-verified against main's implementation rather than the one it was written for: it passes 3/3, and removing main's guard turns it red with the original `ReferenceError: document is not defined`. So it genuinely guards the code that shipped. This duplication is the failure mode tracked as outstanding issue #292 — two assistants building the same thing because neither checked the open PR list first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UYndHWrYJzirxbBvt68Tmx
7 tasks
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Fixes the large white gap on mobile that cut off the integrated Documents action menu (Search, Upload, Scope, Recent, etc.) on mode home screens.
Root cause
On phone mode homes, the search composer lives in the hero slot inside the scrolling
mainregion. The integrated+action menu was absolutely positioned inside that scroll container, so opening downward clipped the bottom rows against overflow and the vertically centred layout left dead space below the content.Changes
document.bodywith fixed positioning so they escape scroll clippingmode-action-popup.tsxfor PrettierminBodyHeightand thesetState-in-effect cleanup that failed ESLintreact-hooks/set-state-in-effectTest plan
mode-action-popup.tsxaddressed+, confirm all action tiles are visible or scroll inside the menu without a white cutoff band