-
Notifications
You must be signed in to change notification settings - Fork 748
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
base: master
Are you sure you want to change the base?
Changes from all commits
1e5dce7
4026949
4a42af2
dd6ffe9
aaf7873
bc133c7
9603ac8
87b2c70
d9b71a9
65b7afd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -138,9 +138,35 @@ const TextField = (props: InternalTextFieldProps) => { | |
| [typographyStyle, colorStyle, others.style, centeredTextStyle, hasValue]); | ||
| const dummyPlaceholderStyle = useMemo(() => [inputStyle, styles.dummyPlaceholder], [inputStyle]); | ||
|
|
||
| const accessibilityLabel = useMemo(() => { | ||
| const parts: string[] = []; | ||
|
|
||
| if (label) { | ||
| parts.push(label); | ||
| } | ||
|
|
||
| if (context.isMandatory) { | ||
| parts.push('required'); | ||
| } | ||
|
|
||
| parts.push('textField'); | ||
|
|
||
| if (helperText) { | ||
| parts.push(helperText); | ||
| } else if (placeholder) { | ||
| parts.push(placeholder); | ||
| } | ||
|
|
||
| if (showCharCounter && others.maxLength) { | ||
| parts.push(`you can enter up to ${others.maxLength} characters`); | ||
| } | ||
|
|
||
| return parts.join(', '); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like it will work, for English :)
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should I use i18n? Do I need to change something with the reading order of the label in other languages? |
||
| }, [label, context.isMandatory, helperText, placeholder, showCharCounter, others.maxLength]); | ||
|
|
||
| return ( | ||
| <FieldContext.Provider value={context}> | ||
| <View {...containerProps} style={[margins, positionStyle, containerStyle, centeredContainerStyle]}> | ||
| <View {...containerProps} accessible accessibilityLabel={accessibilityLabel} style={[margins, positionStyle, containerStyle, centeredContainerStyle]}> | ||
| <View row spread style={centeredContainerStyle}> | ||
| <Label | ||
| label={label} | ||
|
|
||
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.
Have we decided to include the placeholder? it can also be some weird example..
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.
That's what Alexey said in the case helper text is empty :|