Repository navigation
Conversation
Wrap controls only at widths at or below 760px and anchor Style/Export menus to the toolbar. Add a real-Chrome regression for English/Chinese, four styles and narrow/desktop viewports; retain keyboard access and print hiding. Rebuild the deterministic Skill ZIP with official Node 22.
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Thanks @BlueboxC for turning the shared #1068 evidence into this focused P0 fix and keeping the existing ownership split. I reviewed the 12af763 CSS change: it is limited to <=760px toolbar wrapping and menu anchoring, with desktop rules and print hiding preserved. I did not find a blocking regression in that CSS scope. The new toolbar browser case actually passed in CI 40092367553; the remaining full-regression failure is the centrally tracked README showcase drift, not a newly observed toolbar failure. Two small test-evidence clarifications, without asking you to repeat the published matrix: the Escape helper focuses the toggle before sending Escape, while the handler is on the panel, and there is no closing/focus-return assertion afterward. Please either make that focused assertion exercise the panel correctly or avoid claiming Escape coverage. Also, assigning dataset.style covers the style CSS but does not switch the selected button label, so it does not by itself prove every real selected-label width fits. Please personally review the narrow-screen wrapping and the intentional menu position below the whole toolbar, then complete the author-review checkbox only once that review is actually done. Keep the PR draft pending that confirmation and the remaining acceptance/merge decision. This comment is not a merge approval; no broad additional capture campaign is requested. |
|
Addressed the two test-evidence gaps in 3ff1f02. The existing matrix now chooses each style through its real menu item and verifies its selected label before measuring containment. Escape is sent from the item focused by opening the menu; the test checks hidden state, Adding the closure assertion to the old test reproduced the problem: the menu stayed open because the helper had moved focus to the toggle. The corrected 24 locale/style/viewport combinations pass, and the full nine-case Explainer Chrome suite passes on official Node22.23.2. No new screenshots or expanded capture campaign, and no runtime/CSS/package change in this follow-up. The PR remains draft with the personal author-review checkbox unchecked. Your requested human review and upstream acceptance are still separate. The previously disclosed README showcase drift has not been hidden. BlueboxC |
|
Thanks for the focused follow-up at 3ff1f02. I verified that the test now selects styles through the real menu and checks the selected label, and sends Escape with a menu item focused before asserting hidden state, aria-expanded=false and focus returning to the toggle. Both earlier test-evidence findings are closed. I also checked this head's CI 40095300022: the narrow-viewport toolbar case actually passes, with browser shards 135 + 170 passing and no skips. The remaining full-regression failure is the separate shared README showcase drift; I found no new toolbar failure. No broader matrix or runtime changes are requested. Thank you for keeping the draft and personal-review confirmation accurate; those remaining acceptance steps and merge approval are still separate. |
Problem and value
Agreed P0 from #1068. On current
dev82c7e0f089a02a4d514714adf37dc2061682da5f, the English toolbar reaches 401px in a 320px viewport. This prevents reliable access to Style/Export.Wrap the toolbar at <=760px and anchor its menus to that wrapping container. The runtime change is one media rule; no JavaScript, dependencies, release identities, or P1/P2 changes. Base remains unchanged as of publication.
AI assistance and accountability
This is a draft; the personal human-review confirmation remains unchecked.
Stability impact
Shared Explainer reader CSS; desktop >760px and existing print behavior are preserved. The test extends the existing Chrome fixture/helpers and checks observable containment, keyboard operation and hidden print controls rather than freezing layout coordinates. The two locale/four style combinations exercise their actual interactions. No concrete duplication or abstraction warranted a separate simplification.
Tests run
Candidate
12af763836e0e71aff585adfb41e3b6c3cfdb158on official Node22.23.2, bundled zlib1.3.1-e00f703:npm run test:full: generated checks pass; 3351 tests pass, 129 skip, with two failures investigated. The stale ZIP failure is corrected by rebuilding and rerunning the package gate (36 pass, 2 platform skips). The README motion-proof hash mismatch is also reproducible from unchangeddevfiles; it remains visible and is outside this agreed P0.Visual evidence
The new real-browser test is the reproducible entry point. Same document, locale, style and viewport on base/candidate; toolbar/menu bounds and keyboard behavior are automated. Perceptual human review is pending. The maintainer-agreed CSS P0 does not change generation semantics or initiate the repository-generation benchmark pilot.
Generated artifacts
archify.ziprebuilt using the canonical toolchain and verified by the existing reproducibility/package gate. Gallery artifacts do not embed this Explainer reader change; no unrelated showcase/Gallery refresh is included.BlueboxC