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

Highlight linked comment #27227

Merged
merged 25 commits into from
Oct 2, 2023
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
cb99986
create withRoute
perunt Sep 12, 2023
9ae049c
highlight background color If needed
perunt Sep 12, 2023
8d34f87
lint
perunt Sep 12, 2023
58dbaf3
add dependency
perunt Sep 12, 2023
0e2ebde
Merge branch 'main' of https://github.com/margelo/expensify-app-fork …
perunt Sep 15, 2023
a7db8b3
Merge branch 'main' of https://github.com/margelo/expensify-app-fork …
perunt Sep 19, 2023
43a586c
use hook instead of hoc
perunt Sep 19, 2023
a080732
use getBackgroundColorStyle
perunt Sep 22, 2023
77cf7f9
Merge branch 'main' of https://github.com/Expensify/App into feat/##2…
perunt Sep 22, 2023
836e394
highlighting money request
perunt Sep 23, 2023
2fdb1cb
change link when click active report in LHN
perunt Sep 23, 2023
2dea92d
clean
perunt Sep 23, 2023
7b7e985
trying to remove weird 'iou' error
perunt Sep 23, 2023
29c077d
trying to remove weird 'iou' error
perunt Sep 23, 2023
9e44fc3
trying to remove weird 'iou' error II
perunt Sep 23, 2023
c9e5fa5
restore to state before iou error
perunt Sep 23, 2023
a98f407
test highlightedBackgroundColorIfNeeded
perunt Sep 23, 2023
274df33
Merge branch 'main' of https://github.com/Expensify/App into feat/##2…
perunt Sep 23, 2023
e494d9f
Merge branch 'main' of https://github.com/Expensify/App into feat/##2…
perunt Sep 25, 2023
ea94621
return highlighting money request
perunt Sep 25, 2023
0c164d1
getTopmostReportActionID
perunt Sep 25, 2023
97d6dd4
lint
perunt Sep 25, 2023
1dd422a
Merge branch 'main' of https://github.com/Expensify/App into feat/##2…
perunt Sep 28, 2023
91384f5
move isReportActionLinked to props
perunt Sep 29, 2023
534ffba
spacing
perunt Sep 29, 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
5 changes: 5 additions & 0 deletions src/libs/Navigation/Navigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import linkingConfig from './linkingConfig';
import navigationRef from './navigationRef';
import NAVIGATORS from '../../NAVIGATORS';
import originalGetTopmostReportId from './getTopmostReportId';
import originalGetTopmostReportActionId from './getTopmostReportActionID';
import getStateFromPath from './getStateFromPath';
import SCREENS from '../../SCREENS';
import CONST from '../../CONST';
Expand Down Expand Up @@ -46,6 +47,9 @@ function canNavigate(methodName, params = {}) {
// Re-exporting the getTopmostReportId here to fill in default value for state. The getTopmostReportId isn't defined in this file to avoid cyclic dependencies.
const getTopmostReportId = (state = navigationRef.getState()) => originalGetTopmostReportId(state);

// Re-exporting the getTopmostReportActionID here to fill in default value for state. The getTopmostReportActionID isn't defined in this file to avoid cyclic dependencies.
const getTopmostReportActionId = (state = navigationRef.getState()) => originalGetTopmostReportActionId(state);

/**
* Method for finding on which index in stack we are.
* @param {Object} route
Expand Down Expand Up @@ -268,6 +272,7 @@ export default {
setIsNavigationReady,
getTopmostReportId,
getRouteNameFromStateEvent,
getTopmostReportActionId,
};

export {navigationRef};
42 changes: 42 additions & 0 deletions src/libs/Navigation/getTopmostReportActionID.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import lodashFindLast from 'lodash/findLast';
import lodashGet from 'lodash/get';

// This function is in a separate file than Navigation.js to avoid cyclic dependency.

/**
* Find the last visited report screen in the navigation state and get the linked reportActionID of it.
*
* @param {Object} state - The react-navigation state
* @returns {String | undefined} - It's possible that there is no report screen
*/
function getTopmostReportActionID(state) {
if (!state) {
return;
}
const topmostCentralPane = lodashFindLast(state.routes, (route) => route.name === 'CentralPaneNavigator');

if (!topmostCentralPane) {
return;
}

const directReportActionIDParam = lodashGet(topmostCentralPane, 'params.params.reportActionID');

if (!topmostCentralPane.state && !directReportActionIDParam) {
return;
}

if (directReportActionIDParam) {
return directReportActionIDParam;
}

const topmostReport = lodashFindLast(topmostCentralPane.state.routes, (route) => route.name === 'Report');
if (!topmostReport) {
return;
}

const topmostReportActionID = lodashGet(topmostReport, 'params.reportActionID');

return topmostReportActionID;
}

export default getTopmostReportActionID;
15 changes: 11 additions & 4 deletions src/pages/home/report/ReportActionItem.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import _ from 'underscore';
import lodashGet from 'lodash/get';
import React, {useState, useRef, useEffect, memo, useCallback, useContext} from 'react';
import React, {useState, useRef, useEffect, memo, useCallback, useContext, useMemo} from 'react';
import {InteractionManager, View} from 'react-native';
import PropTypes from 'prop-types';
import {withOnyx} from 'react-native-onyx';
import {useRoute} from '@react-navigation/native';
import CONST from '../../../CONST';
import ONYXKEYS from '../../../ONYXKEYS';
import reportActionPropTypes from './reportActionPropTypes';
Expand Down Expand Up @@ -61,12 +62,13 @@ import * as Session from '../../../libs/actions/Session';
import MoneyRequestView from '../../../components/ReportActionItem/MoneyRequestView';
import {hideContextMenu} from './ContextMenu/ReportActionContextMenu';
import * as PersonalDetailsUtils from '../../../libs/PersonalDetailsUtils';
import ReportActionItemBasicMessage from './ReportActionItemBasicMessage';
import * as store from '../../../libs/actions/ReimbursementAccount/store';
import * as BankAccounts from '../../../libs/actions/BankAccounts';
import {ReactionListContext} from '../ReportScreenContext';
import usePrevious from '../../../hooks/usePrevious';
import Permissions from '../../../libs/Permissions';
import themeColors from '../../../styles/themes/default';
import ReportActionItemBasicMessage from './ReportActionItemBasicMessage';
import RenderHTML from '../../../components/RenderHTML';
import ReportAttachmentsContext from './ReportAttachmentsContext';

Expand Down Expand Up @@ -127,6 +129,7 @@ const defaultProps = {
};

function ReportActionItem(props) {
const route = useRoute();
Copy link
Contributor

Choose a reason for hiding this comment

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

For performance, I think it's better useRoute only once in parent view (report screen.)
And pass only flag to ReportActionItem.
It's not performant to subscribe route change on every item.

Copy link
Contributor

Choose a reason for hiding this comment

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

@perunt ⬆️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In that case, we will rerender all messages if we want to highlight a message already on the screen.
I'll make changes in a moment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

const [isContextMenuActive, setIsContextMenuActive] = useState(ReportActionContextMenu.isActiveReportAction(props.action.reportActionID));
const [isHidden, setIsHidden] = useState(false);
const [moderationDecision, setModerationDecision] = useState(CONST.MODERATION.MODERATOR_DECISION_APPROVED);
Expand All @@ -138,6 +141,10 @@ function ReportActionItem(props) {
const prevDraftMessage = usePrevious(props.draftMessage);
const originalReportID = ReportUtils.getOriginalReportID(props.report.reportID, props.action);
const originalReport = props.report.reportID === originalReportID ? props.report : ReportUtils.getReport(originalReportID);
const reportActionID = lodashGet(route, 'params.reportActionID');
const isReportActionLinked = reportActionID === props.action.reportActionID;

const highlightedBackgroundColorIfNeeded = useMemo(() => (isReportActionLinked ? StyleUtils.getBackgroundColorStyle(themeColors.highlightBG) : {}), [isReportActionLinked]);

// When active action changes, we need to update the `isContextMenuActive` state
const isActiveReportActionForMenu = ReportActionContextMenu.isActiveReportAction(props.action.reportActionID);
Expand Down Expand Up @@ -594,7 +601,7 @@ function ReportActionItem(props) {
disabled={Boolean(props.draftMessage)}
>
{(hovered) => (
<View>
<View style={highlightedBackgroundColorIfNeeded}>
{props.shouldDisplayNewMarker && <UnreadActionIndicator reportActionID={props.action.reportActionID} />}
<MiniReportActionContextMenu
reportID={props.report.reportID}
Expand Down Expand Up @@ -643,7 +650,7 @@ function ReportActionItem(props) {
/>
</View>
)}
{renderReportActionItem(hovered, isWhisper, hasErrors)}
{renderReportActionItem(hovered || isReportActionLinked, isWhisper, hasErrors)}
</OfflineWithFeedback>
</View>
</View>
Expand Down
7 changes: 6 additions & 1 deletion src/pages/home/sidebar/SidebarLinks.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,12 @@ class SidebarLinks extends React.PureComponent {
// or when clicking the active LHN row on large screens
// or when continuously clicking different LHNs, only apply to small screen
// since getTopmostReportId always returns on other devices
if (this.props.isCreateMenuOpen || option.reportID === Navigation.getTopmostReportId() || (this.props.isSmallScreenWidth && this.props.isActiveReport(option.reportID))) {
const reportActionID = Navigation.getTopmostReportActionId();
if (
this.props.isCreateMenuOpen ||
(option.reportID === Navigation.getTopmostReportId() && !reportActionID) ||
(this.props.isSmallScreenWidth && this.props.isActiveReport(option.reportID) && !reportActionID)
) {
return;
}
Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(option.reportID));
Expand Down
Loading