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

Fix link is shown in a separate line #20440

Merged
merged 2 commits into from
Jun 8, 2023
Merged

Fix link is shown in a separate line #20440

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8883de7
fix link is shown in a separate line
bernhardoj Jun 8, 2023
e37a362
remove unused import
bernhardoj Jun 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
68 changes: 33 additions & 35 deletions src/components/AnchorForCommentsOnly/BaseAnchorForCommentsOnly.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,41 +51,39 @@ const BaseAnchorForCommentsOnly = (props) => {
const isEmail = Str.isValidEmailMarkdown(props.href.replace(/mailto:/i, ''));

return (
<View style={[...props.containerStyles]}>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the View from here and move it to AnchorForCommentsOnly/index.js. Having a view here will misaligned the text in native.
image

<PressableWithSecondaryInteraction
inline
onSecondaryInteraction={(event) => {
ReportActionContextMenu.showContextMenu(
isEmail ? ContextMenuActions.CONTEXT_MENU_TYPES.EMAIL : ContextMenuActions.CONTEXT_MENU_TYPES.LINK,
event,
props.href,
lodashGet(linkRef, 'current'),
);
}}
onPress={linkProps.onPress}
onPressIn={props.onPressIn}
onPressOut={props.onPressOut}
>
<Tooltip text={props.href}>
<Text
ref={(el) => (linkRef = el)}
style={StyleSheet.flatten([props.style, defaultTextStyle])}
accessibilityRole="link"
hrefAttrs={{
rel: props.rel,
target: isEmail ? '_self' : props.target,
}}
href={linkProps.href}
// Add testID so it gets selected as an anchor tag by SelectionScraper
testID="a"
// eslint-disable-next-line react/jsx-props-no-spreading
{...rest}
>
{props.children}
</Text>
</Tooltip>
</PressableWithSecondaryInteraction>
</View>
<PressableWithSecondaryInteraction
inline
onSecondaryInteraction={(event) => {
ReportActionContextMenu.showContextMenu(
isEmail ? ContextMenuActions.CONTEXT_MENU_TYPES.EMAIL : ContextMenuActions.CONTEXT_MENU_TYPES.LINK,
event,
props.href,
lodashGet(linkRef, 'current'),
);
}}
onPress={linkProps.onPress}
onPressIn={props.onPressIn}
onPressOut={props.onPressOut}
>
<Tooltip text={props.href}>
<Text
ref={(el) => (linkRef = el)}
style={StyleSheet.flatten([props.style, defaultTextStyle])}
accessibilityRole="link"
hrefAttrs={{
rel: props.rel,
target: isEmail ? '_self' : props.target,
}}
href={linkProps.href}
// Add testID so it gets selected as an anchor tag by SelectionScraper
testID="a"
// eslint-disable-next-line react/jsx-props-no-spreading
{...rest}
>
{props.children}
</Text>
</Tooltip>
</PressableWithSecondaryInteraction>
);
};

Expand Down
20 changes: 9 additions & 11 deletions src/components/AnchorForCommentsOnly/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,20 @@
import React from 'react';
import {View} from 'react-native';
import * as anchorForCommentsOnlyPropTypes from './anchorForCommentsOnlyPropTypes';
import BaseAnchorForCommentsOnly from './BaseAnchorForCommentsOnly';
import * as DeviceCapabilities from '../../libs/DeviceCapabilities';
import ControlSelection from '../../libs/ControlSelection';
import styles from '../../styles/styles';

const AnchorForCommentsOnly = (props) => (
<BaseAnchorForCommentsOnly
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
onPressIn={() => DeviceCapabilities.canUseTouchScreen() && ControlSelection.block()}
onPressOut={() => ControlSelection.unblock()}
// HTML renderer root view display is flex. Using flex will force all child elements
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this comment because actually we can use the display inline, so I think the comment is not valid anymore.

Copy link
Contributor

Choose a reason for hiding this comment

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

@bernhardoj can you please tell me the purpose of this View wrapper with display inline here?
I was working on #19545 which was also a regression from the same PR as your original issue and I will remove this wrapper.

I am checking all the related issues to make sure it doesn't break anything but just wanted to get your opinion.

// to be block elements even when they have display inline added to them.
// This will affect elements like <a> which are inline by default.
// Setting display block to the container view will solve that.
containerStyles={[styles.dBlock]}
/>
<View style={styles.dInline}>
<BaseAnchorForCommentsOnly
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
onPressIn={() => DeviceCapabilities.canUseTouchScreen() && ControlSelection.block()}
onPressOut={() => ControlSelection.unblock()}
/>
</View>
);

AnchorForCommentsOnly.propTypes = anchorForCommentsOnlyPropTypes.propTypes;
Expand Down