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

Dont display tooltips on platforms that dont support hover #12168

Merged
merged 52 commits into from
Jan 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
d78b0d0
Create function to detect internet explorer
roryabraham Oct 26, 2022
2f372e2
Create more accurate implementation of hasHoverSupport
roryabraham Oct 26, 2022
1faba5d
Handle edge case for ancient IE mobile
roryabraham Oct 26, 2022
346e074
Add documentation for IE edge case
roryabraham Oct 26, 2022
852c0c3
Combine canUseTouchscreen and hasHoverSupport into DeviceCapabilities…
roryabraham Oct 26, 2022
5e8f723
Dont render tooltips if the device does not support hover
roryabraham Oct 26, 2022
a8360e5
Upgrade testing-library/react-native and transitive peer dependencies
roryabraham Oct 27, 2022
18ff316
Merge branch 'main' into Rory-NoTooltipWithoutMouse
roryabraham Nov 17, 2022
d17c423
Reinstall packages after merge
roryabraham Nov 17, 2022
9091ee5
Reapply changes from 12078
roryabraham Nov 17, 2022
932d999
Add eslint-plugin-lwc transitive dependency
roryabraham Nov 17, 2022
f8938cf
Nix deprecated babel-eslint and replace with @babel/eslint-parser
roryabraham Nov 17, 2022
22da5de
Separately install jest-environment-jsdom
roryabraham Nov 17, 2022
f583bdf
Migrate to DeviceCapabilities lib in OptionsList/index.js
roryabraham Nov 17, 2022
e7f26c3
Fix JS style
roryabraham Nov 17, 2022
6b473ed
Upgrade jest to match jest-environment-eslint
roryabraham Nov 17, 2022
3b0e1b2
Upgrade babel/preset-env
roryabraham Nov 17, 2022
e9097a7
Fix babel config
roryabraham Nov 17, 2022
5652989
Upgrade reanimated to fix jest 28 bug
roryabraham Nov 17, 2022
f720dac
Fix noisy loose tests
roryabraham Nov 17, 2022
0b3123e
Fix fake timers
roryabraham Nov 17, 2022
515a3c3
Create draft of TooltipTest
roryabraham Nov 18, 2022
9dfe38e
Merge branch 'main' into Rory-NoTooltipWithoutMouse
roryabraham Dec 14, 2022
28b2462
Remove IE considerations
roryabraham Dec 14, 2022
05e466d
Finish removing IE support
roryabraham Dec 14, 2022
73f0bdf
Swap out ReactDom.createPortal for @gorhom/portal
roryabraham Dec 14, 2022
aece03e
Fix typo in touchscreen
roryabraham Dec 14, 2022
46b2140
Finish automated tests
roryabraham Dec 14, 2022
ecb7894
Remove console log
roryabraham Dec 14, 2022
8e0a11a
Merge branch 'main' into Rory-NoTooltipWithoutMouse
roryabraham Dec 16, 2022
f1b5250
Mostly fix UnreadIndicatorsTest
roryabraham Dec 16, 2022
518aad2
Fully fix UnreadIndicatorsTest
roryabraham Dec 16, 2022
6df5f55
Fix sidebar tests
roryabraham Dec 16, 2022
2cf41c1
Move setTimeout to file level instead of beforeEach
roryabraham Dec 16, 2022
6bd8c69
Try using built-in jest sharding
roryabraham Dec 16, 2022
afc55b6
Move fail-fast property to the correct place:
roryabraham Dec 16, 2022
9394b78
Add setupNode back
roryabraham Dec 16, 2022
dc7c330
Add setupNode to shellTests jobs
roryabraham Dec 16, 2022
8ac00ea
Remove getPullRequestsMergedBetween from jest job
roryabraham Dec 16, 2022
bf7b724
Use dynamic job total
roryabraham Dec 16, 2022
3b68d03
Try adding max-workers to address handing test
roryabraham Dec 16, 2022
a22b336
Button up TooltipTest
roryabraham Dec 17, 2022
ec9dce1
Add a comment explaining why TooltipTest manually imports Tooltip/ind…
roryabraham Dec 19, 2022
08bedee
Merge branch 'main' into Rory-NoTooltipWithoutMouse
roryabraham Dec 19, 2022
4236e8f
Merge branch main into Rory-NoTooltipWithoutMouse
roryabraham Jan 2, 2023
d09744c
Revert test-related changes
roryabraham Jan 2, 2023
9361ad7
Ooops I didn't commit everything :D
roryabraham Jan 2, 2023
611e0d8
Add code comment for ref callback
roryabraham Jan 2, 2023
92f0efd
Remove unrelated lint change
roryabraham Jan 2, 2023
0706760
Move tooltip down a level in the component tree and fix issues with s…
roryabraham Jan 2, 2023
a62d5fd
Hide tooltip if correct position not yet measured
roryabraham Jan 2, 2023
d3aa664
Merge branch 'main' into Rory-NoTooltipWithoutMouse
roryabraham Jan 6, 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
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import PressableWithSecondaryInteraction from '../PressableWithSecondaryInteract
import * as ReportActionContextMenu from '../../pages/home/report/ContextMenu/ReportActionContextMenu';
import * as ContextMenuActions from '../../pages/home/report/ContextMenu/ContextMenuActions';
import Tooltip from '../Tooltip';
import canUseTouchScreen from '../../libs/canUseTouchscreen';
import * as DeviceCapabilities from '../../libs/DeviceCapabilities';
import styles from '../../styles/styles';
import withWindowDimensions, {windowDimensionsPropTypes} from '../withWindowDimensions';
import {propTypes as anchorForCommentsOnlyPropTypes, defaultProps} from './anchorForCommentsOnlyPropTypes';
Expand All @@ -30,7 +30,7 @@ const BaseAnchorForCommentsOnly = (props) => {
} else {
linkProps.href = props.href;
}
const defaultTextStyle = canUseTouchScreen() || props.isSmallScreenWidth ? {} : styles.userSelectText;
const defaultTextStyle = DeviceCapabilities.canUseTouchScreen() || props.isSmallScreenWidth ? {} : styles.userSelectText;

return (
<PressableWithSecondaryInteraction
Expand Down
4 changes: 2 additions & 2 deletions src/components/HTMLEngineProvider/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ import React from 'react';
import BaseHTMLEngineProvider from './BaseHTMLEngineProvider';
import {defaultProps, propTypes} from './htmlEnginePropTypes';
import withWindowDimensions from '../withWindowDimensions';
import canUseTouchScreen from '../../libs/canUseTouchscreen';
import * as DeviceCapabilities from '../../libs/DeviceCapabilities';

const HTMLEngineProvider = props => (
<BaseHTMLEngineProvider
debug={props.debug}
textSelectable={!canUseTouchScreen() || !props.isSmallScreenWidth}
textSelectable={!DeviceCapabilities.canUseTouchScreen() || !props.isSmallScreenWidth}
>
{props.children}
</BaseHTMLEngineProvider>
Expand Down
4 changes: 2 additions & 2 deletions src/components/ImageView/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
} from 'react-native';
import styles from '../../styles/styles';
import * as StyleUtils from '../../styles/StyleUtils';
import canUseTouchScreen from '../../libs/canUseTouchscreen';
import * as DeviceCapabilities from '../../libs/DeviceCapabilities';
import withWindowDimensions, {windowDimensionsPropTypes} from '../withWindowDimensions';
import FullscreenLoadingIndicator from '../FullscreenLoadingIndicator';

Expand All @@ -19,7 +19,7 @@ class ImageView extends PureComponent {
constructor(props) {
super(props);
this.scrollableRef = null;
this.canUseTouchScreen = canUseTouchScreen();
this.canUseTouchScreen = DeviceCapabilities.canUseTouchScreen();
this.onContainerLayoutChanged = this.onContainerLayoutChanged.bind(this);
this.onContainerPressIn = this.onContainerPressIn.bind(this);
this.onContainerPress = this.onContainerPress.bind(this);
Expand Down
6 changes: 3 additions & 3 deletions src/components/OptionsList/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import {Keyboard} from 'react-native';
import _ from 'underscore';
import BaseOptionsList from './BaseOptionsList';
import withWindowDimensions from '../withWindowDimensions';
import canUseTouchscreen from '../../libs/canUseTouchscreen';
import {propTypes, defaultProps} from './optionsListPropTypes';
import * as DeviceCapabilities from '../../libs/DeviceCapabilities';

class OptionsList extends Component {
constructor(props) {
Expand All @@ -15,7 +15,7 @@ class OptionsList extends Component {
}

componentDidMount() {
if (!canUseTouchscreen()) {
if (!DeviceCapabilities.canUseTouchScreen()) {
return;
}

Expand All @@ -26,7 +26,7 @@ class OptionsList extends Component {
}

componentWillUnmount() {
if (!canUseTouchscreen()) {
if (!DeviceCapabilities.canUseTouchScreen()) {
return;
}

Expand Down
4 changes: 2 additions & 2 deletions src/components/PressableWithSecondaryInteraction/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {Pressable} from 'react-native';
import {LongPressGestureHandler, State} from 'react-native-gesture-handler';
import * as pressableWithSecondaryInteractionPropTypes from './pressableWithSecondaryInteractionPropTypes';
import styles from '../../styles/styles';
import hasHoverSupport from '../../libs/hasHoverSupport';
import * as DeviceCapabilities from '../../libs/DeviceCapabilities';

/**
* This is a special Pressable that calls onSecondaryInteraction when LongPressed, or right-clicked.
Expand All @@ -31,7 +31,7 @@ class PressableWithSecondaryInteraction extends Component {
* @param {Object} e
*/
callSecondaryInteractionWithMappedEvent(e) {
if ((e.nativeEvent.state !== State.ACTIVE) || hasHoverSupport()) {
if ((e.nativeEvent.state !== State.ACTIVE) || DeviceCapabilities.hasHoverSupport()) {
return;
}

Expand Down
47 changes: 29 additions & 18 deletions src/components/Tooltip/TooltipRenderedOnPageBody.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import PropTypes from 'prop-types';
import {Animated, View} from 'react-native';
import ReactDOM from 'react-dom';
import {Portal} from '@gorhom/portal';
import getTooltipStyles from '../../styles/getTooltipStyles';
import Text from '../Text';

Expand Down Expand Up @@ -69,10 +69,6 @@ class TooltipRenderedOnPageBody extends React.PureComponent {
this.updateTooltipTextWidth = this.updateTooltipTextWidth.bind(this);
}

componentDidMount() {
this.updateTooltipTextWidth();
}

componentDidUpdate(prevProps) {
if (prevProps.text === this.props.text) {
return;
Expand Down Expand Up @@ -122,19 +118,34 @@ class TooltipRenderedOnPageBody extends React.PureComponent {
this.props.shiftHorizontal,
this.props.shiftVertical,
);
return ReactDOM.createPortal(
<Animated.View
onLayout={this.measureTooltip}
style={[tooltipWrapperStyle, animationStyle]}
>
<Text numberOfLines={this.props.numberOfLines} style={tooltipTextStyle}>
<Text style={tooltipTextStyle} ref={ref => this.textRef = ref}>{this.props.text}</Text>
</Text>
<View style={pointerWrapperStyle}>
<View style={pointerStyle} />
</View>
</Animated.View>,
document.querySelector('body'),
return (
<Portal>
<Animated.View
onLayout={this.measureTooltip}
style={[tooltipWrapperStyle, animationStyle]}
>
<Text numberOfLines={this.props.numberOfLines} style={tooltipTextStyle}>
<Text
style={tooltipTextStyle}
ref={(ref) => {
// Once the text for the tooltip first renders, update the width of the tooltip dynamically to fit the width of the text.
// Note that we can't have this code in componentDidMount because the ref for the text won't be set until after the first render
if (this.textRef) {
roryabraham marked this conversation as resolved.
Show resolved Hide resolved
return;
}

this.textRef = ref;
this.updateTooltipTextWidth();
}}
>
{this.props.text}
</Text>
</Text>
<View style={pointerWrapperStyle}>
<View style={pointerStyle} />
</View>
</Animated.View>
</Portal>
);
}
}
Expand Down
13 changes: 4 additions & 9 deletions src/components/Tooltip/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import withWindowDimensions from '../withWindowDimensions';
import {propTypes, defaultProps} from './tooltipPropTypes';
import TooltipSense from './TooltipSense';
import makeCancellablePromise from '../../libs/MakeCancellablePromise';
import * as Browser from '../../libs/Browser';
import * as DeviceCapabilities from '../../libs/DeviceCapabilities';

class Tooltip extends PureComponent {
constructor(props) {
Expand All @@ -32,6 +32,7 @@ class Tooltip extends PureComponent {
this.isTooltipSenseInitiator = false;
this.shouldStartShowAnimation = false;
this.animation = new Animated.Value(0);
this.hasHoverSupport = DeviceCapabilities.hasHoverSupport();

this.getWrapperPosition = this.getWrapperPosition.bind(this);
this.showTooltip = this.showTooltip.bind(this);
Expand Down Expand Up @@ -80,12 +81,6 @@ class Tooltip extends PureComponent {
* Display the tooltip in an animation.
*/
showTooltip() {
// On mWeb we do not show Tooltips as there are no way to hide them besides blurring.
// That's due to that fact that on mWeb there is no MouseLeave events.
if (Browser.isMobile()) {
return;
}

if (!this.state.isRendered) {
this.setState({isRendered: true});
}
Expand Down Expand Up @@ -147,8 +142,8 @@ class Tooltip extends PureComponent {
}

render() {
// Skip the tooltip and return the children, if the text is empty.
if (_.isEmpty(this.props.text)) {
// Skip the tooltip and return the children if the text is empty or the device does not support hovering
if (_.isEmpty(this.props.text) || !this.hasHoverSupport) {
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 is the most critical behavior change in this PR

return this.props.children;
}
let child = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,8 @@
*
* @returns {Boolean}
*/

import * as Browser from '../Browser';

const hasHoverSupport = () => !Browser.isMobile();
function hasHoverSupport() {
return !window.matchMedia('(hover: none)').matches;
roryabraham marked this conversation as resolved.
Show resolved Hide resolved
}

export default hasHoverSupport;

7 changes: 7 additions & 0 deletions src/libs/DeviceCapabilities/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import canUseTouchScreen from './canUseTouchScreen';
import hasHoverSupport from './hasHoverSupport';

export {
canUseTouchScreen,
hasHoverSupport,
};
4 changes: 2 additions & 2 deletions src/libs/canFocusInputOnScreenFocus/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
import canUseTouchScreen from '../canUseTouchscreen';
import * as DeviceCapabilities from '../DeviceCapabilities';

export default () => !canUseTouchScreen();
export default () => !DeviceCapabilities.canUseTouchScreen();
16 changes: 8 additions & 8 deletions src/pages/home/HeaderView.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,15 @@ const HeaderView = (props) => {
<View style={[styles.appContentHeader]} nativeID="drag-area">
<View style={[styles.appContentHeaderTitle, !props.isSmallScreenWidth && styles.pl5]}>
{props.isSmallScreenWidth && (
<Tooltip text={props.translate('common.back')}>
<Pressable
onPress={props.onNavigationMenuButtonClicked}
style={[styles.LHNToggle]}
accessibilityHint="Navigate back to chats list"
>
<Pressable
onPress={props.onNavigationMenuButtonClicked}
style={[styles.LHNToggle]}
accessibilityHint="Navigate back to chats list"
>
<Tooltip text={props.translate('common.back')} shiftVertical={4}>
<Icon src={Expensicons.BackArrow} />
</Pressable>
</Tooltip>
</Tooltip>
</Pressable>
)}
{Boolean(props.report && title) && (
<View
Expand Down
4 changes: 2 additions & 2 deletions src/pages/home/report/ReportActionCompose.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import {withNetwork, withPersonalDetails} from '../../../components/OnyxProvider
import * as User from '../../../libs/actions/User';
import Tooltip from '../../../components/Tooltip';
import EmojiPickerButton from '../../../components/EmojiPicker/EmojiPickerButton';
import canUseTouchScreen from '../../../libs/canUseTouchscreen';
import * as DeviceCapabilities from '../../../libs/DeviceCapabilities';
import toggleReportActionComposeView from '../../../libs/toggleReportActionComposeView';
import OfflineIndicator from '../../../components/OfflineIndicator';
import ExceededCommentLength from '../../../components/ExceededCommentLength';
Expand Down Expand Up @@ -687,7 +687,7 @@ class ReportActionCompose extends React.Component {
</>
)}
</AttachmentModal>
{canUseTouchScreen() && this.props.isMediumScreenWidth ? null : (
{DeviceCapabilities.canUseTouchScreen() && this.props.isMediumScreenWidth ? null : (
<EmojiPickerButton
isDisabled={isBlockedFromConcierge || this.props.disabled}
onModalHide={() => this.focus(true)}
Expand Down
4 changes: 2 additions & 2 deletions src/pages/home/report/ReportActionItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import ReportActionItemCreated from './ReportActionItemCreated';
import compose from '../../../libs/compose';
import withWindowDimensions, {windowDimensionsPropTypes} from '../../../components/withWindowDimensions';
import ControlSelection from '../../../libs/ControlSelection';
import canUseTouchScreen from '../../../libs/canUseTouchscreen';
import * as DeviceCapabilities from '../../../libs/DeviceCapabilities';
import MiniReportActionContextMenu from './ContextMenu/MiniReportActionContextMenu';
import * as ReportActionContextMenu from './ContextMenu/ReportActionContextMenu';
import * as ContextMenuActions from './ContextMenu/ContextMenuActions';
Expand Down Expand Up @@ -180,7 +180,7 @@ class ReportActionItem extends Component {
return (
<PressableWithSecondaryInteraction
ref={el => this.popoverAnchor = el}
onPressIn={() => this.props.isSmallScreenWidth && canUseTouchScreen() && ControlSelection.block()}
onPressIn={() => this.props.isSmallScreenWidth && DeviceCapabilities.canUseTouchScreen() && ControlSelection.block()}
onPressOut={() => ControlSelection.unblock()}
onSecondaryInteraction={this.showPopover}
preventDefaultContentMenu={!this.props.draftMessage}
Expand Down
4 changes: 2 additions & 2 deletions src/pages/home/report/ReportActionItemFragment.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import Tooltip from '../../../components/Tooltip';
import * as EmojiUtils from '../../../libs/EmojiUtils';
import withWindowDimensions, {windowDimensionsPropTypes} from '../../../components/withWindowDimensions';
import withLocalize, {withLocalizePropTypes} from '../../../components/withLocalize';
import canUseTouchScreen from '../../../libs/canUseTouchscreen';
import * as DeviceCapabilities from '../../../libs/DeviceCapabilities';
import compose from '../../../libs/compose';
import * as StyleUtils from '../../../styles/StyleUtils';

Expand Down Expand Up @@ -120,7 +120,7 @@ const ReportActionItemFragment = (props) => {
return (
<Text
family="EMOJI_TEXT_FONT"
selectable={!canUseTouchScreen() || !props.isSmallScreenWidth}
selectable={!DeviceCapabilities.canUseTouchScreen() || !props.isSmallScreenWidth}
style={[EmojiUtils.containsOnlyEmojis(text) ? styles.onlyEmojisText : undefined, styles.ltr, ...props.style]}
>
{StyleUtils.convertToLTR(Str.htmlDecode(text))}
Expand Down
4 changes: 2 additions & 2 deletions src/pages/iou/steps/IOUAmountPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import withLocalize, {withLocalizePropTypes} from '../../../components/withLocal
import compose from '../../../libs/compose';
import Button from '../../../components/Button';
import CONST from '../../../CONST';
import canUseTouchScreen from '../../../libs/canUseTouchscreen';
import * as DeviceCapabilities from '../../../libs/DeviceCapabilities';
import TextInputWithCurrencySymbol from '../../../components/TextInputWithCurrencySymbol';

const propTypes = {
Expand Down Expand Up @@ -271,7 +271,7 @@ class IOUAmountPage extends React.Component {
/>
</View>
<View style={[styles.w100, styles.justifyContentEnd, styles.pageWrapper]}>
{canUseTouchScreen()
{DeviceCapabilities.canUseTouchScreen()
? (
<BigNumberPad
numberPressed={this.updateAmountNumberPad}
Expand Down
4 changes: 2 additions & 2 deletions src/pages/settings/AppDownloadLinks.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import * as Link from '../../libs/actions/Link';
import PressableWithSecondaryInteraction from '../../components/PressableWithSecondaryInteraction';
import ControlSelection from '../../libs/ControlSelection';
import withWindowDimensions, {windowDimensionsPropTypes} from '../../components/withWindowDimensions';
import canUseTouchScreen from '../../libs/canUseTouchscreen';
import * as DeviceCapabilities from '../../libs/DeviceCapabilities';
import * as ReportActionContextMenu from '../home/report/ContextMenu/ReportActionContextMenu';
import * as ContextMenuActions from '../home/report/ContextMenu/ContextMenuActions';
import PopoverReportActionContextMenu from '../home/report/ContextMenu/PopoverReportActionContextMenu';
Expand Down Expand Up @@ -87,7 +87,7 @@ const AppDownloadLinksPage = (props) => {
{_.map(menuItems, item => (
<PressableWithSecondaryInteraction
key={item.translationKey}
onPressIn={() => props.isSmallScreenWidth && canUseTouchScreen() && ControlSelection.block()}
onPressIn={() => props.isSmallScreenWidth && DeviceCapabilities.canUseTouchScreen() && ControlSelection.block()}
onPressOut={() => ControlSelection.unblock()}
onSecondaryInteraction={e => showPopover(e, item.link)}
ref={el => popoverAnchor = el}
Expand Down
Loading