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

refactor tooltip to fix double focusable component #16052

Merged
merged 33 commits into from
May 25, 2023
Merged
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
24381ed
refactor tooltip to fix double focusable component
bernhardoj Mar 16, 2023
bd13ee3
remove console log
bernhardoj Mar 16, 2023
79031f1
fix lint
bernhardoj Mar 16, 2023
1d72fc3
remove absolute prop
bernhardoj Mar 16, 2023
0f7ed54
remove focusable prop
bernhardoj Mar 16, 2023
7498bf7
Merge branch 'main' into fix/15796
bernhardoj Mar 20, 2023
a877422
merged with main
bernhardoj Apr 7, 2023
85a6892
Merge branch 'main' into fix/15796
bernhardoj Apr 14, 2023
a0090b6
remove focusable and onblur
bernhardoj Apr 14, 2023
c8cf0d2
revert wrong change
bernhardoj Apr 14, 2023
12b33eb
refactor tooltip usage
bernhardoj Apr 14, 2023
b327335
remove tooltip workaround
bernhardoj Apr 14, 2023
3278c9e
fix incorrect way to access the children
bernhardoj Apr 14, 2023
84915ac
remove unused import
bernhardoj Apr 14, 2023
6dd4cc2
add view wrapper to social icons
bernhardoj Apr 17, 2023
90f9dd4
Merge branch 'main' into fix/15796
bernhardoj Apr 26, 2023
5cbd61d
Merge branch 'main' into fix/15796
bernhardoj Apr 28, 2023
e3e79b9
fix tooltip does not show
bernhardoj Apr 28, 2023
ab5923f
Merge branch 'main' into fix/15796
bernhardoj May 1, 2023
ce8949a
Merge branch 'main' into fix/15796
bernhardoj May 4, 2023
23e5799
Merge branch 'main' into fix/15796
bernhardoj May 5, 2023
2a5c071
Merge branch 'main' into fix/15796
bernhardoj May 10, 2023
6cd6bdc
merge main
bernhardoj May 15, 2023
8cc71f6
remove absolute props
bernhardoj May 15, 2023
1d66b01
Merge branch 'main' into fix/15796
bernhardoj May 17, 2023
b978fd7
fix prettier
bernhardoj May 17, 2023
da65972
remove unnecessary prop types
bernhardoj May 18, 2023
4d528af
Merge branch 'main' into fix/15796
bernhardoj May 21, 2023
7fd940d
Merge branch 'main' into fix/15796
bernhardoj May 23, 2023
a627cf5
temporarily remove shitf vertical
bernhardoj May 23, 2023
2d1dce1
add View wrapper to checkbox
bernhardoj May 23, 2023
21af5fc
Merge branch 'main' into fix/15796
bernhardoj May 24, 2023
123e9ad
Merge branch 'main' into fix/15796
bernhardoj May 25, 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 @@ -60,10 +60,7 @@ const BaseAnchorForCommentsOnly = (props) => {
onPressIn={props.onPressIn}
onPressOut={props.onPressOut}
>
<Tooltip
containerStyles={[styles.dInline]}
text={props.href}
>
<Tooltip text={props.href}>
<Text
ref={(el) => (linkRef = el)}
style={StyleSheet.flatten([props.style, defaultTextStyle])}
Expand Down
16 changes: 8 additions & 8 deletions src/components/AttachmentCarousel/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -296,8 +296,8 @@ class AttachmentCarousel extends React.Component {
{this.state.shouldShowArrow && (
<>
{!isBackDisabled && (
<View style={[styles.attachmentArrow, this.props.isSmallScreenWidth ? styles.l2 : styles.l8]}>
<Tooltip text={this.props.translate('common.previous')}>
<Tooltip text={this.props.translate('common.previous')}>
<View style={[styles.attachmentArrow, this.props.isSmallScreenWidth ? styles.l2 : styles.l8]}>
<Button
small
innerStyles={[styles.arrowIcon]}
Expand All @@ -311,12 +311,12 @@ class AttachmentCarousel extends React.Component {
onPressIn={this.cancelAutoHideArrow}
onPressOut={this.autoHideArrow}
/>
</Tooltip>
</View>
</View>
</Tooltip>
)}
{!isForwardDisabled && (
<View style={[styles.attachmentArrow, this.props.isSmallScreenWidth ? styles.r2 : styles.r8]}>
<Tooltip text={this.props.translate('common.next')}>
<Tooltip text={this.props.translate('common.next')}>
<View style={[styles.attachmentArrow, this.props.isSmallScreenWidth ? styles.r2 : styles.r8]}>
<Button
small
innerStyles={[styles.arrowIcon]}
Expand All @@ -330,8 +330,8 @@ class AttachmentCarousel extends React.Component {
onPressIn={this.cancelAutoHideArrow}
onPressOut={this.autoHideArrow}
/>
</Tooltip>
</View>
</View>
</Tooltip>
)}
</>
)}
Expand Down
8 changes: 4 additions & 4 deletions src/components/AttachmentView.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,11 @@ const AttachmentView = (props) => {

<Text style={[styles.textStrong, styles.flexShrink1, styles.breakAll, styles.flexWrap, styles.mw100]}>{props.file && props.file.name}</Text>
{!props.shouldShowLoadingSpinnerIcon && props.shouldShowDownloadIcon && (
<View style={styles.ml2}>
<Tooltip text={props.translate('common.download')}>
<Tooltip text={props.translate('common.download')}>
<View style={styles.ml2}>
<Icon src={Expensicons.Download} />
</Tooltip>
</View>
</View>
</Tooltip>
)}
{props.shouldShowLoadingSpinnerIcon && (
<View style={styles.ml2}>
Expand Down
16 changes: 9 additions & 7 deletions src/components/AvatarCropModal/AvatarCropModal.js
Original file line number Diff line number Diff line change
Expand Up @@ -410,13 +410,15 @@ const AvatarCropModal = (props) => {
text={props.translate('common.rotate')}
shiftVertical={-2}
>
<Button
medium
icon={Expensicons.Rotate}
iconFill={themeColors.inverse}
iconStyles={[styles.mr0]}
onPress={rotateImage}
/>
<View>
<Button
medium
icon={Expensicons.Rotate}
iconFill={themeColors.inverse}
iconStyles={[styles.mr0]}
onPress={rotateImage}
/>
</View>
</Tooltip>
</View>
</>
Expand Down
5 changes: 1 addition & 4 deletions src/components/AvatarCropModal/Slider.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,7 @@ const Slider = (props) => {
>
<Animated.View style={[styles.sliderKnob, rSliderStyle]}>
{tooltipIsVisible && (
<Tooltip
text={props.translate('common.zoom')}
shiftVertical={-2}
>
<Tooltip text={props.translate('common.zoom')}>
<View style={[styles.sliderKnobTooltipView]} />
</Tooltip>
)}
Expand Down
31 changes: 15 additions & 16 deletions src/components/AvatarWithImagePicker.js
Original file line number Diff line number Diff line change
Expand Up @@ -259,26 +259,25 @@ class AvatarWithImagePicker extends React.Component {
<Pressable onPress={() => this.setState({isMenuVisible: true})}>
<View style={[styles.pRelative, styles.avatarLarge]}>
<Tooltip text={this.props.translate('avatarWithImagePicker.editImage')}>
{this.props.source ? (
<Avatar
containerStyles={styles.avatarLarge}
imageStyles={[styles.avatarLarge, styles.alignSelfCenter]}
source={this.props.source}
fallbackIcon={this.props.fallbackIcon}
size={this.props.size}
type={this.props.type}
/>
) : (
<DefaultAvatar />
)}
<View>
{this.props.source ? (
<Avatar
containerStyles={styles.avatarLarge}
imageStyles={[styles.avatarLarge, styles.alignSelfCenter]}
source={this.props.source}
fallbackIcon={this.props.fallbackIcon}
size={this.props.size}
type={this.props.type}
/>
) : (
<DefaultAvatar />
)}
</View>
</Tooltip>
<AttachmentPicker type={CONST.ATTACHMENT_PICKER_TYPE.IMAGE}>
{({openPicker}) => (
<>
<Tooltip
absolute
text={this.props.translate('avatarWithImagePicker.editImage')}
>
<Tooltip text={this.props.translate('avatarWithImagePicker.editImage')}>
<View style={[styles.smallEditIcon, styles.smallAvatarEditIcon]}>
<Icon
src={Expensicons.Camera}
Expand Down
8 changes: 4 additions & 4 deletions src/components/AvatarWithIndicator.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,12 @@ const AvatarWithIndicator = (props) => {
const indicatorStyles = [styles.alignItemsCenter, styles.justifyContentCenter, styles.statusIndicator(indicatorColor)];

return (
<View style={[styles.sidebarAvatar]}>
<Tooltip text={props.tooltipText}>
<Tooltip text={props.tooltipText}>
<View style={[styles.sidebarAvatar]}>
<Avatar source={ReportUtils.getSmallSizeAvatar(props.source)} />
{(shouldShowErrorIndicator || shouldShowInfoIndicator) && <View style={StyleSheet.flatten(indicatorStyles)} />}
</Tooltip>
</View>
</View>
</Tooltip>
);
};

Expand Down
12 changes: 11 additions & 1 deletion src/components/CheckboxWithTooltip/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,17 @@ const CheckboxWithTooltip = (props) => {
disabled={props.disabled}
/>
);
return <View style={props.style}>{props.toggleTooltip ? <Tooltip text={props.text}>{checkbox}</Tooltip> : checkbox}</View>;
return (
<View style={props.style}>
{props.toggleTooltip ? (
<Tooltip text={props.text}>
<View>{checkbox}</View>
</Tooltip>
) : (
checkbox
)}
</View>
);
};

CheckboxWithTooltip.propTypes = propTypes;
Expand Down
1 change: 0 additions & 1 deletion src/components/DisplayNames/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ class DisplayNames extends PureComponent {
<Tooltip
key={index}
text={tooltip}
containerStyles={[styles.dInline]}
shiftHorizontal={() => this.getTooltipShiftX(index)}
>
{/* // We need to get the refs to all the names which will be used to correct
Expand Down
29 changes: 12 additions & 17 deletions src/components/EmojiPicker/CategoryShortcutButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,30 +33,25 @@ class CategoryShortcutButton extends PureComponent {

render() {
return (
<Pressable
onPress={this.props.onPress}
onHoverIn={() => this.setState({isHighlighted: true})}
onHoverOut={() => this.setState({isHighlighted: false})}
style={({pressed}) => [
StyleUtils.getButtonBackgroundColorStyle(getButtonState(false, pressed)),
styles.categoryShortcutButton,
this.state.isHighlighted && styles.emojiItemHighlighted,
]}
>
<Tooltip
focusable={false}
containerStyles={[styles.flex1, styles.alignSelfStretch, styles.alignItemsCenter, styles.justifyContentCenter]}
text={this.props.translate(`emojiPicker.headers.${this.props.code}`)}
shiftVertical={-4}
<Tooltip text={this.props.translate(`emojiPicker.headers.${this.props.code}`)}>
<Pressable
onPress={this.props.onPress}
onHoverIn={() => this.setState({isHighlighted: true})}
onHoverOut={() => this.setState({isHighlighted: false})}
style={({pressed}) => [
StyleUtils.getButtonBackgroundColorStyle(getButtonState(false, pressed)),
styles.categoryShortcutButton,
this.state.isHighlighted && styles.emojiItemHighlighted,
]}
>
<Icon
fill={themeColors.icon}
src={this.props.icon}
height={variables.iconSizeNormal}
width={variables.iconSizeNormal}
/>
</Tooltip>
</Pressable>
</Pressable>
</Tooltip>
);
}
}
Expand Down
5 changes: 1 addition & 4 deletions src/components/EmojiPicker/EmojiPickerButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,7 @@ const defaultProps = {
const EmojiPickerButton = (props) => {
let emojiPopoverAnchor = null;
return (
<Tooltip
containerStyles={[styles.alignSelfEnd]}
text={props.translate('reportActionCompose.emoji')}
>
<Tooltip text={props.translate('reportActionCompose.emoji')}>
<Pressable
ref={(el) => (emojiPopoverAnchor = el)}
style={({hovered, pressed}) => [styles.chatItemEmojiButton, StyleUtils.getButtonBackgroundColorStyle(getButtonState(hovered, pressed))]}
Expand Down
5 changes: 1 addition & 4 deletions src/components/FloatingActionButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,7 @@ class FloatingActionButton extends PureComponent {
});

return (
<Tooltip
absolute
text={this.props.translate('common.new')}
>
<Tooltip text={this.props.translate('common.new')}>
<View style={styles.floatingActionButtonContainer}>
<AnimatedPressable
ref={(el) => (this.fabPressable = el)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,7 @@ const MentionUserRenderer = (props) => {

return (
<Text>
<Tooltip
absolute
text={loginWhithoutLeadingAt}
>
<Tooltip text={loginWhithoutLeadingAt}>
<Text
// eslint-disable-next-line react/jsx-props-no-spreading
{...defaultRendererProps}
Expand Down
9 changes: 0 additions & 9 deletions src/components/Hoverable/hoverablePropTypes.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,9 @@
import PropTypes from 'prop-types';

const propTypes = {
/** Whether to disable additional wrapper around the children. It will only work for single native(View|Text) child. */
absolute: PropTypes.bool,

/** Children to wrap with Hoverable. */
children: PropTypes.oneOfType([PropTypes.node, PropTypes.func]).isRequired,

/** Styles to be assigned to the Hoverable Container */
// eslint-disable-next-line react/forbid-prop-types
containerStyles: PropTypes.arrayOf(PropTypes.object),

/** Function that executes when the mouse moves over the children. */
onHoverIn: PropTypes.func,

Expand All @@ -19,8 +12,6 @@ const propTypes = {
};

const defaultProps = {
absolute: false,
containerStyles: [],
onHoverIn: () => {},
onHoverOut: () => {},
};
Expand Down
88 changes: 36 additions & 52 deletions src/components/Hoverable/index.js
bernhardoj marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import _ from 'underscore';
import React, {Component} from 'react';
import {View} from 'react-native';
import {propTypes, defaultProps} from './hoverablePropTypes';

/**
Expand Down Expand Up @@ -55,61 +54,46 @@ class Hoverable extends Component {
}

render() {
if (this.props.absolute && React.isValidElement(this.props.children)) {
return React.cloneElement(React.Children.only(this.props.children), {
ref: (el) => {
this.wrapperView = el;

// Call the original ref, if any
const {ref} = this.props.children;
if (_.isFunction(ref)) {
ref(el);
}
},
onMouseEnter: (el) => {
this.setIsHovered(true);

if (_.isFunction(this.props.children.props.onMouseEnter)) {
this.props.children.props.onMouseEnter(el);
}
},
onMouseLeave: (el) => {
this.setIsHovered(false);
const child = _.isFunction(this.props.children) ? this.props.children(this.state.isHovered) : this.props.children;

if (_.isFunction(this.props.children.props.onMouseLeave)) {
this.props.children.props.onMouseLeave(el);
}
},
onBlur: (el) => {
if (!this.wrapperView.contains(el.relatedTarget)) {
this.setIsHovered(false);
}

if (_.isFunction(this.props.children.props.onBlur)) {
this.props.children.props.onBlur(el);
}
},
});
if (!React.isValidElement(child)) {
bernhardoj marked this conversation as resolved.
Show resolved Hide resolved
throw Error('Children is not a valid element.');
}
return (
<View
style={this.props.containerStyles}
ref={(el) => (this.wrapperView = el)}
onMouseEnter={() => this.setIsHovered(true)}
onMouseLeave={() => this.setIsHovered(false)}
onBlur={(el) => {
if (this.wrapperView.contains(el.relatedTarget)) {
return;
}

return React.cloneElement(React.Children.only(child), {
ref: (el) => {
this.wrapperView = el;

// Call the original ref, if any
const {ref} = child;
if (_.isFunction(ref)) {
ref(el);
}
},
onMouseEnter: (el) => {
this.setIsHovered(true);

if (_.isFunction(child.props.onMouseEnter)) {
child.props.onMouseEnter(el);
}
},
onMouseLeave: (el) => {
this.setIsHovered(false);

if (_.isFunction(child.props.onMouseLeave)) {
child.props.onMouseLeave(el);
}
},
onBlur: (el) => {
if (!this.wrapperView.contains(el.relatedTarget)) {
this.setIsHovered(false);
}}
>
{
// If this.props.children is a function, call it to provide the hover state to the children.
_.isFunction(this.props.children) ? this.props.children(this.state.isHovered) : this.props.children
}
</View>
);

if (_.isFunction(child.props.onBlur)) {
child.props.onBlur(el);
}
},
});
}
}

Expand Down
Loading