From 076c1a5d086f9f9b2910e584d1cef412b6b7cb93 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Fri, 9 Apr 2021 14:52:21 -1000 Subject: [PATCH 1/8] make New indicator permanent until navigating away from chat; --- src/components/UnreadActionIndicator.js | 20 ++++----------- src/pages/home/report/ReportActionsView.js | 30 +++------------------- src/styles/styles.js | 12 ++------- src/styles/variables.js | 2 ++ 4 files changed, 13 insertions(+), 51 deletions(-) diff --git a/src/components/UnreadActionIndicator.js b/src/components/UnreadActionIndicator.js index 0a3fa8d3d95d..4cd062afeb82 100644 --- a/src/components/UnreadActionIndicator.js +++ b/src/components/UnreadActionIndicator.js @@ -1,29 +1,19 @@ import React from 'react'; -import {Animated, View} from 'react-native'; -import PropTypes from 'prop-types'; -import styles, {getOpacityStyle} from '../styles/styles'; +import {View} from 'react-native'; +import styles from '../styles/styles'; import Text from './Text'; -const propTypes = { - // Animated opacity - // eslint-disable-next-line react/forbid-prop-types - animatedOpacity: PropTypes.object.isRequired, -}; - -const UnreadActionIndicator = props => ( - ( + NEW - + ); -UnreadActionIndicator.propTypes = propTypes; UnreadActionIndicator.displayName = 'UnreadActionIndicator'; - export default UnreadActionIndicator; diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index d41e7a3c0d86..a829e875e40c 100644 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -71,7 +71,6 @@ class ReportActionsView extends React.Component { this.loadMoreChats = this.loadMoreChats.bind(this); this.sortedReportActions = []; this.timers = []; - this.unreadIndicatorOpacity = new Animated.Value(1); // Helper variable that keeps track of the unread action count before it updates to zero this.unreadActionCount = 0; @@ -89,7 +88,7 @@ class ReportActionsView extends React.Component { this.keyboardEvent = Keyboard.addListener('keyboardDidShow', this.scrollToListBottom); this.recordMaxAction(); fetchActions(this.props.reportID); - this.setUpUnreadActionIndicator(); + this.initialUnreadActionCount = this.props.report.unreadActionCount; Timing.end(CONST.TIMING.SWITCH_REPORT, CONST.TIMING.COLD); } @@ -146,25 +145,6 @@ class ReportActionsView extends React.Component { } } - /** - * Checks if the unreadActionIndicator should be shown. - * If it does, starts a timeout for the fading out animation and creates - * a flag to not show it again if the report is still open - */ - setUpUnreadActionIndicator() { - this.unreadActionCount = this.props.report.unreadActionCount; - - if (this.unreadActionCount > 0) { - this.unreadIndicatorOpacity = new Animated.Value(1); - this.timers.push(setTimeout(() => { - Animated.timing(this.unreadIndicatorOpacity, { - toValue: 0, - useNativeDriver: false, - }).start(); - }, 3000)); - } - } - /** * Retrieves the next set of report actions for the chat once we are nearing the end of what we are currently * displaying. @@ -232,8 +212,7 @@ class ReportActionsView extends React.Component { } /** - * When the bottom of the list is reached, this is triggered, so it's a little different than recording the max - * action when scrolled + * Recorded when the report first opens and when the list is scrolled to the bottom */ recordMaxAction() { const reportActions = lodashGet(this.props, 'reportActions', {}); @@ -306,8 +285,8 @@ class ReportActionsView extends React.Component { // are implemented on native and web/desktop which leads to // the unread indicator on native to render below the message instead of above it. - {this.unreadActionCount > 0 && index === this.unreadActionCount - 1 && ( - + {this.initialUnreadActionCount > 0 && index === this.initialUnreadActionCount - 1 && ( + )} this.actionListElement = el} diff --git a/src/styles/styles.js b/src/styles/styles.js index fae27fd6d72c..c4f16e1b7998 100644 --- a/src/styles/styles.js +++ b/src/styles/styles.js @@ -835,7 +835,7 @@ const styles = { left: 0, top: 0, bottom: 0, - zIndex: 2, + zIndex: variables.zIndexTop, }, navigationModalOverlay: { @@ -1153,6 +1153,7 @@ const styles = { paddingHorizontal: 20, flexDirection: 'row', alignItems: 'center', + zIndex: variables.zIndexMiddle, }, unreadIndicatorLine: { @@ -1520,14 +1521,6 @@ function getWidthAndHeightStyle(width, height) { }; } -/** - * @param {Number} opacity - * @returns {Object} - */ -function getOpacityStyle(opacity) { - return {opacity}; -} - /** * @param {Object} params * @returns {Object} @@ -1566,6 +1559,5 @@ export { getIconFillColor, getAnimatedFABStyle, getWidthAndHeightStyle, - getOpacityStyle, getModalPaddingStyles, }; diff --git a/src/styles/variables.js b/src/styles/variables.js index 9b70c5d5b072..3d3dbe3054b0 100644 --- a/src/styles/variables.js +++ b/src/styles/variables.js @@ -24,4 +24,6 @@ export default { sideBarWidth: 375, pdfPageMaxWidth: 992, tooltipzIndex: 10050, + zIndexMiddle: 50, + zIndexTop: 999, }; From b824e84a216be653793a86667e50b314621c6d57 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Fri, 9 Apr 2021 15:04:44 -1000 Subject: [PATCH 2/8] remove animated --- src/pages/home/report/ReportActionsView.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index a829e875e40c..e40993b6a1f1 100644 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -1,6 +1,5 @@ import React from 'react'; import { - Animated, View, Keyboard, AppState, From b465ce39661d5e9c0d1a9e85e3bad6c8b6bd5472 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 12 Apr 2021 06:30:21 -1000 Subject: [PATCH 3/8] Move indicator into ReportActionItem --- src/pages/home/report/ReportActionItem.js | 6 ++++++ src/pages/home/report/ReportActionsView.js | 13 ++++++------- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js index 3cc279702809..4079b543833a 100644 --- a/src/pages/home/report/ReportActionItem.js +++ b/src/pages/home/report/ReportActionItem.js @@ -13,6 +13,7 @@ import PopoverWithMeasuredContent from '../../../components/PopoverWithMeasuredC import ReportActionItemSingle from './ReportActionItemSingle'; import ReportActionItemGrouped from './ReportActionItemGrouped'; import ReportActionContextMenu from './ReportActionContextMenu'; +import UnreadActionIndicator from '../../../components/UnreadActionIndicator'; const propTypes = { // The ID of the report this action is on. @@ -23,6 +24,9 @@ const propTypes = { // Should the comment have the appearance of being grouped with the previous comment? displayAsGroup: PropTypes.bool.isRequired, + + // Should we display the new indicator on top of the comment? + shouldDisplayNewIndicator: PropTypes.bool.isRequired, }; class ReportActionItem extends Component { @@ -46,6 +50,7 @@ class ReportActionItem extends Component { shouldComponentUpdate(nextProps, nextState) { return this.state.isPopoverVisible !== nextState.isPopoverVisible || this.props.displayAsGroup !== nextProps.displayAsGroup + || (this.props.shouldDisplayNewIndicator !== nextProps.shouldDisplayNewIndicator) || !_.isEqual(this.props.action, nextProps.action); } @@ -85,6 +90,7 @@ class ReportActionItem extends Component { {hovered => ( + {this.props.shouldDisplayNewIndicator && } {!this.props.displayAsGroup ? diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index e40993b6a1f1..b46f363794ff 100644 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -10,7 +10,6 @@ import _ from 'underscore'; import lodashGet from 'lodash/get'; import {withOnyx} from 'react-native-onyx'; import Text from '../../../components/Text'; -import UnreadActionIndicator from '../../../components/UnreadActionIndicator'; import { fetchActions, updateLastReadActionID, @@ -280,18 +279,18 @@ class ReportActionsView extends React.Component { }) { return ( - // Using instead of a Fragment because there is a difference between how - // are implemented on native and web/desktop which leads to - // the unread indicator on native to render below the message instead of above it. + // Using instead of a Fragment because there is a difference between how are + // implemented on native and web/desktop which leads to the unread indicator on native to render below the + // message instead of above it. - {this.initialUnreadActionCount > 0 && index === this.initialUnreadActionCount - 1 && ( - - )} 0 + && index === this.initialUnreadActionCount - 1 + } /> ); From 126274056af87da3e25ff1a0005df9b573f3148c Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 12 Apr 2021 07:48:19 -1000 Subject: [PATCH 4/8] Use maxSequenceNumber since chats can be paginated --- src/pages/home/report/ReportActionsView.js | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index b46f363794ff..4f17d0931cb3 100644 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -37,6 +37,9 @@ const propTypes = { report: PropTypes.shape({ // Number of actions unread unreadActionCount: PropTypes.number, + + // The largest sequenceNumber on this report + maxSequenceNumber: PropTypes.number, }), // Array of report actions for this report @@ -52,6 +55,7 @@ const propTypes = { const defaultProps = { report: { unreadActionCount: 0, + maxSequenceNumber: 0, }, reportActions: {}, session: {}, @@ -70,8 +74,9 @@ class ReportActionsView extends React.Component { this.sortedReportActions = []; this.timers = []; - // Helper variable that keeps track of the unread action count before it updates to zero - this.unreadActionCount = 0; + this.initialNewMarkerPosition = props.report.unreadActionCount === 0 + ? 0 + : (props.report.maxSequenceNumber + 1) - props.report.unreadActionCount; this.state = { isLoadingMoreChats: false, @@ -86,7 +91,6 @@ class ReportActionsView extends React.Component { this.keyboardEvent = Keyboard.addListener('keyboardDidShow', this.scrollToListBottom); this.recordMaxAction(); fetchActions(this.props.reportID); - this.initialUnreadActionCount = this.props.report.unreadActionCount; Timing.end(CONST.TIMING.SWITCH_REPORT, CONST.TIMING.COLD); } @@ -288,9 +292,8 @@ class ReportActionsView extends React.Component { action={item.action} displayAsGroup={this.isConsecutiveActionMadeByPreviousActor(index)} onLayout={onLayout} - shouldDisplayNewIndicator={this.initialUnreadActionCount > 0 - && index === this.initialUnreadActionCount - 1 - } + shouldDisplayNewIndicator={this.initialNewMarkerPosition > 0 + && item.action.sequenceNumber === this.initialNewMarkerPosition} /> ); From 3a9c16478e51bf47f5fe32b7257bf31c7b109ce3 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 12 Apr 2021 14:45:40 -1000 Subject: [PATCH 5/8] undo zindex stuff --- src/pages/home/report/ReportActionItem.js | 1 - src/pages/home/report/ReportActionsView.js | 6 ++++-- src/styles/styles.js | 2 +- src/styles/variables.js | 1 - 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js index 94976b97f04d..0bb1dd957b03 100644 --- a/src/pages/home/report/ReportActionItem.js +++ b/src/pages/home/report/ReportActionItem.js @@ -90,7 +90,6 @@ class ReportActionItem extends Component { {hovered => ( - {this.props.shouldDisplayNewIndicator && } {!this.props.displayAsGroup ? diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index 4f17d0931cb3..c21e5df51ec8 100644 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -26,6 +26,7 @@ import Visibility from '../../../libs/Visibility'; import Timing from '../../../libs/actions/Timing'; import CONST from '../../../CONST'; import themeColors from '../../../styles/themes/default'; +import UnreadActionIndicator from '../../../components/UnreadActionIndicator'; const propTypes = { // The ID of the report actions will be created for @@ -246,8 +247,8 @@ class ReportActionsView extends React.Component { /** * This function overrides the CellRendererComponent (defaults to a plain View), giving each ReportActionItem a - * higher z-index than the one below it. This prevents issues where the ReportActionContextMenu overlapping between - * rows is hidden beneath other rows. + * higher z-index than the one below it. This prevents issues where the ReportActionContextMenu overlapping between + * rows is hidden beneath other rows. * * @param {Object} index - The ReportAction item in the FlatList. * @param {Object|Array} style – The default styles of the CellRendererComponent provided by the CellRenderer. @@ -287,6 +288,7 @@ class ReportActionsView extends React.Component { // implemented on native and web/desktop which leads to the unread indicator on native to render below the // message instead of above it. + Date: Mon, 12 Apr 2021 17:11:16 -1000 Subject: [PATCH 6/8] Fix Android bug. Undo a few things --- src/pages/home/report/ReportActionItem.js | 3 +++ src/pages/home/report/ReportActionsView.js | 25 ++++++---------------- src/styles/getReportActionItemStyles.js | 7 +++++- src/styles/styles.js | 2 +- src/styles/variables.js | 1 - 5 files changed, 17 insertions(+), 21 deletions(-) diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js index 0bb1dd957b03..7d0608ce4aa2 100644 --- a/src/pages/home/report/ReportActionItem.js +++ b/src/pages/home/report/ReportActionItem.js @@ -90,6 +90,9 @@ class ReportActionItem extends Component { {hovered => ( + {!hovered && this.props.shouldDisplayNewIndicator && ( + + )} {!this.props.displayAsGroup ? diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index c21e5df51ec8..347dd3d5ee48 100644 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -26,7 +26,6 @@ import Visibility from '../../../libs/Visibility'; import Timing from '../../../libs/actions/Timing'; import CONST from '../../../CONST'; import themeColors from '../../../styles/themes/default'; -import UnreadActionIndicator from '../../../components/UnreadActionIndicator'; const propTypes = { // The ID of the report actions will be created for @@ -273,31 +272,21 @@ class ReportActionsView extends React.Component { * @param {Object} args * @param {Object} args.item * @param {Number} args.index - * @param {Function} args.onLayout * * @returns {React.Component} */ renderItem({ item, index, - onLayout, }) { return ( - - // Using instead of a Fragment because there is a difference between how are - // implemented on native and web/desktop which leads to the unread indicator on native to render below the - // message instead of above it. - - - 0 - && item.action.sequenceNumber === this.initialNewMarkerPosition} - /> - + 0 + && item.action.sequenceNumber === this.initialNewMarkerPosition} + /> ); } diff --git a/src/styles/getReportActionItemStyles.js b/src/styles/getReportActionItemStyles.js index 20054eb0eef0..7923c085b536 100644 --- a/src/styles/getReportActionItemStyles.js +++ b/src/styles/getReportActionItemStyles.js @@ -1,3 +1,4 @@ +import colors from './colors'; import themeColors from './themes/default'; import positioning from './utilities/positioning'; @@ -11,7 +12,11 @@ export function getReportActionItemStyle(isHovered = false) { return { display: 'flex', justifyContent: 'space-between', - backgroundColor: isHovered ? themeColors.hoverComponentBG : themeColors.componentBG, + backgroundColor: isHovered + ? themeColors.hoverComponentBG + + // Warning: Setting this to a non-transparent color will cause unread indicator to break on Android + : colors.transparent, cursor: 'default', }; } diff --git a/src/styles/styles.js b/src/styles/styles.js index c853938ddaf0..4e7ccabc0633 100644 --- a/src/styles/styles.js +++ b/src/styles/styles.js @@ -833,7 +833,7 @@ const styles = { left: 0, top: 0, bottom: 0, - zIndex: variables.zIndexTop, + zIndex: 2, }, navigationModalOverlay: { diff --git a/src/styles/variables.js b/src/styles/variables.js index 7c893c1199b6..9b70c5d5b072 100644 --- a/src/styles/variables.js +++ b/src/styles/variables.js @@ -24,5 +24,4 @@ export default { sideBarWidth: 375, pdfPageMaxWidth: 992, tooltipzIndex: 10050, - zIndexTop: 999, }; From 9b73413ef514786dc1771f407e0c5affaf5d1d69 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Tue, 13 Apr 2021 05:37:17 -1000 Subject: [PATCH 7/8] remove unused style --- src/styles/styles.js | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/styles/styles.js b/src/styles/styles.js index 4e7ccabc0633..f50e34339a47 100644 --- a/src/styles/styles.js +++ b/src/styles/styles.js @@ -828,14 +828,6 @@ const styles = { marginRight: 4, }, - navigationMenuOpenAbsolute: { - position: 'absolute', - left: 0, - top: 0, - bottom: 0, - zIndex: 2, - }, - navigationModalOverlay: { position: 'absolute', width: '100%', From 6746749096df83b9de19f21070dd84e964abd671 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Tue, 13 Apr 2021 07:04:03 -1000 Subject: [PATCH 8/8] clean up style --- src/components/UnreadActionIndicator.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/components/UnreadActionIndicator.js b/src/components/UnreadActionIndicator.js index 4cd062afeb82..7c6789c0a66a 100644 --- a/src/components/UnreadActionIndicator.js +++ b/src/components/UnreadActionIndicator.js @@ -4,10 +4,7 @@ import styles from '../styles/styles'; import Text from './Text'; const UnreadActionIndicator = () => ( - + NEW