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

Fix chat doesn't scroll to bottom #43021

Merged
merged 11 commits into from
Jun 10, 2024
Merged

Fix chat doesn't scroll to bottom #43021

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
0c7c71d
fix chat doesn't scroll to bottom
nkdengineer Jun 3, 2024
276b5fd
Merge branch 'main' into fix/40724
nkdengineer Jun 4, 2024
d4e7d5d
fix lint
nkdengineer Jun 4, 2024
5975d53
Merge branch 'main' into fix/40724
nkdengineer Jun 5, 2024
9a3491b
move update to buildOnyxDataForMoneyRequest
nkdengineer Jun 5, 2024
7fcaeca
merge main
nkdengineer Jun 5, 2024
9e034c7
Merge branch 'main' into fix/40724
nkdengineer Jun 6, 2024
5a8f69a
fix lastVisibleActionCreated of task and split case
nkdengineer Jun 6, 2024
2abf605
Merge branch 'main' into fix/40724
nkdengineer Jun 7, 2024
9a8a96b
revert unrelated change
nkdengineer Jun 7, 2024
9f7ea10
Merge branch 'main' into fix/40724
nkdengineer Jun 10, 2024
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
3 changes: 2 additions & 1 deletion src/libs/DateUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,8 @@ function getDBTime(timestamp: string | number = ''): string {
*/
function getDBTimeWithSkew(timestamp: string | number = ''): string {
if (networkTimeSkew > 0) {
return getDBTime(new Date(timestamp).valueOf() + networkTimeSkew);
const datetime = timestamp ? new Date(timestamp) : new Date();
return getDBTime(datetime.valueOf() + networkTimeSkew);
}
return getDBTime(timestamp);
}
Expand Down
5 changes: 4 additions & 1 deletion src/libs/actions/IOU.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,7 @@ function buildOnyxDataForMoneyRequest(
...iouReport,
lastMessageText: iouAction.message?.[0]?.text,
lastMessageHtml: iouAction.message?.[0]?.html,
lastVisibleActionCreated: iouAction.created,
pendingFields: {
...(shouldCreateNewMoneyRequestReport ? {createChat: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD} : {preview: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE}),
},
Expand Down Expand Up @@ -1983,6 +1984,7 @@ function getMoneyRequestInformation(
reportPreviewAction = ReportUtils.updateReportPreview(iouReport, reportPreviewAction as ReportPreviewAction, false, comment, optimisticTransaction);
} else {
reportPreviewAction = ReportUtils.buildOptimisticReportPreview(chatReport, iouReport, comment, optimisticTransaction);
chatReport.lastVisibleActionCreated = reportPreviewAction.created;

// Generated ReportPreview action is a parent report action of the iou report.
// We are setting the iou report's parentReportActionID to display subtitle correctly in IOU page when offline.
Expand Down Expand Up @@ -3835,6 +3837,7 @@ function createSplitsAndOnyxData(
splitChatReport.lastMessageText = splitIOUReportAction.message?.[0]?.text;
splitChatReport.lastMessageHtml = splitIOUReportAction.message?.[0]?.html;
splitChatReport.lastActorAccountID = currentUserAccountID;
splitChatReport.lastVisibleActionCreated = splitIOUReportAction.created;
Copy link
Contributor

Choose a reason for hiding this comment

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

Mutating the object early here may have caused a regression: #54528
For more details, check: #54528 (comment)


let splitChatReportNotificationPreference = splitChatReport.notificationPreference;
if (splitChatReportNotificationPreference === CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN) {
Expand Down Expand Up @@ -4639,7 +4642,7 @@ function startSplitBill({
API.write(WRITE_COMMANDS.START_SPLIT_BILL, parameters, {optimisticData, successData, failureData});

Navigation.dismissModalWithReport(splitChatReport);
Report.notifyNewAction(splitChatReport.chatReportID ?? '', currentUserAccountID);
Report.notifyNewAction(splitChatReport.reportID ?? '', currentUserAccountID);
}

/** Used for editing a split expense while it's still scanning or when SmartScan fails, it completes a split expense started by startSplitBill above.
Expand Down
2 changes: 1 addition & 1 deletion src/libs/actions/Report.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,7 @@ function addActions(reportID: string, text = '', file?: FileObject) {
const lastCommentText = ReportUtils.formatReportLastMessageText(lastComment?.text ?? '');

const optimisticReport: Partial<Report> = {
lastVisibleActionCreated: currentTime,
lastVisibleActionCreated: lastAction?.created,
lastMessageTranslationKey: lastComment?.translationKey ?? '',
lastMessageText: lastCommentText,
lastMessageHtml: lastCommentText,
Expand Down
4 changes: 2 additions & 2 deletions src/libs/actions/Task.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,10 @@ function createTaskAndNavigate(
const optimisticAddCommentReport = ReportUtils.buildOptimisticTaskCommentReportAction(taskReportID, title, assigneeAccountID, `task for ${title}`, parentReportID);
optimisticTaskReport.parentReportActionID = optimisticAddCommentReport.reportAction.reportActionID;

const currentTime = DateUtils.getDBTime();
const currentTime = DateUtils.getDBTimeWithSkew();
const lastCommentText = ReportUtils.formatReportLastMessageText(optimisticAddCommentReport?.reportAction?.message?.[0]?.text ?? '');
const optimisticParentReport = {
lastVisibleActionCreated: currentTime,
lastVisibleActionCreated: optimisticAddCommentReport.reportAction.created,
lastMessageText: lastCommentText,
lastActorAccountID: currentUserAccountID,
lastReadTime: currentTime,
Expand Down
5 changes: 1 addition & 4 deletions src/pages/home/report/ReportActionsList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,12 +210,9 @@ function ReportActionsList({
[sortedReportActions, isOffline],
);

// whisper action doesn't affect lastVisibleActionCreated, so we should not take it into account while checking if there is the newest report action
const newestVisibleReportAction = useMemo(() => sortedVisibleReportActions.find((item) => !ReportActionsUtils.isWhisperAction(item)) ?? null, [sortedVisibleReportActions]);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like we will reintroduce #41188

@parasharrajat

I believe it's a back-end bug. After we start s split bill, BE doesn't return the updated lastVisibleActionCreated of the report but when OpenReport API is called, we can see the lastVisibleActionCreated is returned as the created of split whisper action above.

Screenshot 2024-06-05 at 15 03 54

So I removed this change from this PR. If not when we reload this report and send any action like message, expense, ... the chat will not scroll to the bottom

Copy link
Member

Choose a reason for hiding this comment

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

@mountiny Do you know why it is different on backend? The comment says that whisper does not affect the lastVisibleActionCreated time.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this for the same account that created the split bill request? If so, we should use the whisper created, no?

Copy link
Contributor

Choose a reason for hiding this comment

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

Commented in Slack

Copy link
Contributor

Choose a reason for hiding this comment

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

Following along because I'm the BZ team member on #41188. Do you all feel that a regression was introduced in that issue? It's due to be paid out, but I will hold on that if there's a regression.

Copy link
Member

Choose a reason for hiding this comment

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

There was no regression as such.

Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome. Thanks, @parasharrajat!


const lastActionIndex = sortedVisibleReportActions[0]?.reportActionID;
const reportActionSize = useRef(sortedVisibleReportActions.length);
const hasNewestReportAction = newestVisibleReportAction?.created === report.lastVisibleActionCreated;
const hasNewestReportAction = sortedVisibleReportActions[0]?.created === report.lastVisibleActionCreated;
const hasNewestReportActionRef = useRef(hasNewestReportAction);
hasNewestReportActionRef.current = hasNewestReportAction;
const previousLastIndex = useRef(lastActionIndex);
Expand Down
Loading