fix(frontend): the reduced-motion preference reaches the page - #1149
Closed
ExtraToast wants to merge 1 commit into
Closed
fix(frontend): the reduced-motion preference reaches the page#1149ExtraToast wants to merge 1 commit into
ExtraToast wants to merge 1 commit into
Conversation
`use.reducedMotion` was ignored by the runner-managed context on every Playwright from 1.55 to 1.62, so every project but the motion one believed it ran as a visitor who asked for reduced motion and in fact ran with the full choreography playing. Upstream fixed it in 1.63 (microsoft/playwright#42001), so the fix is the upgrade rather than emulating the preference ourselves. The dependabot hold on `>= 1.61` cited Yarn PnP, which this package left for the node-modules linker, so it goes with it. Two specs emulated the preference by hand because of this and no longer need to. The four in the motion project keep theirs: that project asks for no preference on purpose, so a reduced-motion test living there has to say so, and their comments now give that reason rather than pointing at #852. Closes #852
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
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.
Why
playwright.config.tsasks every project but the motion one to run as a visitor who asked forreduced motion, and says so in a comment. That never happened. The option was ignored by the
runner-managed context, so the reduced-motion path had never been exercised by an end-to-end
test, in any project, and the suites that were meant to be deterministic ran with the full
choreography playing.
closes #852
What this achieves
The preference reaches the page, so the comment is true and the reduced-motion path is exercised
by every test rather than by none. The suite also runs in roughly half the time, which is the
choreography no longer playing.
How
An upgrade, not a workaround. The bug is upstream and long-standing rather than a regression:
probed across releases, the preference fails to reach the page on 1.55.1, 1.56.1, 1.57.0, 1.58.2,
1.59.1, 1.60.0, 1.61.1 and 1.62.1, and works on 1.63.0 — microsoft/playwright#42001, closed on
30 July, milestone v1.63. Emulating it ourselves would have put a workaround in every spec for a
bug the vendor has already fixed, and left the config's comment still lying.
The dependabot hold at
>= 1.61goes with it. It cited Yarn PnP, which this package left for thenode-modules linker.
Not in scope
The system tests pin
com.microsoft.playwright:playwrightseparately, with their own driver andbrowsers, and never set this option — untouched. CI installs browsers from the lockfile, so there
is no second version to bump.
Worth a reviewer's attention
Merge #1148 first. This branch is stacked on it. With that change reverted and this upgrade in
place,
boards-swipe.motion.spec.ts:176fails 3 times out of 3 onReceived: 0— the newerChromium exposes a pre-existing race in a test helper, which #1148 fixes on its own terms.
Two
emulateMediacalls went; four stayed, and their comments were wrong. The four in themotion project carried comments saying they should be deleted once this issue was fixed. Doing
that would have deleted the coverage: that project asks for no preference on purpose, so a
reduced-motion case living there has to set the preference itself. Their comments now give that
reason. The two that genuinely were working around this bug — in
esports-season-on-showandmembership— are gone.A handful of other
#852mentions elsewhere in the specs are now stale, including one thatrepeats the same wrong instruction. Left alone here rather than widened into unrelated files; worth
a follow-up.
Nothing else broke. Making the preference real changes the whole suite's behaviour at once, so
the fallout was measured rather than assumed: 820 passed before, 820 passed after, zero specs
needed changing.
Verification
Full
yarn test:e2estacked on #1148: exit 0, 820 passed, 0 failed, 36 skipped, down from 4.7minutes to 2.8.
yarn typecheck,yarn lintandyarn test:unit(1540 tests) all exit 0.Confirmed after the upgrade that
testInfo.project.use.reducedMotionand the page now agree:resolved=reduce inPage=true, in both chromium and mobile-chrome.Diff breakdown —
█added░removed, scaled to the largest row.