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.

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

Show full link when hovering over hyperlink #8815

Merged
merged 16 commits into from
May 4, 2022
Merged
Show file tree
Hide file tree
Changes from 7 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 @@ -10,6 +10,7 @@ import * as ReportActionContextMenu from '../../../pages/home/report/ContextMenu
import * as ContextMenuActions from '../../../pages/home/report/ContextMenu/ContextMenuActions';
import AttachmentView from '../../AttachmentView';
import fileDownload from '../../../libs/fileDownload';
import Tooltip from '../../Tooltip';

/*
* This is a default anchor component for regular links.
Expand Down Expand Up @@ -70,20 +71,22 @@ class BaseAnchorForCommentsOnly extends React.Component {
}
}
>
<Text
ref={el => linkRef = el}
style={StyleSheet.flatten(this.props.style)}
accessibilityRole="link"
href={this.props.href}
hrefAttrs={{
rel: this.props.rel,
target: this.props.target,
}}
// eslint-disable-next-line react/jsx-props-no-spreading
{...rest}
>
{this.props.children}
</Text>
<Tooltip text={Str.isValidEmail(this.props.displayName) ? '' : this.props.href}>
<Text
ref={el => linkRef = el}
style={StyleSheet.flatten(this.props.style)}
accessibilityRole="link"
href={this.props.href}
hrefAttrs={{
rel: this.props.rel,
target: this.props.target,
}}
// eslint-disable-next-line react/jsx-props-no-spreading
{...rest}
>
{this.props.children}
</Text>
</Tooltip>
</PressableWithSecondaryInteraction>
)
);
Expand Down
5 changes: 5 additions & 0 deletions src/components/ContextMenuItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,17 @@ const propTypes = {

/** Automatically reset the success status */
autoReset: PropTypes.bool,

/** A description text to show under the title */
description: PropTypes.string,
};

const defaultProps = {
isMini: false,
successIcon: null,
successText: '',
autoReset: false,
description: '',
};

class ContextMenuItem extends Component {
Expand Down Expand Up @@ -109,6 +113,7 @@ class ContextMenuItem extends Component {
onPress={this.triggerPressAndUpdateSuccess}
wrapperStyle={styles.pr9}
success={this.state.success}
description={this.props.description}
/>
)
);
Expand Down
2 changes: 1 addition & 1 deletion src/components/MenuItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ const MenuItem = props => (
>
{props.title}
</Text>
{props.description && (
{Boolean(props.description) && (
<Text style={[styles.textLabelSupporting, styles.ml3, styles.mt1]}>
{props.description}
</Text>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,15 @@ import {
} from './genericReportActionContextMenuPropTypes';
import withLocalize, {withLocalizePropTypes} from '../../../../components/withLocalize';
import ContextMenuActions, {CONTEXT_MENU_TYPES} from './ContextMenuActions';
import compose from '../../../../libs/compose';
import withWindowDimensions, {windowDimensionsPropTypes} from '../../../../components/withWindowDimensions';

const propTypes = {
/** String representing the context menu type [LINK, REPORT_ACTION] which controls context menu choices */
type: PropTypes.string,
...genericReportActionContextMenuPropTypes,
...withLocalizePropTypes,
...windowDimensionsPropTypes,
};

const defaultProps = {
Expand Down Expand Up @@ -49,6 +52,7 @@ class BaseReportActionContextMenu extends React.Component {
draftMessage: this.props.draftMessage,
selection: this.props.selection,
})}
description={contextAction.getDescription ? contextAction.getDescription({selection: this.props.selection, isSmallScreenWidth: this.props.isSmallScreenWidth}) : ''}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
description={contextAction.getDescription ? contextAction.getDescription({selection: this.props.selection, isSmallScreenWidth: this.props.isSmallScreenWidth}) : ''}
description={contextAction.getDescription ? contextAction.getDescription(this.props.selection, this.props.isSmallScreenWidth) : ''}

Copy link
Contributor

Choose a reason for hiding this comment

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

No need to pass those arguments as an object.

/>
))}
</View>
Expand All @@ -59,4 +63,7 @@ class BaseReportActionContextMenu extends React.Component {
BaseReportActionContextMenu.propTypes = propTypes;
BaseReportActionContextMenu.defaultProps = defaultProps;

export default withLocalize(BaseReportActionContextMenu);
export default compose(
withLocalize,
withWindowDimensions,
)(BaseReportActionContextMenu);
2 changes: 2 additions & 0 deletions src/pages/home/report/ContextMenu/ContextMenuActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import CONST from '../../../../CONST';
import getAttachmentDetails from '../../../../libs/fileDownload/getAttachmentDetails';
import fileDownload from '../../../../libs/fileDownload';
import addEncryptedAuthTokenToURL from '../../../../libs/addEncryptedAuthTokenToURL';
import * as ContextMenuUtils from './ContextMenuUtils';

/**
* Gets the HTML version of the message in an action.
Expand Down Expand Up @@ -62,6 +63,7 @@ export default [
Clipboard.setString(selection);
hideContextMenu(true, ReportActionComposeFocusManager.focus);
},
getDescription: ({selection, isSmallScreenWidth}) => (ContextMenuUtils.shouldShowDescription(isSmallScreenWidth) ? selection : ''),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
getDescription: ({selection, isSmallScreenWidth}) => (ContextMenuUtils.shouldShowDescription(isSmallScreenWidth) ? selection : ''),
getDescription: ContextMenuUtils.getPopoverDescription,

Copy link
Contributor

Choose a reason for hiding this comment

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

This cleans up really well now and it's just passing a reference

},
{
textTranslateKey: 'reportActionContextMenu.copyEmailToClipboard',
Expand Down
15 changes: 15 additions & 0 deletions src/pages/home/report/ContextMenu/ContextMenuUtils/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/* eslint-disable import/prefer-default-export */

/**
* We should show popover description only for mWeb
*
* @param {Boolean} isSmallScreenWidth
* @returns {Boolean}
*/
function shouldShowDescription(isSmallScreenWidth) {
return isSmallScreenWidth;
}

export {
shouldShowDescription,
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
function shouldShowDescription(isSmallScreenWidth) {
return isSmallScreenWidth;
}
export {
shouldShowDescription,
};
function getPopoverDescription(selection, isSmallScreenWidth) {
return isSmallScreenWidth ? selection : '';
}
export {
getPopoverDescription,
};

Copy link
Contributor

Choose a reason for hiding this comment

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

Change the method name to be more descriptive and perform slightly different logic.

Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/* eslint-disable import/prefer-default-export */

/**
* Always show popover description for native platforms
*
* @returns {Boolean}
*/
function shouldShowDescription() {
return true;
}

export {
shouldShowDescription,
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
function shouldShowDescription() {
return true;
}
export {
shouldShowDescription,
};
function getPopoverDescription() {
return '';
}
export {
getPopoverDescription,
};