Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ function BaseTextInput({
iconContainerStyle,
shouldUseDefaultLineHeightForPrefix = true,
ref,
sentryLabel,
...props
}: BaseTextInputProps) {
const InputComponent = InputComponentMap.get(type) ?? RNTextInput;
Expand Down Expand Up @@ -289,10 +290,11 @@ function BaseTextInput({
role={CONST.ROLE.PRESENTATION}
onPress={onPress}
tabIndex={-1}
accessible={false}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we setting accessible = false on our base text input?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@roryabraham Thanks for calling this out.
We set accessible={false} on the PressableWithoutFeedback wrapper because that wrapper is only a touch/focus helper, not the semantic form control.
In #77329, TalkBack was focusing this wrapper first (it had an accessibilityLabel), then focusing the real input (InputComponent), which caused duplicate announcements.
Making the wrapper non-accessible (and removing its label) leaves only the real TextInput in the accessibility tree, which resolves the duplicate focus stop.

sentryLabel={sentryLabel}
// 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={accessibilityLabel}
style={[
autoGrowHeight &&
!isAutoGrowHeightMarkdown &&
Expand Down
24 changes: 16 additions & 8 deletions src/components/TextInput/BaseTextInput/implementation/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -297,11 +297,7 @@ function BaseTextInput({
// eslint-disable-next-line react/jsx-props-no-spreading
{...(shouldInterceptSwipe && SwipeInterceptPanResponder.panHandlers)}
>
<PressableWithoutFeedback
role={CONST.ROLE.PRESENTATION}
onPress={onPress}
tabIndex={-1}
accessibilityLabel={accessibilityLabel}
<View
// 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}
Expand All @@ -317,9 +313,20 @@ function BaseTextInput({
!isMultiline && styles.componentHeightLarge,
touchableInputWrapperStyle,
]}
sentryLabel={sentryLabel}
>
<PressableWithoutFeedback
role={CONST.ROLE.PRESENTATION}
onPress={onPress}
tabIndex={-1}
accessible={false}
accessibilityElementsHidden
importantForAccessibility="no"
aria-hidden
style={[StyleSheet.absoluteFillObject]}
sentryLabel={sentryLabel}
/>
<View
pointerEvents="box-none"
style={[
newTextInputContainerStyles,

Expand Down Expand Up @@ -448,7 +455,8 @@ function BaseTextInput({
readOnly={isReadOnly}
defaultValue={defaultValue}
markdownStyle={markdownStyle}
accessibilityLabel={inputProps.accessibilityLabel}
accessibilityLabel={inputProps.accessibilityLabel ?? accessibilityLabel}
keyboardType={inputProps.keyboardType}
/>
{!!suffixCharacter && (
<View style={[styles.textInputSuffixWrapper, suffixContainerStyle]}>
Expand Down Expand Up @@ -518,7 +526,7 @@ function BaseTextInput({
)}
</View>
</View>
</PressableWithoutFeedback>
</View>
{!!inputHelpText && (
<FormHelpMessage
isError={!!errorText}
Expand Down
3 changes: 3 additions & 0 deletions src/components/TextInput/TextInputLabel/index.native.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ function TextInputLabel({label, labelScale, labelTranslateY, isMultiline}: TextI
return (
<Animated.View style={[styles.textInputLabelContainer, animatedStyle]}>
<Animated.Text
accessible={false}
accessibilityElementsHidden
importantForAccessibility="no"
numberOfLines={!isMultiline ? 1 : undefined}
ellipsizeMode={!isMultiline ? 'tail' : undefined}
allowFontScaling={false}
Expand Down
4 changes: 4 additions & 0 deletions src/components/TextInput/TextInputLabel/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ function TextInputLabel({for: inputId = '', label, labelTranslateY, labelScale,
ellipsizeMode={!isMultiline ? 'tail' : undefined}
ref={textRef(labelRef)}
role={CONST.ROLE.PRESENTATION}
accessible={false}
accessibilityElementsHidden
importantForAccessibility="no"
aria-hidden
style={[styles.textInputLabelContainer, styles.textInputLabel, animatedStyle, styles.pointerEventsNone]}
>
{label}
Expand Down
12 changes: 12 additions & 0 deletions src/components/TextInput/TextInputMeasurement/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,21 @@ function TextInputMeasurement({
onSetTextInputWidth(e.nativeEvent.layout.width);
onSetTextInputHeight(e.nativeEvent.layout.height);
}}
accessible={false}
accessibilityElementsHidden
importantForAccessibility="no"
aria-hidden
>
<Text
style={[
inputStyle,
autoGrowHeight && styles.autoGrowHeightHiddenInput(width ?? 0, typeof maxAutoGrowHeight === 'number' ? maxAutoGrowHeight : undefined),
{width: contentWidth},
]}
accessible={false}
accessibilityElementsHidden
importantForAccessibility="no"
aria-hidden
>
{/* \u200B added to solve the issue of not expanding the text input enough when the value ends with '\n' (https://github.com/Expensify/App/issues/21271) */}
{value ? `${value}${value.endsWith('\n') ? '\u200B' : ''}` : placeholder}
Expand All @@ -66,6 +74,10 @@ function TextInputMeasurement({
styles.hiddenElementOutsideOfWindow,
styles.visibilityHidden,
]}
accessible={false}
accessibilityElementsHidden
importantForAccessibility="no"
aria-hidden
onLayout={(e) => {
if (e.nativeEvent.layout.width === 0 && e.nativeEvent.layout.height === 0) {
return;
Expand Down
49 changes: 21 additions & 28 deletions src/pages/signin/SignInPageLayout/Footer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import SignInGradient from '@assets/images/home-fade-gradient--mobile.svg';
import Hoverable from '@components/Hoverable';
import ImageSVG from '@components/ImageSVG';
import Text from '@components/Text';
import type {LinkProps, PressProps} from '@components/TextLink';
import TextLink from '@components/TextLink';
import {useMemoizedLazyExpensifyIcons} from '@hooks/useLazyAsset';
import useLocalize from '@hooks/useLocalize';
import useResponsiveLayout from '@hooks/useResponsiveLayout';
Expand All @@ -18,14 +16,11 @@ import Socials from '@pages/signin/Socials';
import variables from '@styles/variables';
import CONST from '@src/CONST';
import type {TranslationPaths} from '@src/languages/types';
import type {SignInPageLayoutProps} from './types';
import FooterRow from './FooterRow';
import type {FooterColumnRow, SignInPageLayoutProps} from './types';

type FooterProps = Pick<SignInPageLayoutProps, 'navigateFocus'>;

type FooterColumnRow = (LinkProps | PressProps) & {
translationPath: TranslationPaths;
};

type FooterColumnData = {
translationPath: TranslationPaths;
rows: FooterColumnRow[];
Expand Down Expand Up @@ -178,27 +173,25 @@ function Footer({navigateFocus}: FooterProps) {
>
<Text style={[styles.textHeadline, styles.footerTitle]}>{translate(column.translationPath)}</Text>
<View style={[styles.footerRow]}>
{column.rows.map(({href, onPress, translationPath}) => (
<Hoverable key={translationPath}>
{(hovered) => (
<View>
{onPress ? (
<TextLink
style={getTextLinkStyle(hovered)}
onPress={onPress}
>
{translate(translationPath)}
</TextLink>
) : (
<TextLink
style={getTextLinkStyle(hovered)}
href={href}
>
{translate(translationPath)}
</TextLink>
)}
</View>
)}
{column.rows.map((row) => (
<Hoverable key={row.translationPath}>
{(hovered) =>
row.onPress ? (
<FooterRow
onPress={row.onPress}
translationPath={row.translationPath}
text={translate(row.translationPath)}
style={getTextLinkStyle(hovered)}
/>
) : (
<FooterRow
href={row.href}
translationPath={row.translationPath}
text={translate(row.translationPath)}
style={getTextLinkStyle(hovered)}
/>
)
}
</Hoverable>
))}
{i === 2 && (
Expand Down
47 changes: 47 additions & 0 deletions src/pages/signin/SignInPageLayout/FooterRow/index.native.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import React from 'react';
import type {GestureResponderEvent, StyleProp, TextStyle} from 'react-native';
import {PressableWithoutFeedback} from '@components/Pressable';
import Text from '@components/Text';
import useEnvironment from '@hooks/useEnvironment';
import useThemeStyles from '@hooks/useThemeStyles';
import type {FooterColumnRow} from '@pages/signin/SignInPageLayout/types';
import {openLink as openLinkUtil} from '@userActions/Link';
import CONST from '@src/CONST';

type FooterRowProps = FooterColumnRow & {
text: string;
style: StyleProp<TextStyle>;
};

function FooterRow({href, onPress, translationPath, text, style}: FooterRowProps) {
const styles = useThemeStyles();
const {environmentURL} = useEnvironment();

return (
<PressableWithoutFeedback
accessible
accessibilityRole={CONST.ROLE.LINK}
accessibilityLabel={text}
sentryLabel={translationPath}
onPress={() => {
if (onPress) {
onPress({} as GestureResponderEvent);
return;
}
if (href) {
openLinkUtil(href, environmentURL);
}
}}
>
<Text
accessible={false}
suppressHighlighting
style={[styles.link, style]}
>
{text}
</Text>
</PressableWithoutFeedback>
);
}

export default FooterRow;
33 changes: 33 additions & 0 deletions src/pages/signin/SignInPageLayout/FooterRow/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import React from 'react';
import type {StyleProp, TextStyle} from 'react-native';
import TextLink from '@components/TextLink';
import type {FooterColumnRow} from '@pages/signin/SignInPageLayout/types';

type FooterRowProps = FooterColumnRow & {
text: string;
style: StyleProp<TextStyle>;
};

function FooterRow({href, onPress, text, style}: FooterRowProps) {
if (onPress) {
return (
<TextLink
style={style}
onPress={onPress}
>
{text}
</TextLink>
);
}

return (
<TextLink
style={style}
href={href}
>
{text}
</TextLink>
);
}

export default FooterRow;
8 changes: 7 additions & 1 deletion src/pages/signin/SignInPageLayout/types.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import type React from 'react';
import type {ForwardedRef} from 'react';
import type {LinkProps, PressProps} from '@components/TextLink';
import type {TranslationPaths} from '@src/languages/types';

type SignInPageLayoutProps = {
/** The children to show inside the layout */
Expand Down Expand Up @@ -35,4 +37,8 @@ type SignInPageLayoutRef = {
scrollPageToTop: (animated?: boolean) => void;
};

export type {SignInPageLayoutRef, SignInPageLayoutProps};
type FooterColumnRow = (LinkProps | PressProps) & {
translationPath: TranslationPaths;
};

export type {SignInPageLayoutRef, SignInPageLayoutProps, FooterColumnRow};
7 changes: 4 additions & 3 deletions tests/ui/AddressPageTest.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,13 @@ describe('AddressPageTest', () => {
renderPage(SCREENS.SETTINGS.PROFILE.ADDRESS);

await waitForBatchedUpdatesWithAct();
const state = screen.getAllByLabelText('State / Province');
expect(state.at(1)?.props.value).toEqual('Test');
const stateInput = screen.getByLabelText('State / Province');
expect(stateInput.props.value).toEqual('Test');
Navigation.setParams({
country: 'VN',
});
await waitForBatchedUpdatesWithAct();
expect(state?.at(1)?.props.value).toEqual('Test');
const stateInputAfterParams = screen.getByLabelText('State / Province');
expect(stateInputAfterParams.props.value).toEqual('Test');
});
});
4 changes: 2 additions & 2 deletions tests/ui/components/TextInputLabel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ describe('TextInputLabel', () => {
labelScale,
});
// Find the Animated.Text component by its text content
const labelElement = screen.getByText(longLabel);
const labelElement = screen.getByText(longLabel, {includeHiddenElements: true});
// Verify the component renders the correct text
expect(labelElement).toBeTruthy();
// Verify the props for shortening behavior
Expand All @@ -44,7 +44,7 @@ describe('TextInputLabel', () => {
labelScale,
});
// Find the Animated.Text component by its text content
const labelElement = screen.getByText(label);
const labelElement = screen.getByText(label, {includeHiddenElements: true});
// Verify the component renders the correct text
expect(labelElement).toBeTruthy();
// Verify that numberOfLines and ellipsizeMode are undefined
Expand Down
Loading