Skip to content
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.

Sign up for GitHub

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

Jump to bottom

suppressHighlighting on Text components with onPress #24244

Merged
merged 2 commits into from
Aug 14, 2023
Merged

suppressHighlighting on Text components with onPress #24244

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3744bfc
onpress
c3024 Aug 8, 2023
e17367f
typo
c3024 Aug 8, 2023
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
1 change: 1 addition & 0 deletions src/components/AnchorForCommentsOnly/BaseAnchorForCommentsOnly.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ function BaseAnchorForCommentsOnly({onPressIn, onPressOut, href = '', rel = '',
event.preventDefault();
linkProps.onPress();
}}
suppressHighlighting
Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't suppress here. Its already wrapped under PressableWithSecondaryInteraction which means its expecting to have opacity change on press. @c3024

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This component is used in ReportActionFragment and if we don't suppress the highlighting this still remains inconsistent in iOS native compared to other platforms.
Please check this video.

Screen.Recording.2023-08-09.at.8.48.17.AM.mov

Extra highlighting for Text components in iOS is only suppressed here. The other interactions of the wrapped components are unaffected. So, I think we should add suppressHighlighting here as well.
@abdulrahuman5196

// Add testID so it gets selected as an anchor tag by SelectionScraper
testID="a"
// eslint-disable-next-line react/jsx-props-no-spreading
Expand Down
1 change: 1 addition & 0 deletions src/components/Banner.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ function Banner(props) {
<Text
style={[...props.textStyles]}
onPress={props.onPress}
suppressHighlighting
>
{props.text}
</Text>
Expand Down
1 change: 1 addition & 0 deletions src/components/HTMLEngineProvider/HTMLRenderers/AnchorRenderer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ function AnchorRenderer(props) {
<Text
style={styles.link}
onPress={navigateToLink}
suppressHighlighting
>
<TNodeChildrenRenderer tnode={props.tnode} />
</Text>
Expand Down
2 changes: 2 additions & 0 deletions src/components/ReportWelcomeText.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ function ReportWelcomeText(props) {
<Text
style={[styles.textStrong]}
onPress={() => Navigation.navigate(ROUTES.getReportDetailsRoute(props.report.reportID))}
suppressHighlighting
>
{ReportUtils.getReportName(props.report)}
</Text>
Expand All @@ -105,6 +106,7 @@ function ReportWelcomeText(props) {
<Text
style={[styles.textStrong]}
onPress={() => Navigation.navigate(ROUTES.getProfileRoute(accountID))}
suppressHighlighting
>
{displayName}
</Text>
Expand Down
1 change: 1 addition & 0 deletions src/components/TextLink.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ function TextLink(props) {
onMouseDown={props.onMouseDown}
onKeyDown={openLinkIfEnterKeyPressed}
ref={props.forwardedRef}
suppressHighlighting
Copy link
Contributor

Choose a reason for hiding this comment

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

Same goes here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TextLink is used in Bills section of Workspace page.
Here as well if we do not suppress, the difference remains in iOS native.
Here is the video.
https://github.com/Expensify/App/assets/102477862/0c264cfa-e347-41ae-bef5-b7253d72d46a

// eslint-disable-next-line react/jsx-props-no-spreading
{...rest}
>
Expand Down
Loading