Skip to content

Commit

Permalink
Merge pull request #30269 from margelo/feat/##23229-linking
Browse files Browse the repository at this point in the history
HIGH: (Comment linking: Step 4) Main "glue" PR for Comment Linking
  • Loading branch information
roryabraham authored Mar 20, 2024
2 parents 68a96ed + cb86c85 commit b306886
Show file tree
Hide file tree
Showing 21 changed files with 1,938 additions and 304 deletions.
3 changes: 3 additions & 0 deletions src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3417,6 +3417,9 @@ const CONST = {

REPORT_FIELD_TITLE_FIELD_ID: 'text_title',

MOBILE_PAGINATION_SIZE: 15,
WEB_PAGINATION_SIZE: 50,

/** Dimensions for illustration shown in Confirmation Modal */
CONFIRM_CONTENT_SVG_SIZE: {
HEIGHT: 220,
Expand Down
16 changes: 8 additions & 8 deletions src/components/FlatList/MVCPFlatList.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,15 @@ const MVCPFlatList = React.forwardRef(({maintainVisibleContentPosition, horizont
if (scrollRef.current == null) {
return 0;
}
return horizontal ? scrollRef.current.getScrollableNode().scrollLeft : scrollRef.current.getScrollableNode().scrollTop;
return horizontal ? scrollRef.current?.getScrollableNode()?.scrollLeft : scrollRef.current?.getScrollableNode()?.scrollTop;
}, [horizontal]);

const getContentView = React.useCallback(() => scrollRef.current?.getScrollableNode().childNodes[0], []);
const getContentView = React.useCallback(() => scrollRef.current?.getScrollableNode()?.childNodes[0], []);

const scrollToOffset = React.useCallback(
(offset, animated) => {
const behavior = animated ? 'smooth' : 'instant';
scrollRef.current?.getScrollableNode().scroll(horizontal ? {left: offset, behavior} : {top: offset, behavior});
scrollRef.current?.getScrollableNode()?.scroll(horizontal ? {left: offset, behavior} : {top: offset, behavior});
},
[horizontal],
);
Expand All @@ -68,12 +68,13 @@ const MVCPFlatList = React.forwardRef(({maintainVisibleContentPosition, horizont
}

const scrollOffset = getScrollOffset();
lastScrollOffsetRef.current = scrollOffset;

const contentViewLength = contentView.childNodes.length;
for (let i = mvcpMinIndexForVisible; i < contentViewLength; i++) {
const subview = contentView.childNodes[i];
const subviewOffset = horizontal ? subview.offsetLeft : subview.offsetTop;
if (subviewOffset > scrollOffset || i === contentViewLength - 1) {
if (subviewOffset > scrollOffset) {
prevFirstVisibleOffsetRef.current = subviewOffset;
firstVisibleViewRef.current = subview;
break;
Expand Down Expand Up @@ -126,6 +127,7 @@ const MVCPFlatList = React.forwardRef(({maintainVisibleContentPosition, horizont
}

adjustForMaintainVisibleContentPosition();
prepareForMaintainVisibleContentPosition();
});
});
mutationObserver.observe(contentView, {
Expand All @@ -135,7 +137,7 @@ const MVCPFlatList = React.forwardRef(({maintainVisibleContentPosition, horizont
});

mutationObserverRef.current = mutationObserver;
}, [adjustForMaintainVisibleContentPosition, getContentView, getScrollOffset, scrollToOffset]);
}, [adjustForMaintainVisibleContentPosition, prepareForMaintainVisibleContentPosition, getContentView, getScrollOffset, scrollToOffset]);

React.useEffect(() => {
if (!isListRenderedRef.current) {
Expand Down Expand Up @@ -172,13 +174,11 @@ const MVCPFlatList = React.forwardRef(({maintainVisibleContentPosition, horizont

const onScrollInternal = React.useCallback(
(ev) => {
lastScrollOffsetRef.current = getScrollOffset();

prepareForMaintainVisibleContentPosition();

onScroll?.(ev);
},
[getScrollOffset, prepareForMaintainVisibleContentPosition, onScroll],
[prepareForMaintainVisibleContentPosition, onScroll],
);

return (
Expand Down
34 changes: 24 additions & 10 deletions src/components/InvertedFlatList/BaseInvertedFlatList.tsx
Original file line number Diff line number Diff line change
@@ -1,23 +1,35 @@
import type {ForwardedRef} from 'react';
import React, {forwardRef} from 'react';
import type {FlatListProps} from 'react-native';
import React, {forwardRef, useMemo} from 'react';
import type {FlatListProps, ScrollViewProps} from 'react-native';
import FlatList from '@components/FlatList';

const WINDOW_SIZE = 15;
type BaseInvertedFlatListProps<T> = FlatListProps<T> & {
shouldEnableAutoScrollToTopThreshold?: boolean;
};

const AUTOSCROLL_TO_TOP_THRESHOLD = 128;

const maintainVisibleContentPosition = {
minIndexForVisible: 0,
autoscrollToTopThreshold: AUTOSCROLL_TO_TOP_THRESHOLD,
};
function BaseInvertedFlatList<T>(props: BaseInvertedFlatListProps<T>, ref: ForwardedRef<FlatList>) {
const {shouldEnableAutoScrollToTopThreshold, ...rest} = props;

const maintainVisibleContentPosition = useMemo(() => {
const config: ScrollViewProps['maintainVisibleContentPosition'] = {
// This needs to be 1 to avoid using loading views as anchors.
minIndexForVisible: 1,
};

if (shouldEnableAutoScrollToTopThreshold) {
config.autoscrollToTopThreshold = AUTOSCROLL_TO_TOP_THRESHOLD;
}

return config;
}, [shouldEnableAutoScrollToTopThreshold]);

function BaseInvertedFlatList<T>(props: FlatListProps<T>, ref: ForwardedRef<FlatList>) {
return (
<FlatList
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
{...rest}
ref={ref}
windowSize={WINDOW_SIZE}
maintainVisibleContentPosition={maintainVisibleContentPosition}
inverted
/>
Expand All @@ -27,3 +39,5 @@ function BaseInvertedFlatList<T>(props: FlatListProps<T>, ref: ForwardedRef<Flat
BaseInvertedFlatList.displayName = 'BaseInvertedFlatList';

export default forwardRef(BaseInvertedFlatList);

export {AUTOSCROLL_TO_TOP_THRESHOLD};
6 changes: 5 additions & 1 deletion src/components/InvertedFlatList/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,13 @@ import CONST from '@src/CONST';
import BaseInvertedFlatList from './BaseInvertedFlatList';
import CellRendererComponent from './CellRendererComponent';

type InvertedFlatListProps<T> = FlatListProps<T> & {
shouldEnableAutoScrollToTopThreshold?: boolean;
};

// This is adapted from https://codesandbox.io/s/react-native-dsyse
// It's a HACK alert since FlatList has inverted scrolling on web
function InvertedFlatList<T>({onScroll: onScrollProp = () => {}, ...props}: FlatListProps<T>, ref: ForwardedRef<FlatList>) {
function InvertedFlatList<T>({onScroll: onScrollProp = () => {}, ...props}: InvertedFlatListProps<T>, ref: ForwardedRef<FlatList>) {
const lastScrollEvent = useRef<number | null>(null);
const scrollEndTimeout = useRef<NodeJS.Timeout | null>(null);
const updateInProgress = useRef<boolean>(false);
Expand Down
1 change: 1 addition & 0 deletions src/libs/API/parameters/OpenReportParams.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
type OpenReportParams = {
reportID: string;
reportActionID?: string;
emailList?: string;
accountIDList?: string;
parentReportActionID?: string;
Expand Down
10 changes: 9 additions & 1 deletion src/libs/NumberUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,12 @@ function clamp(value: number, min: number, max: number): number {
return Math.min(Math.max(value, min), max);
}

export {rand64, generateHexadecimalValue, generateRandomInt, parseFloatAnyLocale, roundDownToLargestMultiple, roundToTwoDecimalPlaces, clamp};
function generateNewRandomInt(old: number, min: number, max: number): number {
let newNum = old;
while (newNum === old) {
newNum = generateRandomInt(min, max);
}
return newNum;
}

export {rand64, generateHexadecimalValue, generateRandomInt, parseFloatAnyLocale, roundDownToLargestMultiple, roundToTwoDecimalPlaces, clamp, generateNewRandomInt};
81 changes: 68 additions & 13 deletions src/libs/ReportActionsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ type MemberChangeMessageElement = MessageTextElement | MemberChangeMessageUserMe
const policyChangeActionsSet = new Set<string>(Object.values(CONST.REPORT.ACTIONS.TYPE.POLICYCHANGELOG));

const allReports: OnyxCollection<Report> = {};

Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT,
callback: (report, key) => {
Expand Down Expand Up @@ -219,7 +220,7 @@ function isTransactionThread(parentReportAction: OnyxEntry<ReportAction> | Empty
* This gives us a stable order even in the case of multiple reportActions created on the same millisecond
*
*/
function getSortedReportActions(reportActions: ReportAction[] | null, shouldSortInDescendingOrder = false, shouldMarkTheFirstItemAsNewest = false): ReportAction[] {
function getSortedReportActions(reportActions: ReportAction[] | null, shouldSortInDescendingOrder = false): ReportAction[] {
if (!Array.isArray(reportActions)) {
throw new Error(`ReportActionsUtils.getSortedReportActions requires an array, received ${typeof reportActions}`);
}
Expand Down Expand Up @@ -247,15 +248,58 @@ function getSortedReportActions(reportActions: ReportAction[] | null, shouldSort
return (first.reportActionID < second.reportActionID ? -1 : 1) * invertedMultiplier;
});

// If shouldMarkTheFirstItemAsNewest is true, label the first reportAction as isNewestReportAction
if (shouldMarkTheFirstItemAsNewest && sortedActions?.length > 0) {
sortedActions[0] = {
...sortedActions[0],
isNewestReportAction: true,
};
return sortedActions;
}

/**
* Returns the largest gapless range of reportActions including a the provided reportActionID, where a "gap" is defined as a reportAction's `previousReportActionID` not matching the previous reportAction in the sortedReportActions array.
* See unit tests for example of inputs and expected outputs.
* Note: sortedReportActions sorted in descending order
*/
function getContinuousReportActionChain(sortedReportActions: ReportAction[], id?: string): ReportAction[] {
let index;

if (id) {
index = sortedReportActions.findIndex((obj) => obj.reportActionID === id);
} else {
index = sortedReportActions.findIndex((obj) => obj.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD);
}

return sortedActions;
if (index === -1) {
return [];
}

let startIndex = index;
let endIndex = index;

// Iterate forwards through the array, starting from endIndex. This loop checks the continuity of actions by:
// 1. Comparing the current item's previousReportActionID with the next item's reportActionID.
// This ensures that we are moving in a sequence of related actions from newer to older.
while (
(endIndex < sortedReportActions.length - 1 && sortedReportActions[endIndex].previousReportActionID === sortedReportActions[endIndex + 1].reportActionID) ||
!!sortedReportActions[endIndex + 1]?.whisperedToAccountIDs?.length ||
!!sortedReportActions[endIndex]?.whisperedToAccountIDs?.length ||
sortedReportActions[endIndex]?.actionName === CONST.REPORT.ACTIONS.TYPE.ROOMCHANGELOG.INVITE_TO_ROOM ||
sortedReportActions[endIndex + 1]?.actionName === CONST.REPORT.ACTIONS.TYPE.CLOSED ||
sortedReportActions[endIndex + 1]?.actionName === CONST.REPORT.ACTIONS.TYPE.CREATED
) {
endIndex++;
}

// Iterate backwards through the sortedReportActions, starting from startIndex. This loop has two main checks:
// 1. It compares the current item's reportActionID with the previous item's previousReportActionID.
// This is to ensure continuity in a sequence of actions.
// 2. If the first condition fails, it then checks if the previous item has a pendingAction of 'add'.
// This additional check is to include recently sent messages that might not yet be part of the established sequence.
while (
(startIndex > 0 && sortedReportActions[startIndex].reportActionID === sortedReportActions[startIndex - 1].previousReportActionID) ||
sortedReportActions[startIndex - 1]?.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD ||
sortedReportActions[startIndex - 1]?.actionName === CONST.REPORT.ACTIONS.TYPE.ROOMCHANGELOG.INVITE_TO_ROOM
) {
startIndex--;
}

return sortedReportActions.slice(startIndex, endIndex + 1);
}

/**
Expand Down Expand Up @@ -525,12 +569,22 @@ function filterOutDeprecatedReportActions(reportActions: ReportActions | null):
* to ensure they will always be displayed in the same order (in case multiple actions have the same timestamp).
* This is all handled with getSortedReportActions() which is used by several other methods to keep the code DRY.
*/
function getSortedReportActionsForDisplay(reportActions: ReportActions | ReportAction[] | null, shouldMarkTheFirstItemAsNewest = false): ReportAction[] {
const filteredReportActions = Object.entries(reportActions ?? {})
.filter(([key, reportAction]) => shouldReportActionBeVisible(reportAction, key))
.map((entry) => entry[1]);
function getSortedReportActionsForDisplay(reportActions: ReportActions | null | ReportAction[], shouldIncludeInvisibleActions = false): ReportAction[] {
let filteredReportActions: ReportAction[] = [];
if (!reportActions) {
return [];
}

if (shouldIncludeInvisibleActions) {
filteredReportActions = Object.values(reportActions);
} else {
filteredReportActions = Object.entries(reportActions)
.filter(([key, reportAction]) => shouldReportActionBeVisible(reportAction, key))
.map(([, reportAction]) => reportAction);
}

const baseURLAdjustedReportActions = filteredReportActions.map((reportAction) => replaceBaseURLInPolicyChangeLogAction(reportAction));
return getSortedReportActions(baseURLAdjustedReportActions, true, shouldMarkTheFirstItemAsNewest);
return getSortedReportActions(baseURLAdjustedReportActions, true);
}

/**
Expand Down Expand Up @@ -1012,6 +1066,7 @@ export {
shouldReportActionBeVisible,
shouldHideNewMarker,
shouldReportActionBeVisibleAsLastAction,
getContinuousReportActionChain,
hasRequestFromCurrentAccount,
getFirstVisibleReportActionID,
isMemberChangeAction,
Expand Down
15 changes: 8 additions & 7 deletions src/libs/actions/Policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -866,13 +866,14 @@ function removeMembers(accountIDs: number[], policyID: string) {
},
});
});
optimisticClosedReportActions.forEach((reportAction, index) => {
optimisticData.push({
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${workspaceChats?.[index]?.reportID}`,
value: {[reportAction.reportActionID]: reportAction as ReportAction},
});
});
// comment out for time this issue would be resolved https://github.com/Expensify/App/issues/35952
// optimisticClosedReportActions.forEach((reportAction, index) => {
// optimisticData.push({
// onyxMethod: Onyx.METHOD.MERGE,
// key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${workspaceChats?.[index]?.reportID}`,
// value: {[reportAction.reportActionID]: reportAction as ReportAction},
// });
// });

// If the policy has primaryLoginsInvited, then it displays informative messages on the members page about which primary logins were added by secondary logins.
// If we delete all these logins then we should clear the informative messages since they are no longer relevant.
Expand Down
Loading

0 comments on commit b306886

Please sign in to comment.