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

[No QA] Small refactors. #2744

Merged
merged 4 commits into from
May 13, 2021
Merged

[No QA] Small refactors. #2744

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6db8d76
chg: removed unnecessary re-render & helps in animations.
parasharrajat May 7, 2021
3c4833f
made condition easier to read
parasharrajat May 7, 2021
8f0dbc4
recfactor Tooltip
parasharrajat May 7, 2021
99101e0
updated props type
parasharrajat May 7, 2021
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
88 changes: 61 additions & 27 deletions src/components/Tooltip/TooltipRenderedOnPageBody.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,41 @@ import React, {memo} from 'react';
import PropTypes from 'prop-types';
import {Animated, Text, View} from 'react-native';
import ReactDOM from 'react-dom';
import getTooltipStyles from '../../styles/getTooltipStyles';

const propTypes = {
// Style for Animation
// eslint-disable-next-line react/forbid-prop-types
animationStyle: PropTypes.object.isRequired,
// Window width
windowWidth: PropTypes.number.isRequired,

// Syle for Tooltip wrapper
// Tooltip Animation value
// eslint-disable-next-line react/forbid-prop-types
tooltipWrapperStyle: PropTypes.object.isRequired,
animation: PropTypes.object.isRequired,

// Style for the text rendered inside tooltip
// eslint-disable-next-line react/forbid-prop-types
tooltipTextStyle: PropTypes.object.isRequired,
// The distance between the left side of the wrapper view and the left side of the window
xOffset: PropTypes.number.isRequired,

// Style for the Tooltip pointer Wrapper
// eslint-disable-next-line react/forbid-prop-types
pointerWrapperStyle: PropTypes.object.isRequired,
// The distance between the top of the wrapper view and the top of the window
yOffset: PropTypes.number.isRequired,

// Style for the Tooltip pointer
// eslint-disable-next-line react/forbid-prop-types
pointerStyle: PropTypes.object.isRequired,
// The width of the tooltip wrapper
wrapperWidth: PropTypes.number.isRequired,

// The Height of the tooltip wrapper
wrapperHeight: PropTypes.number.isRequired,

// The width of the tooltip itself
tooltipWidth: PropTypes.number.isRequired,

// The Height of the tooltip itself
tooltipHeight: PropTypes.number.isRequired,

// Any additional amount to manually adjust the horizontal position of the tooltip.
// A positive value shifts the tooltip to the right, and a negative value shifts it to the left.
shiftHorizontal: PropTypes.number.isRequired,

// Any additional amount to manually adjust the vertical position of the tooltip.
// A positive value shifts the tooltip down, and a negative value shifts it up.
shiftVertical: PropTypes.number.isRequired,

// Callback to set the Ref to the Tooltip
setTooltipRef: PropTypes.func.isRequired,
Expand All @@ -36,19 +50,39 @@ const propTypes = {

const defaultProps = {};

const TooltipRenderedOnPageBody = props => ReactDOM.createPortal(
<Animated.View
ref={props.setTooltipRef}
onLayout={props.measureTooltip}
style={[props.tooltipWrapperStyle, props.animationStyle]}
>
<Text style={props.tooltipTextStyle} numberOfLines={1}>{props.text}</Text>
<View style={props.pointerWrapperStyle}>
<View style={props.pointerStyle} />
</View>
</Animated.View>,
document.querySelector('body'),
);
const TooltipRenderedOnPageBody = (props) => {
const {
animationStyle,
tooltipWrapperStyle,
tooltipTextStyle,
pointerWrapperStyle,
pointerStyle,
} = getTooltipStyles(
props.animation,
props.windowWidth,
props.xOffset,
props.yOffset,
props.wrapperWidth,
props.wrapperHeight,
props.tooltipWidth,
props.tooltipHeight,
props.shiftHorizontal,
props.shiftVertical,
);
return ReactDOM.createPortal(
<Animated.View
ref={props.setTooltipRef}
onLayout={props.measureTooltip}
style={[tooltipWrapperStyle, animationStyle]}
>
<Text style={tooltipTextStyle} numberOfLines={1}>{props.text}</Text>
<View style={pointerWrapperStyle}>
<View style={pointerStyle} />
</View>
</Animated.View>,
document.querySelector('body'),
);
};

TooltipRenderedOnPageBody.propTypes = propTypes;
TooltipRenderedOnPageBody.defaultProps = defaultProps;
Expand Down
63 changes: 18 additions & 45 deletions src/components/Tooltip/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,16 @@ import {Animated, View} from 'react-native';
import TooltipRenderedOnPageBody from './TooltipRenderedOnPageBody';
import Hoverable from '../Hoverable';
import withWindowDimensions from '../withWindowDimensions';
import getTooltipStyles from '../../styles/getTooltipStyles';
import {propTypes, defaultProps} from './TooltipPropTypes';

class Tooltip extends PureComponent {
constructor(props) {
super(props);

this.state = {
// Is tooltip rendered?
isRendered: false,

// The distance between the left side of the wrapper view and the left side of the window
xOffset: 0,

Expand All @@ -38,7 +40,6 @@ class Tooltip extends PureComponent {
this.animation = new Animated.Value(0);

this.getWrapperPosition = this.getWrapperPosition.bind(this);
this.measureWrapperAndGetPosition = this.measureWrapperAndGetPosition.bind(this);
this.measureTooltip = this.measureTooltip.bind(this);
this.showTooltip = this.showTooltip.bind(this);
this.hideTooltip = this.hideTooltip.bind(this);
Expand Down Expand Up @@ -92,25 +93,6 @@ class Tooltip extends PureComponent {
}));
}

/**
* Measure the size and position of the wrapper view.
*
* @param {Object} nativeEvent
*/
measureWrapperAndGetPosition({nativeEvent}) {
const {width, height} = nativeEvent.layout;

// We need to use `measureInWindow` instead of the layout props provided by `onLayout`
// because `measureInWindow` provides the x and y offset relative to the window, rather than the parent element.
this.getWrapperPosition()
.then(({x, y}) => this.setStateIfMounted({
wrapperWidth: width,
wrapperHeight: height,
xOffset: x,
yOffset: y,
}));
}

/**
* Measure the size of the tooltip itself.
*
Expand All @@ -127,6 +109,9 @@ class Tooltip extends PureComponent {
* Display the tooltip in an animation.
*/
showTooltip() {
if (!this.state.isRendered) {
this.setState({isRendered: true});
}
this.animation.stopAnimation();
this.shouldStartShowAnimation = true;

Expand Down Expand Up @@ -168,44 +153,32 @@ class Tooltip extends PureComponent {
}

render() {
const {
animationStyle,
tooltipWrapperStyle,
tooltipTextStyle,
pointerWrapperStyle,
pointerStyle,
} = getTooltipStyles(
this.animation,
this.props.windowWidth,
this.state.xOffset,
this.state.yOffset,
this.state.wrapperWidth,
this.state.wrapperHeight,
this.state.tooltipWidth,
this.state.tooltipHeight,
_.result(this.props, 'shiftHorizontal'),
_.result(this.props, 'shiftVertical'),
);
return (
<>
{this.state.isRendered && (
<TooltipRenderedOnPageBody
animationStyle={animationStyle}
tooltipWrapperStyle={tooltipWrapperStyle}
tooltipTextStyle={tooltipTextStyle}
pointerWrapperStyle={pointerWrapperStyle}
pointerStyle={pointerStyle}
animation={this.animation}
windowWidth={this.props.windowWidth}
xOffset={this.state.xOffset}
yOffset={this.state.yOffset}
wrapperWidth={this.state.wrapperWidth}
wrapperHeight={this.state.wrapperHeight}
tooltipWidth={this.state.tooltipWidth}
tooltipHeight={this.state.tooltipHeight}
setTooltipRef={el => this.tooltip = el}
shiftHorizontal={_.result(this.props, 'shiftHorizontal')}
shiftVertical={_.result(this.props, 'shiftVertical')}
measureTooltip={this.measureTooltip}
text={this.props.text}
/>
)}
<Hoverable
containerStyle={this.props.containerStyle}
onHoverIn={this.showTooltip}
onHoverOut={this.hideTooltip}
>
<View
ref={el => this.wrapperView = el}
onLayout={this.measureWrapperAndGetPosition}
style={this.props.containerStyle}
>
{this.props.children}
Expand Down
4 changes: 3 additions & 1 deletion src/pages/home/report/ReportActionsView.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,10 @@ class ReportActionsView extends React.Component {
// The last sequenceNumber of the same report has changed.
const previousLastSequenceNumber = lodashGet(lastItem(prevProps.reportActions), 'sequenceNumber');
const currentLastSequenceNumber = lodashGet(lastItem(this.props.reportActions), 'sequenceNumber');

// Record the max action when window is visible except when Drawer is open on small screen
const shouldRecordMaxAction = Visibility.isVisible()
&& !(this.props.isDrawerOpen && this.props.isSmallScreenWidth);
&& (!this.props.isSmallScreenWidth || !this.props.isDrawerOpen);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we please add a comment here and list out every situations where we will record the max action?


if (previousLastSequenceNumber !== currentLastSequenceNumber) {
// If a new comment is added and it's from the current user scroll to the bottom otherwise
Expand Down