test(ui): add EditCampaignMetadata tests with accessible label associ… - #1016
test(ui): add EditCampaignMetadata tests with accessible label associ…#1016damilareojetunde wants to merge 5 commits into
Conversation
|
@damilareojetunde 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! 🚀 |
|
Auto-review failed (API error). Leaving PR for human review. |
1 similar comment
|
Auto-review failed (API error). Leaving PR for human review. |
davidmaronio
left a comment
davidmaronio
left a comment
There was a problem hiding this comment.
the actual component and test work here is good: label/input association via htmlFor + ids in EditCampaignMetadata, image url validation with aria-invalid/role="alert" errors, and the focus trap + escape handling in InstallFreighterModal are all solid accessibility wins. two blockers:
- pnpm-lock.yaml: ~6k lines of lockfile churn with no package.json change in this PR. a test/accessibility PR should not regenerate the lockfile; please drop the pnpm-lock.yaml changes from the branch entirely.
- branch is in conflict with main and needs a rebase.
with the lockfile removed and a clean rebase this is an easy approve.
…ations - Add htmlFor/id associations to labels/inputs for accessible names - Add image URL validation with inline error display - Add 9 comprehensive tests covering toggle, validation, save, clear, error resilience Closes Iris-IV#801
…FreighterModal - Save and restore focus on modal open/close (Iris-IV#807) - Add Escape key handler to close modal - Add focus trap with Tab/Shift+Tab cycling - Add 8 tests covering focus, keyboard, and retry behavior
Fixes the single ESLint error (unused-imports/no-unused-imports) that fails the CI lint job.
Formatting-only changes required for the repo-wide prettier --check CI job to pass.
b45c0f8 to
b1cdafd
Compare
|
Auto-review failed (API error). Leaving PR for human review. |
HI Chief, this has been worked on, kindly review. |
|
this is verified and approved, but today's merge wave put the branch into conflict before it reached the front of the queue (lots of prs touching the same i18n and test files landed at once). please rebase onto current main and push, i'll merge as soon as it's green, no re-review needed. |
|
Auto-review failed (API error). Leaving PR for human review. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| setCoverImageUrl(initialCoverImageUrl); | ||
| }; | ||
|
|
||
| const titleId = `edit-title-${campaignId}`; |
| }; | ||
|
|
||
| const titleId = `edit-title-${campaignId}`; | ||
| const descriptionId = `edit-description-${campaignId}`; |
|
|
||
| const titleId = `edit-title-${campaignId}`; | ||
| const descriptionId = `edit-description-${campaignId}`; | ||
| const coverImageId = `edit-cover-image-url-${campaignId}`; |
|
the fork runs came back with real failures on this head: lint and the production build are red (main is green after #867), so this needs a rebase onto current main plus whatever the lint log points at in your own files. i'll merge once it's green, the approval stands. |
5 participants
closes #801
closes #807