LG-7771 Add correct screenreader tag for incomplete steps#7613
LG-7771 Add correct screenreader tag for incomplete steps#7613soniaconnolly merged 1 commit intomainfrom
Conversation
Co-authored-by: Eric Gade <eric.gade@gsa.gov> changelog: User-Facing Improvements, Accessibility, Step indicator screen reader tags
aduth
left a comment
aduth
left a comment
There was a problem hiding this comment.
Good fix! Small note about test coverage, but otherwise LGTM. 👍
I also checked to confirm that this only appears to affect the JavaScript implementation, and the equivalent Rails View Component is working correctly.
|
|
||
| const title = getByText('Step'); | ||
| const status = getByText('step_indicator.status.current'); | ||
| const status = getByText('step_indicator.status.not_complete'); |
There was a problem hiding this comment.
Can we add a test case for the "current step" context?
There was a problem hiding this comment.
Is there something you want covered in addition to lines 5-20 in this file?
There was a problem hiding this comment.
Is there something you want covered in addition to lines 5-20 in this file?
Oh, good point! I totally missed we had test coverage for StepStatus.CURRENT already. I suppose it could be nice to have all of them covered (PENDING seems to be missing), but that's not in the scope of your work, so I wouldn't consider it a blocker.
There was a problem hiding this comment.
We were wondering about PENDING, and if it was really used. I'll go ahead and merge this, but there might be additional cleanup. Do you know what PENDING is for?
There was a problem hiding this comment.
It was originally used for marking the GPO letter (verify by mail) step as pending, but I seem to recall there may have been some recent revisions to the flow where we no longer show the pending state. Let me dig that up, since it would be nice if we could drop the extra logic (I had a hard time re-wrapping my head around the differences between "incomplete", "current", and "pending").
There was a problem hiding this comment.
It looks like the pending status was meant to have been removed as part of #6860. I can follow-up to clean up the lingering references, since I'm to blame for not having done it in the first place 😅
|
|
||
| const title = getByText('Step'); | ||
| const status = getByText('step_indicator.status.current'); | ||
| const status = getByText('step_indicator.status.not_complete'); |
There was a problem hiding this comment.
Is there something you want covered in addition to lines 5-20 in this file?
Oh, good point! I totally missed we had test coverage for StepStatus.CURRENT already. I suppose it could be nice to have all of them covered (PENDING seems to be missing), but that's not in the scope of your work, so I wouldn't consider it a blocker.
3 participants
🎫 Ticket
LG-7771
🛠 Summary of changes
Add INCOMPLETE StepStatus to StepIndicatorStep custom component and set statusText to step_indicator.status.not_complete
📜 Testing Plan
Provide a checklist of steps to confirm the changes.