test(#722): guard keyboard accessibility of the sell key modal - #741
Merged
Chucks1093 merged 1 commit intoAug 1, 2026
Conversation
… sell key modal TradeDialog is shared between the buy and sell flows; accesslayerorg#700 already implemented and tested keyboard accessibility for the buy modal (initial focus on the amount input, focus trapping, Escape-to-close, Enter/Space activation of Confirm, focus return to the trigger). The sell variant shares that implementation but had no coverage guarding it. Add a sell-variant block to the TradeDialog a11y suite that verifies all five acceptance criteria from accesslayerorg#722 against the real controls, plus an end-to-end LandingPage test proving the actual Sell button regains focus after the modal closes with Escape.
|
@OpensrcLord 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
3 participants
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 #722
The sell key modal (
TradeDialog, shared by the buy and sell flows) is opened fromLandingPagevia externalopen/onOpenChangeprops rather than Radix's own<DialogTrigger>. Keyboard accessibility was implemented for the buy modal in #700 (initial focus on the quantity input, focus trapping, Escape-to-close, Enter/Space activation of Confirm, and focus return to the actual trigger). SinceTradeDialogis a single shared component, the sell modal already inherits that behavior — but no tests guarded the sell variant, so a regression in the shared wiring would go unnoticed.This PR adds coverage that verifies all five acceptance criteria from #722 hold for the sell modal:
Changes
src/components/common/__tests__/TradeDialog.a11y.test.tsx— parameterized the trigger helper bysideand added asell key modal keyboard accessibility (#722)block with 8 tests covering every acceptance criterion against the real controls.src/pages/__tests__/LandingPage.sellFlow.integration.test.tsx— added an end-to-end test that opens the real sell modal from the actual Sell button, asserts focus lands on the amount input, closes with Escape, and asserts focus returns to the Sell button.Verification
TradeDialog.a11y.test.tsx: 17/17 pass (9 buy + 8 sell)LandingPage.sellFlow.integration.test.tsx: 4/4 passeslintandtsc -b --noEmitclean on changed filesPre-existing failures on
devinLandingPage.sellConfirmationModal.integration.test.tsxandTradeDialog.focusOrder.test.tsx(stalenetworkFeeEstimateProviderprop tests) are unrelated to this change and were confirmed failing before it.