fix: remove the app-side causes of eight Playwright workarounds - #2172
Merged
Conversation
Base UI renders the popup with role="dialog" and takes its accessible name from a Popover.Title, which PopoverBaseUI never rendered — all 19 popovers announced as an unnamed dialog. The title is now a required prop so a new popover cannot ship without one, rendered visually hidden and as a span, so naming the popup stays out of the heading outline. The icon-only Actions trigger had no accessible name either; it now carries triggerTitle, the prop PopoverBaseUI already forwarded to both title and aria-label. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BVmU3hKdNyzNPxFQSFJ8oe
YesNoCheckbox renders an sr-only input, and hideLabel drops the Ja/Nein text as well — which was never a name, only the state. The notification matrix and the profit distribution table therefore offered five checkboxes per row that a screen reader announces without any hint of what they toggle. Both now pass an aria-label. The other hideLabel callers already carry an own <label htmlFor>, which the prop's doc comment now spells out as the alternative. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BVmU3hKdNyzNPxFQSFJ8oe
EditableField wraps a flex form and a flex display button in a span that was left at display: inline, so the span generated line boxes next to them and won the hit test for clicks aimed at the button. Playwright reported it as "<span> intercepts pointer events" and had to click the edit and save buttons with force, which also hid any real overlay regression from those tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BVmU3hKdNyzNPxFQSFJ8oe
…cked Moving the pointer onto a trigger with openOnHover already opens the popup after the 50ms delay, and the click that instinctively follows toggled it straight back closed — so clicking the Apps or the Create button in the top bar dismissed the menu it had just shown. The first trigger press after a hover-open is now swallowed; pressing again closes as before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BVmU3hKdNyzNPxFQSFJ8oe
getUnleashFlag cached the fetched definitions for a hardcoded 30 seconds, with no way to shorten that from outside the process. The Playwright stack therefore had to poll for up to 90 seconds per flag state inside a five minute test timeout. UNLEASH_REVALIDATE_SECONDS is optional and defaults to the same 30 seconds, so production is unchanged; the test stack sets it to one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BVmU3hKdNyzNPxFQSFJ8oe
- Dialogs are located by their accessible name. That retires the bare
getByRole("dialog") in four specs, and the local modal() copy in
iam.spec.ts, whose comment still announced the Base UI rewrite as
planned — it shipped, and Modal has named its popup ever since.
- Row action menus are located by name instead of by being the last
button in the row.
- The Apps popover is opened by clicking its trigger, the way a user
would, instead of hovering to dodge the toggle-shut bug.
- openInlineEditor is gone: without the forced click it was
clickUntilVisible, so the two call sites use that directly.
- The notification settings comment no longer claims the checkboxes have
no accessible name; a case now pins the names down instead.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BVmU3hKdNyzNPxFQSFJ8oe
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
The note type and classification level tiles mutated through REST routes and then called router.refresh() without awaiting it. The refresh landed after the UI had already settled and re-rendered the server tree under whatever was open — opening a row's actions menu right after another mutation could have it vanish under the cursor. Six actions replace the four API routes, authorizing and auditing exactly as those did, and revalidate the settings page from inside the action, so the refresh is part of the awaited transition instead of racing it. That also closes two guideline breaches the routes carried: mutations going through API endpoints rather than actions, and fetch() without a timeout. Errors now render inline through ActionErrorNote instead of only passing by as a toast. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BVmU3hKdNyzNPxFQSFJ8oe
The actions menu no longer disappears mid-sequence now that the settings record mutations revalidate inside their action, so opening one is a click rather than a loop that reopened the popover on every attempt. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BVmU3hKdNyzNPxFQSFJ8oe
Removing the retry loop around opening a row action dropped the guard it was wrapped around, which was doing separate work: the actions menu stays open behind the modal it opened, so by the delete step the action button is already showing. Clicking the trigger again toggled the menu shut and detached the button before the click landed. Whether a re-render had closed the menu first decided which way it went, so the suite passed locally and failed on CI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BVmU3hKdNyzNPxFQSFJ8oe
The participation tile adopted the stored comment whenever it changed, which is right after a fresh sign-up but wrong at any other time: those re-renders arrive whenever anything revalidates the event, and the adopt overwrote whatever the user had typed since. The tile was controlled precisely to survive such refreshes, so the adopt reintroduced the very clobber it was guarding against. It now only adopts while the draft still matches the last value taken — an unsaved edit wins, including over a comment stored elsewhere in the meantime. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BVmU3hKdNyzNPxFQSFJ8oe
The server offered nothing an orchestrator could probe — an open port only means the socket is bound, not that requests are being served — and it exited on a signal without persisting anything, so a deploy or a container restart dropped up to the store debounce (10s) worth of edits. GET /health answers once the server is serving. SIGTERM and SIGINT now go through Hocuspocus' destroy(), which closes the connections, flushes the pending stores and resolves only once every document has unloaded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BVmU3hKdNyzNPxFQSFJ8oe
Waiting for a listening port let the first request reach a server that was not serving yet, which the replace test papered over by retrying the whole call. It now waits for /health and posts once, with the timeout that fetch was missing. The truncate deadlock retry and the separate collab-http project stay: they cover the previous test's teardown writes racing the next test's TRUNCATE inside a still-running container, which shutdown behaviour does not touch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BVmU3hKdNyzNPxFQSFJ8oe
|
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
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.
An audit of the Playwright suite turned up 17 recurring workarounds. This PR
fixes the app-side causes of the cheapest eight of them and deletes the
workarounds they were paying for.
Four of the six changes are user-facing bug fixes, not test plumbing.
App changes
Popovers had no accessible name. Base UI renders
Popover.Popupwithrole="dialog"and takes its name from aPopover.Title, whichPopoverBaseUInever rendered — all 19 popovers announced as an unnameddialog.
titleis now a required prop so a new popover cannot ship withoutone. The icon-only
Actionstrigger was nameless too and now passestriggerTitle.Checkboxes that hide their label had no name either.
YesNoCheckboxrenders an
sr-onlyinput, andhideLabeldrops the Ja/Nein text — whichwas the state, never a name. The notification matrix and the SINcome table
offered five nameless checkboxes per row; both now pass an
aria-label.The other
hideLabelcallers already carry an own<label htmlFor>.Clicking a hover-opening popover trigger closed it. Moving the pointer
onto the Apps or Create button already opens the menu after the 50 ms hover
delay, and the click that instinctively follows toggled it straight back
closed. Touch users, who get no hover at all, only had that click. The first
trigger press after a hover-open is now swallowed; pressing again closes.
The inline editor's wrapper took its own clicks.
EditableFieldwraps aflex form and a flex button in a
<span>left atdisplay: inline, so thespan won the hit test for clicks aimed at the button. The tests had been
clicking both buttons with
force: true, which also hid any real overlayregression from them.
The Unleash cache window is configurable.
getUnleashFlagcached thedefinitions for a hardcoded 30 s with no way to shorten it, so the flag tests
polled for up to 90 s per state inside a 5 min timeout.
UNLEASH_REVALIDATE_SECONDSis optional and defaults to the same 30 s;the test stack sets it to one second.
Test changes
All of these are consequences of the above, not new test logic:
by filtering on body text
modal()copy iniam.spec.tsis gone, along with itsduplicated
ACTION_FEEDBACK_TIMEOUT— its comment still announced the BaseUI rewrite as "planned"
openInlineEditoris deleted: without the forced click it wasclickUntilVisibleDeliberately not in this PR
because nothing tells a test the page is interactive. It is the biggest
single win in the audit and wants its own PR.
variant-wiki.spec.tspressing Enter instead of clicking is not confirmed;the document scroller already carries
scroll-padding-top: 122px, whichshould be enough, so the fix would be a guess.
router.refresh()unmounting thespynet actions popover, the event comment draft being re-seeded from props,
and the collab server's missing health route and drain. Each needs a design
call rather than a patch.
Popover(4 call sites) still renders unnameddialogs. It is slated for removal, so it was left alone.
🤖 Generated with Claude Code
https://claude.ai/code/session_01BVmU3hKdNyzNPxFQSFJ8oe