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

Bidirectional pagination #26166

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
69 commits
Select commit Hold shift + click to select a range
426707f
add getOlderAction and getNewerAction API
perunt Aug 29, 2023
cfa1c1e
use getOlderAction instead of getOlderAction
perunt Aug 29, 2023
89fefbf
Merge branch 'main' of https://github.com/margelo/expensify-app-fork …
perunt Aug 29, 2023
741a3b9
Merge branch 'main' of https://github.com/Expensify/App into feat/##2…
perunt Sep 1, 2023
a82dc0f
Merge branch 'main' of https://github.com/Expensify/App into feat/##2…
perunt Sep 4, 2023
2619fa8
Merge branch 'main' of https://github.com/Expensify/App into feat/##2…
perunt Sep 5, 2023
42fe76a
update getNewerReportActions
perunt Sep 6, 2023
3df7a19
Merge branch 'main' of https://github.com/Expensify/App into feat/##2…
perunt Sep 6, 2023
66483bf
remove comments
perunt Sep 9, 2023
a480524
Merge branch 'main' of https://github.com/margelo/expensify-app-fork …
perunt Sep 11, 2023
2f69a83
maxToRenderPerBatch=5
perunt Sep 11, 2023
694c423
Merge branch 'main' of https://github.com/margelo/expensify-app-fork …
perunt Sep 15, 2023
35076a4
change loader
perunt Sep 15, 2023
fe22397
fix loop
perunt Sep 17, 2023
e6f23df
Merge branch 'main' of https://github.com/margelo/expensify-app-fork …
perunt Sep 18, 2023
045b07d
Merge branch 'main' of https://github.com/margelo/expensify-app-fork …
perunt Sep 19, 2023
a9243e3
use distanceFromStart for avoiding rerenders
perunt Sep 19, 2023
99cead6
prettier
perunt Sep 19, 2023
bd27f36
clean
perunt Sep 19, 2023
063ef90
Merge branch 'main' of https://github.com/margelo/expensify-app-fork …
perunt Sep 20, 2023
9fb4792
use the separate component for android for ListHeaderComponentLoader
perunt Sep 20, 2023
84d1953
lint
perunt Sep 20, 2023
cfe420d
rename isLoadingReportActions
perunt Sep 21, 2023
120eefa
add comment
perunt Sep 21, 2023
484f596
add throttle
perunt Sep 21, 2023
bde5a21
Merge branch 'main' of https://github.com/margelo/expensify-app-fork …
perunt Sep 21, 2023
821adb8
Merge branch 'main' into feat/##23220-bidirectional-pagination
perunt Sep 21, 2023
652967c
Merge branch 'feat/##23220-bidirectional-pagination' of https://githu…
perunt Sep 21, 2023
5ef510d
create a shared component for header and footer
perunt Sep 21, 2023
fae75dd
lint
perunt Sep 21, 2023
2d094f0
add const LIST_COMPONENTS
perunt Sep 21, 2023
5109429
add JSDoc
perunt Sep 22, 2023
84cb84c
Merge branch 'main' of https://github.com/Expensify/App into feat/##2…
perunt Sep 22, 2023
0849919
fix after merge
perunt Sep 22, 2023
a4f6834
remove redundant dependency
perunt Sep 22, 2023
1829ade
Merge branch 'main' of https://github.com/Expensify/App into feat/##2…
perunt Sep 22, 2023
063af42
Merge branch 'main' of https://github.com/Expensify/App into feat/##2…
perunt Sep 25, 2023
7dfc82e
Merge branch 'main' of https://github.com/Expensify/App into feat/##2…
perunt Sep 28, 2023
1387d21
Fix report metadata
perunt Sep 28, 2023
a3e609d
fix after merge
perunt Sep 28, 2023
35f1ad3
lint
perunt Sep 28, 2023
6839c22
Merge branch 'main' of https://github.com/Expensify/App into feat/##2…
perunt Oct 2, 2023
297e8ec
reduce skeleton size to 3 items for fetchMore
perunt Oct 2, 2023
6a863a4
remove duplications of REPORT_METADATA
perunt Oct 2, 2023
c1201d5
Merge branch 'main' of https://github.com/Expensify/App into feat/##2…
perunt Oct 4, 2023
d48bdc8
lint after merge
perunt Oct 4, 2023
34ea6f8
spacing
perunt Oct 4, 2023
bd08f5e
comments update
perunt Oct 5, 2023
f0ae93f
Merge branch 'main' of https://github.com/Expensify/App into feat/##2…
perunt Oct 5, 2023
244cff4
Merge branch 'main' of https://github.com/Expensify/App into feat/##2…
perunt Oct 9, 2023
6dd8147
Merge branch 'main' of https://github.com/Expensify/App into feat/##2…
perunt Oct 9, 2023
cfb4bee
scroll to bottom
perunt Oct 9, 2023
9043f69
update after review
perunt Oct 9, 2023
8ebb468
lint types
perunt Oct 9, 2023
bf9c309
clean redundant const
perunt Oct 10, 2023
f046ce1
add comment to isFirstRender
perunt Oct 10, 2023
f45bc44
update fetchReportIfNeeded
perunt Oct 10, 2023
06aeba6
replace isFetchNewerWasCalled with isFirstRender
perunt Oct 11, 2023
b9b966c
Merge branch 'main' of https://github.com/Expensify/App into feat/##2…
perunt Oct 11, 2023
9b04dd6
Merge branch 'main' of https://github.com/Expensify/App into feat/##2…
perunt Oct 16, 2023
d5c16b0
add styles after merge
perunt Oct 16, 2023
d185048
fix after switching styles to TS
perunt Oct 18, 2023
463f96c
Merge branch 'main' of https://github.com/Expensify/App into feat/##2…
perunt Oct 19, 2023
fece482
undo possibleVisibleContentItems from default props
perunt Oct 20, 2023
6c6adc4
lint, displayName
perunt Oct 20, 2023
92becc1
Merge branch 'main' of https://github.com/Expensify/App into feat/##2…
perunt Oct 23, 2023
45f2f46
spacing
perunt Oct 24, 2023
d508082
Merge branch 'main' of https://github.com/Expensify/App into feat/##2…
perunt Oct 24, 2023
c9f0620
import useNetwork after merge
perunt Oct 24, 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
7 changes: 7 additions & 0 deletions src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2779,13 +2779,20 @@ const CONST = {
SCROLLING: 'scrolling',
},

CHAT_HEADER_LOADER_HEIGHT: 36,

HORIZONTAL_SPACER: {
DEFAULT_BORDER_BOTTOM_WIDTH: 1,
DEFAULT_MARGIN_VERTICAL: 8,
HIDDEN_MARGIN_VERTICAL: 0,
HIDDEN_BORDER_BOTTOM_WIDTH: 0,
},

LIST_COMPONENTS: {
HEADER: 'header',
FOOTER: 'footer',
},

GLOBAL_NAVIGATION_OPTION: {
HOME: 'home',
CHATS: 'chats',
Expand Down
2 changes: 1 addition & 1 deletion src/ONYXKEYS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ const ONYXKEYS = {
POLICY_RECENTLY_USED_TAGS: 'policyRecentlyUsedTags_',
WORKSPACE_INVITE_MEMBERS_DRAFT: 'workspaceInviteMembersDraft_',
REPORT: 'report_',
// REPORT_METADATA is a perf optimization used to hold loading states (isLoadingReportActions, isLoadingMoreReportActions).
// REPORT_METADATA is a perf optimization used to hold loading states (isLoadingInitialReportActions, isLoadingOlderReportActions, isLoadingNewerReportActions).
// A lot of components are connected to the Report entity and do not care about the actions. Setting the loading state
// directly on the report caused a lot of unnecessary re-renders
REPORT_METADATA: 'reportMetadata_',
Expand Down
3 changes: 3 additions & 0 deletions src/components/InvertedFlatList/BaseInvertedFlatList.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,9 @@ function BaseInvertedFlatList(props) {
// Web requires that items be measured or else crazy things happen when scrolling.
getItemLayout={shouldMeasureItems ? getItemLayout : undefined}
windowSize={15}
maintainVisibleContentPosition={{
minIndexForVisible: 0,
}}
inverted
/>
);
Expand Down
17 changes: 10 additions & 7 deletions src/components/ReportActionsSkeletonView/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,26 @@ import CONST from '../../CONST';
const propTypes = {
/** Whether to animate the skeleton view */
shouldAnimate: PropTypes.bool,

/** Number of possible visible content items */
possibleVisibleContentItems: PropTypes.number,
};

const defaultProps = {
shouldAnimate: true,
possibleVisibleContentItems: 0,
perunt marked this conversation as resolved.
Show resolved Hide resolved
};

function ReportActionsSkeletonView(props) {
// Determines the number of content items based on container height
const possibleVisibleContentItems = Math.ceil(Dimensions.get('window').height / CONST.CHAT_SKELETON_VIEW.AVERAGE_ROW_HEIGHT);
function ReportActionsSkeletonView({shouldAnimate, possibleVisibleContentItems}) {
const contentItems = possibleVisibleContentItems || Math.ceil(Dimensions.get('window').height / CONST.CHAT_SKELETON_VIEW.AVERAGE_ROW_HEIGHT);
const skeletonViewLines = [];
for (let index = 0; index < possibleVisibleContentItems; index++) {
for (let index = 0; index < contentItems; index++) {
const iconIndex = (index + 1) % 4;
switch (iconIndex) {
case 2:
skeletonViewLines.push(
<SkeletonViewLines
shouldAnimate={props.shouldAnimate}
shouldAnimate={shouldAnimate}
numberOfRows={2}
key={`skeletonViewLines${index}`}
/>,
Expand All @@ -32,7 +35,7 @@ function ReportActionsSkeletonView(props) {
case 0:
skeletonViewLines.push(
<SkeletonViewLines
shouldAnimate={props.shouldAnimate}
shouldAnimate={shouldAnimate}
numberOfRows={3}
key={`skeletonViewLines${index}`}
/>,
Expand All @@ -41,7 +44,7 @@ function ReportActionsSkeletonView(props) {
default:
skeletonViewLines.push(
<SkeletonViewLines
shouldAnimate={props.shouldAnimate}
shouldAnimate={shouldAnimate}
numberOfRows={1}
key={`skeletonViewLines${index}`}
/>,
Expand Down
2 changes: 1 addition & 1 deletion src/libs/actions/Policy.js
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ function createPolicyExpenseChats(policyID, invitedEmailsToAccountIDs) {
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${optimisticReport.reportID}`,
value: {
isLoadingReportActions: false,
isLoadingInitialReportActions: false,
},
});
});
Expand Down
77 changes: 63 additions & 14 deletions src/libs/actions/Report.js
Original file line number Diff line number Diff line change
Expand Up @@ -477,8 +477,9 @@ function openReport(reportID, participantLoginList = [], newReportObject = {}, p
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`,
value: {
isLoadingReportActions: true,
isLoadingMoreReportActions: false,
isLoadingInitialReportActions: true,
isLoadingOlderReportActions: false,
isLoadingNewerReportActions: false,
perunt marked this conversation as resolved.
Show resolved Hide resolved
},
},
];
Expand All @@ -501,7 +502,7 @@ function openReport(reportID, participantLoginList = [], newReportObject = {}, p
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`,
value: {
isLoadingReportActions: false,
isLoadingInitialReportActions: false,
},
},
];
Expand All @@ -511,7 +512,7 @@ function openReport(reportID, participantLoginList = [], newReportObject = {}, p
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`,
value: {
isLoadingReportActions: false,
isLoadingInitialReportActions: false,
},
},
];
Expand Down Expand Up @@ -737,8 +738,9 @@ function reconnect(reportID) {
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`,
value: {
isLoadingReportActions: true,
isLoadingMoreReportActions: false,
isLoadingInitialReportActions: true,
isLoadingNewerReportActions: false,
isLoadingOlderReportActions: false,
},
},
],
Expand All @@ -747,7 +749,7 @@ function reconnect(reportID) {
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`,
value: {
isLoadingReportActions: false,
isLoadingInitialReportActions: false,
},
},
],
Expand All @@ -756,7 +758,7 @@ function reconnect(reportID) {
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`,
value: {
isLoadingReportActions: false,
isLoadingInitialReportActions: false,
},
},
],
Expand All @@ -771,9 +773,9 @@ function reconnect(reportID) {
* @param {String} reportID
* @param {String} reportActionID
*/
function readOldestAction(reportID, reportActionID) {
function getOlderActions(reportID, reportActionID) {
API.read(
'ReadOldestAction',
'GetOlderActions',
{
reportID,
reportActionID,
Expand All @@ -784,7 +786,7 @@ function readOldestAction(reportID, reportActionID) {
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`,
value: {
isLoadingMoreReportActions: true,
isLoadingOlderReportActions: true,
},
},
],
Expand All @@ -793,7 +795,7 @@ function readOldestAction(reportID, reportActionID) {
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`,
value: {
isLoadingMoreReportActions: false,
isLoadingOlderReportActions: false,
},
},
],
Expand All @@ -802,7 +804,53 @@ function readOldestAction(reportID, reportActionID) {
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`,
value: {
isLoadingMoreReportActions: false,
isLoadingOlderReportActions: false,
},
},
],
},
);
}

/**
* Gets the newer actions that have not been read yet.
* Normally happens when you are not located at the bottom of the list and scroll down on a chat.
*
* @param {String} reportID
* @param {String} reportActionID
*/
function getNewerActions(reportID, reportActionID) {
API.read(
'GetNewerActions',
{
reportID,
reportActionID,
},
{
optimisticData: [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`,
value: {
isLoadingNewerReportActions: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

This caused a regression when we try to load more while the device is offline it will cause a loop setting the value of isLoadingNewerReportActions creating a flicker #30503

},
},
],
successData: [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`,
value: {
isLoadingNewerReportActions: false,
},
},
perunt marked this conversation as resolved.
Show resolved Hide resolved
],
failureData: [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`,
value: {
isLoadingNewerReportActions: false,
},
},
],
Expand Down Expand Up @@ -2411,7 +2459,6 @@ export {
expandURLPreview,
markCommentAsUnread,
readNewestAction,
readOldestAction,
openReport,
openReportFromDeepLink,
navigateToAndOpenReport,
Expand All @@ -2436,6 +2483,8 @@ export {
getReportPrivateNote,
clearPrivateNotesError,
hasErrorInPrivateNotes,
getOlderActions,
getNewerActions,
openRoomMembersPage,
savePrivateNotesDraft,
getDraftPrivateNote,
Expand Down
26 changes: 18 additions & 8 deletions src/pages/home/ReportScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,9 @@ const defaultProps = {
hasOutstandingIOU: false,
},
reportMetadata: {
isLoadingReportActions: true,
isLoadingMoreReportActions: false,
isLoadingInitialReportActions: true,
isLoadingOlderReportActions: false,
isLoadingNewerReportActions: false,
},
isComposerFullSize: false,
betas: [],
Expand Down Expand Up @@ -165,7 +166,7 @@ function ReportScreen({
const screenWrapperStyle = [styles.appContent, styles.flex1, {marginTop: viewportOffsetTop}];

// There are no reportActions at all to display and we are still in the process of loading the next set of actions.
const isLoadingInitialReportActions = _.isEmpty(reportActions) && reportMetadata.isLoadingReportActions;
const isLoadingInitialReportActions = _.isEmpty(reportActions) && reportMetadata.isLoadingInitialReportActions;

const isOptimisticDelete = lodashGet(report, 'statusNum') === CONST.REPORT.STATUS.CLOSED;

Expand Down Expand Up @@ -260,6 +261,13 @@ function ReportScreen({
const onSubmitComment = useCallback(
(text) => {
Report.addComment(getReportID(route), text);

// We need to scroll to the bottom of the list after the comment is added
roryabraham marked this conversation as resolved.
Show resolved Hide resolved
const refID = setTimeout(() => {
flatListRef.current.scrollToOffset({animated: false, offset: 0});
}, 10);

return () => clearTimeout(refID);
},
[route],
);
Expand Down Expand Up @@ -372,7 +380,7 @@ function ReportScreen({

// eslint-disable-next-line rulesdir/no-negated-variables
const shouldShowNotFoundPage = useMemo(
() => (!firstRenderRef.current && !report.reportID && !isOptimisticDelete && !reportMetadata.isLoadingReportActions && !isLoading && !userLeavingStatus) || shouldHideReport,
() => (!firstRenderRef.current && !report.reportID && !isOptimisticDelete && !reportMetadata.isLoadingInitialReportActions && !isLoading && !userLeavingStatus) || shouldHideReport,
[report, reportMetadata, isLoading, shouldHideReport, isOptimisticDelete, userLeavingStatus],
);

Expand Down Expand Up @@ -428,8 +436,9 @@ function ReportScreen({
<ReportActionsView
reportActions={reportActions}
report={report}
isLoadingReportActions={reportMetadata.isLoadingReportActions}
isLoadingMoreReportActions={reportMetadata.isLoadingMoreReportActions}
isLoadingInitialReportActions={reportMetadata.isLoadingInitialReportActions}
isLoadingNewerReportActions={reportMetadata.isLoadingNewerReportActions}
isLoadingOlderReportActions={reportMetadata.isLoadingOlderReportActions}
isComposerFullSize={isComposerFullSize}
policy={policy}
/>
Expand Down Expand Up @@ -488,8 +497,9 @@ export default compose(
reportMetadata: {
key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_METADATA}${getReportID(route)}`,
initialValue: {
isLoadingReportActions: true,
isLoadingMoreReportActions: false,
isLoadingInitialReportActions: true,
isLoadingOlderReportActions: false,
isLoadingNewerReportActions: false,
},
},
isComposerFullSize: {
Expand Down
Loading
Loading