feat(accessibility): add isDescribedBy flag to FormContainer and update global accessibility values #162
Conversation
|
|
||
| it('applies container props to Message', () => { | ||
| expect(wrapper.find(Message)).toHaveProp('id', `${CHECKBOX_ID}--message`); | ||
| expect(wrapper.find(Message)).toHaveProp('role', 'alert'); |
There was a problem hiding this comment.
role alert will interrupt the screen reader and read that message when it appears is that what we want?
There was a problem hiding this comment.
From the MDN ARIA techniques docj it mentions a use-case
- An invalid value was entered into a form field
We current use the getMessageProps value on all of our success/warning/error messages so I thought it would be applicable as we normally only show this dynamically based on input states.
There was a problem hiding this comment.
But that isn't always the case right? I can not supply a prop and use it to just display ancillary message that may not be validation related.
There was a problem hiding this comment.
But that isn't always the case right?
True. I forgot about our default <Message />.
If I were to remove the role prop that would remove the need for getMessageProps entirely! It would be a breaking change, but would clean up the API some.
There was a problem hiding this comment.
Could you do some logic there that if there is no validation prop or it's value is none don't apply the role alert?
There was a problem hiding this comment.
We could conditionally apply the getMessageProps based on that value. Or would that be too abstract?
There was a problem hiding this comment.
Hmm not sure I think consumers would be used to using propGetter pattern.
There was a problem hiding this comment.
Good point. I think it would be best to start the deprecation process on this one. In the most recent commit I've:
- Added a deprecation warning to
getMessageProps - Updated testing to cover deprecation warning
- Remove all internal uses of
getMessagePropsin the form related components.
austingreendev
force-pushed
the
austin94/accessible-forms
branch
from
452e527 to
fbbe2bc
Compare
No breaking changes with this one 🎉 Wraps up #66
Everything should be backwards compatible and act as a single
featrelease.Description
This PR closes up the remaining work in #66 and removes the last of the AXE auditing failures.
Detail
This PR includes assorted DOC changes, aria-labels for examples, and some wording tweaks, but the majority of changes are:
AutocompleteContaineraria-ownsvalueMenuContaineraria-controlsvalueFieldContaineraria-describedbyvalues based on an optional customizationCheckbox | Radio | Ranges | Textfields | TogglesFieldContainerwith updatesChecklist
designer as a reviewer)
component
yarn start)src/index.jsexport