fix(AddressDisplay, e2e): complete clipboard a11y and harden the wallet E2E suite (#586, #571) - #642
Merged
Just-Bamford merged 1 commit intoSep 1, 2026
Conversation
…et E2E suite Closes the gaps left by the initial fixes for Sorokit#586 (b0c0165) and Sorokit#571 (59bb51f). AddressDisplay (Sorokit#586): - Clear the copied/failed reset timeout on unmount. clearResetTimer() only ran at the start of the next copy, so unmounting inside the reset window left a pending timeout that fired setState on an unmounted component. - Announce the copy result through an sr-only role="status" live region. The result was conveyed only by an icon swap and a mutating aria-label, neither of which announces reliably, so "the copy silently did nothing" still held for screen-reader users - the exact complaint the issue raised. Mirrors the live region already used in NetworkSwitcher.tsx. - Harden the execCommand fallback: mark the throwaway textarea readOnly so mobile browsers do not raise the on-screen keyboard, and capture/restore the user's existing selection, which textarea.select() otherwise destroys. E2E suite (Sorokit#571): - Add playwright.config.ts and e2e/**/*.ts to tsconfig.node.json. Neither tsconfig covered the E2E directory, so npm run typecheck skipped the whole Playwright suite and breakage surfaced only in the slow CI job. - Wrap each of the four acceptance-criteria behaviors in test.step() so a failure is attributed to the specific behavior that broke instead of aborting one flat block. Tests: 6 new cases covering the announcements, the unmount cleanup, and the fallback's readOnly/selection-restore behavior. AddressDisplay.test.tsx is 30 passed. Typecheck reports the same 3 pre-existing vite-config errors as main - no new ones. eslint clean on all changed files. Closes Sorokit#586 Closes Sorokit#571
|
@Sam-Rytech Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
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.
Summary
This PR closes the remaining gaps in the clipboard-copy behavior of
AddressDisplay(#586) and in the wallet-flow E2E suite (#571).The headline fixes for both issues are in place on
main—b0c0165added theexecCommandfallback and the copy button'saria-label, and59bb51f(#637) added Playwright,playwright.config.ts,e2e/wallet-flow.spec.ts, and thetest:e2escript. Working through both issues end-to-end surfaced five defects those changes left behind, all inside the same code paths. This PR fixes them.#586 — AddressDisplay
1. The reset timer leaked past unmount
clearResetTimer()was only called at the start of the next copy. Unmounting inside the 2s "Copied!" window (or the 1.5s failure window) left a pendingsetTimeoutthat firedsetCopied(false)on an unmounted component.Added a
useEffectcleanup that clears the pending timeout on unmount.2. The copy result was silent for screen readers
This is the other half of the issue's accessibility complaint. The
aria-labelfix means the button is now labelled, but the copy result was still conveyed only by an icon swap and a mutatingaria-label— neither of which is reliably announced by assistive technology. So the original complaint, "users get no feedback that the copy failed", still held for screen-reader users in exactly the non-secure contexts the issue describes.Added an
sr-onlylive region that announces both outcomes:This mirrors the pattern already used in
NetworkSwitcher.tsx:479, so it stays consistent with the codebase.3. The
execCommandfallback had two side effects<textarea>is focused via.select()without beingreadOnly, so mobile browsers pop the keyboard for a fraction of a second. Now setreadOnly(andaria-hidden, since it is never meant to be perceivable).textarea.select()replaces whatever the user currently has highlighted on the page, and nothing put it back. The priorRangeis now captured before the copy and restored in thefinallyblock.#571 — E2E suite
4.
e2e/andplaywright.config.tswere never type-checkedtsconfig.app.jsonincludes onlysrc, andtsconfig.node.jsononlyvite.config.tsandvite.lib.config.ts. Neither covers the E2E directory, sonpm run typechecksilently skipped the entire Playwright suite — a type error or a renamed selector API in a spec would only surface when the slow Playwright job ran in CI, if at all.Added
playwright.config.ts,e2e/**/*.ts, andvitest.config.tstotsconfig.node.json'sinclude.5. Four acceptance criteria were asserted as one flat block
The spec covers four distinct behaviors from the issue, but a failure at "connect wallet" aborted the test and told you nothing about whether history rendering and send-payment still worked.
Wrapped each behavior in
test.step(), so the reporter attributes a failure to the specific behavior that broke. This keeps one browser session and one continuous user journey — the steps are ordered and stateful by design — while making the report per-criterion.Testing
Added 6 unit tests to
src/components/AddressDisplay.test.tsx:aria-live="polite""Address copied to clipboard"on success"Failed to copy address"when both copy paths failreadOnlyduring the copy and is removed afterwardsexecCommandfallbackNo existing test was modified, and no production behavior changed beyond the five fixes above.
Closes #586
Closes #571