-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[A11y] Labels & Descriptions #79146
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
[A11y] Labels & Descriptions #79146
Changes from all commits
ede9130
bc9f355
693ba69
0bf4236
59ade3a
7a438e6
1fad90a
ee2101a
ad55fec
bb3fc54
d14ab85
26493ec
2189785
19d7817
a6c2f98
14563d8
b69e952
3618fd0
d0d7b9b
fe5e14e
2780040
ea723ef
3063a47
94d97d8
a46fb17
0fdf294
9a4503a
e13a406
e7e6989
3d41093
c535cb1
b935bc7
903707b
3c0154d
9f42753
5782675
3b84961
eaeb5c8
d899cfc
0dffcce
fd1137a
26ca8fd
c8fd632
e6daef7
cadf0b4
ac9e969
c9ba2df
4569ab6
0371de8
546669f
fdecaa0
f5d9e6d
879ddb5
95cf28c
432702b
c75907b
7dd7b6b
41d3a66
4ea9c49
008c4fd
4d880a2
7e52a38
65ff1ed
12656b2
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 | ||||
|---|---|---|---|---|---|---|
|
|
@@ -7,7 +7,7 @@ | |||||
| import Checkbox from '@components/Checkbox'; | ||||||
| import FormHelpMessage from '@components/FormHelpMessage'; | ||||||
| import Icon from '@components/Icon'; | ||||||
| import * as Expensicons from '@components/Icon/Expensicons'; | ||||||
|
Check warning on line 10 in src/components/TextInput/BaseTextInput/implementation/index.native.tsx
|
||||||
| import PressableWithoutFeedback from '@components/Pressable/PressableWithoutFeedback'; | ||||||
| import type {AnimatedMarkdownTextInputRef} from '@components/RNMarkdownTextInput'; | ||||||
| import type {AnimatedTextInputRef} from '@components/RNTextInput'; | ||||||
|
|
@@ -280,6 +280,7 @@ | |||||
|
|
||||||
| // Height fix is needed only for Text single line inputs | ||||||
| const shouldApplyHeight = !shouldUseFullInputHeight && !isMultiline && !isMarkdownEnabled; | ||||||
| const accessibilityLabel = [label, hint].filter(Boolean).join(', '); | ||||||
|
Contributor
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.
Suggested change
Can we create a shared utility function for this? For example: function buildAccessibilityLabel(...parts: Array<string | undefined>) {
return parts.filter(Boolean).join(', ');
}I see this logic is used in two places with the same implementation, so it would be better to extract it into a utility function.
Member
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. i don't think we should. this function is clearer without it |
||||||
|
|
||||||
| return ( | ||||||
| <> | ||||||
|
|
@@ -291,7 +292,7 @@ | |||||
| // When autoGrowHeight is true we calculate the width for the text input, so it will break lines properly | ||||||
| // or if multiline is not supplied we calculate the text input height, using onLayout. | ||||||
| onLayout={onLayout} | ||||||
| accessibilityLabel={label} | ||||||
| accessibilityLabel={accessibilityLabel} | ||||||
| style={[ | ||||||
| autoGrowHeight && | ||||||
| !isAutoGrowHeightMarkdown && | ||||||
|
|
@@ -367,6 +368,7 @@ | |||||
| }} | ||||||
| // eslint-disable-next-line | ||||||
| {...inputProps} | ||||||
| accessibilityLabel={inputProps.accessibilityLabel ?? accessibilityLabel} | ||||||
| autoCorrect={inputProps.secureTextEntry ? false : autoCorrect} | ||||||
| placeholder={placeholderValue} | ||||||
| placeholderTextColor={placeholderTextColor ?? theme.placeholderText} | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -289,6 +289,7 @@ function BaseTextInput({ | |
| // This is workaround for https://github.com/Expensify/App/issues/47939: in case when user is using Chrome on Android we set inputMode to 'search' to disable autocomplete bar above the keyboard. | ||
| // If we need some other inputMode (eg. 'decimal'), then the autocomplete bar will show, but we can do nothing about it as it's a known Chrome bug. | ||
| const inputMode = inputProps.inputMode ?? (isMobileChrome() ? 'search' : undefined); | ||
| const accessibilityLabel = [label, hint].filter(Boolean).join(', '); | ||
|
Contributor
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. |
||
| return ( | ||
| <> | ||
| <View | ||
|
|
@@ -300,7 +301,7 @@ function BaseTextInput({ | |
| role={CONST.ROLE.PRESENTATION} | ||
| onPress={onPress} | ||
| tabIndex={-1} | ||
| accessibilityLabel={label} | ||
| accessibilityLabel={accessibilityLabel} | ||
| // When autoGrowHeight is true we calculate the width for the text input, so it will break lines properly | ||
| // or if multiline is not supplied we calculate the text input height, using onLayout. | ||
| onLayout={onLayout} | ||
|
|
@@ -447,6 +448,7 @@ function BaseTextInput({ | |
| readOnly={isReadOnly} | ||
| defaultValue={defaultValue} | ||
| markdownStyle={markdownStyle} | ||
| accessibilityLabel={inputProps.accessibilityLabel} | ||
| /> | ||
| {!!suffixCharacter && ( | ||
| <View style={[styles.textInputSuffixWrapper, suffixContainerStyle]}> | ||
|
|
||
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.
This is not really the same format as the year (
January, current monthinstead of2026, please select a year). I think we should probably be consistent.Generally - have we had marketing take a glance at these? i think it would be good since it's effectively marketing copy.
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.
made it consistent -
2026, current year