-
Notifications
You must be signed in to change notification settings - Fork 747
textField - a11y label improvements #3883
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…seMemo hook usage
lidord-wix
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some comments
BTW, the decision of the reading order is made by the UX?
Co-authored-by: Lidor Dafna <66782556+lidord-wix@users.noreply.github.com>
that was discussed with Alexey and Naama and that's what Alexey said in the end |
|
I've created a PR that wraps this public with i18n I'm assuming this current PR would need to be pushed first. |
lidord-wix
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with a small comment, once fixed - feel free to merge
| return parts.join(', '); | ||
| }, [label, context.isMandatory, helperText, placeholder, showCharCounter, others.maxLength]); | ||
|
|
||
| const accessibilityLabel = props.accessibilityLabel ?? defaultAccessibilityLabel; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can extract the prop like all other props instead of using props.
Description
textField - added better handling of textField a11y label. Added unification of all different parts of the component (Label, helperText, charCounter), ordered by a11y guidelines given.
Changelog
textField - a11y label fix.
Additional info
Ticket 4797