[HOLD for payment 2024-11-22] [$250] Pressing up arrow to edit the previous message, there is a bad overlap with the recipients local time.

[m-natarajan]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 9.0.53-1
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?:
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @tgolen
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1729781934730029

Action Performed:

1. Go to staging.new.expensify.com
2. DM any user and send multiple messages
3. Click the up arrow on keyboard to edit the last message sent

Expected Result:

User able to edit last message sent and no visual issues

Actual Result:

Selected message box overlaps with the recipients local time

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021851399834739628625
• Upwork Job ID: 1851399834739628625
• Last Price Increase: 2024-11-05

Issue Owner

Current Issue Owner: @strepanier03

[melvin-bot]

Triggered auto assignment to @strepanier03 (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[shahinyan11]

Edited by proposal-police: This proposal was edited at 2024-10-24 19:35:48 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

Pressing up arrow to edit the previous message, there is a bad overlap with the recipients local time.

What is the root cause of that problem?

The message edit mode container is large and the scroll position remains the same after expanding the content.

What changes do you think we should make in order to solve the problem?

Add following line const reportScrollManager = useReportScrollManager() in ComposerWithSuggestions component and add bellow code after this line

InteractionManager.runAfterInteractions(()=>{
    reportScrollManager.scrollToBottom()
})

What alternative solutions did you explore? (Optional)

Add bellow useEffect in ReportActionItem component

useEffect(() => {
    if(!draftMessage) return
    
    InteractionManager.runAfterInteractions(()=>{
        reportScrollManager.scrollToIndex(index)
    })
}, [draftMessage, index]);

What alternative solutions did you explore? (Optional)

Add bellow useEffect in ReportActionItemMessageEdit component

useEffect(() => {    
    InteractionManager.runAfterInteractions(()=>{
        reportScrollManager.scrollToIndex(index)
    })
}, [index]);

We can also use setTimeout(()=>{}, 0) instead of InteractionManager.runAfterInteractions everywhere

[bernhardoj]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Pressing the up arrow to edit the last message doesn't show it fully.

What is the root cause of that problem?

In our custom MVCPFlatList, we enable the maintainVisibleContentPosition to maintain the position of the message when the content size changes by scrolling it by the offset of the size differences. But, we also have the mvcpAutoscrollToTopThresholdRef which should auto-scroll to the bottom if it's within the threshold (250).

App/src/components/FlatList/index.tsx

Lines 124 to 131 in 66112d1

	if (Math.abs(delta) > (IS_MOBILE_SAFARI ? 100 : 0.5)) {
	const scrollOffset = lastScrollOffsetRef.current;
	prevFirstVisibleOffsetRef.current = firstVisibleViewOffset;
	scrollToOffset(scrollOffset + delta, false, true);
	if (mvcpAutoscrollToTopThresholdRef.current != null && scrollOffset <= mvcpAutoscrollToTopThresholdRef.current) {
	scrollToOffset(0, true, false);
	}
	}

The issue in this case is, the scroll to the bottom (0) doesn't work and looks like it's conflicting with the edit composer focus. For the scroll to bottom, we set the scroll animated to true (2nd param), which will set the behavior to smooth scrolling. So, the edit composer focus interrupt the scrolling.

App/src/components/FlatList/index.tsx

Lines 127 to 129 in 66112d1

	scrollToOffset(scrollOffset + delta, false, true);
	if (mvcpAutoscrollToTopThresholdRef.current != null && scrollOffset <= mvcpAutoscrollToTopThresholdRef.current) {
	scrollToOffset(0, true, false);

If we remove auto-focus from the edit composer, then it works fine, but we can't do that.

What changes do you think we should make in order to solve the problem?

If the mutation is an addition of an edit composer, then don't scroll with animation.

First, adjustForMaintainVisibleContentPosition will accept a new props to decide whether to animate the scroll or not and pass it to the scroll function.

App/src/components/FlatList/index.tsx

Line 111 in 66112d1

const adjustForMaintainVisibleContentPosition = useCallback(() => {

App/src/components/FlatList/index.tsx

Line 129 in 66112d1

scrollToOffset(0, true, false);

const adjustForMaintainVisibleContentPosition = useCallback((animated = true) => {
    ...
    scrollToOffset(0, animated, false);

Then, we check if the mutation list contains an added nodes of the edit composer.

App/src/components/FlatList/index.tsx

Lines 142 to 166 in 66112d1

	const mutationObserver = new MutationObserver((mutations) => {
	// Check if the first visible view is removed and re-calculate it
	// if needed.
	mutations.forEach((mutation) => {
	mutation.removedNodes.forEach((node) => {
	if (node !== firstVisibleViewRef.current) {
	return;
	}
	firstVisibleViewRef.current = null;
	});
	});
	if (firstVisibleViewRef.current == null) {
	prepareForMaintainVisibleContentPosition();
	}
	// When the list is hidden, the size will be 0.
	// Ignore the callback if the list is hidden because scrollOffset will always be 0.
	if (!getScrollableNode(scrollRef.current)?.clientHeight) {
	return;
	}
	adjustForMaintainVisibleContentPosition();
	prepareForMaintainVisibleContentPosition();
	});

let isEditComposerAdded = false;
mutations.forEach((mutation) => {
    ...

    mutation.addedNodes.forEach((node) => {
        if (node.nodeType === Node.ELEMENT_NODE && node.dataset.isEditing === "true") {
            isEditComposerAdded = true;
            return;
        }
    });
});

...

adjustForMaintainVisibleContentPosition(!isEditComposerAdded);
prepareForMaintainVisibleContentPosition();

If an edit composer action is added, then we pass the animated as false. We check whether it's an edit composer action by checking the node.dataset.isEditing === "true" instead of recursively checking the children to make it simpler (let me know if we want to recursively checking the children).

Last, we need to add the isEditing dataset to the element here.

App/src/pages/home/report/ReportActionItem.tsx

Line 966 in 66112d1

<View style={highlightedBackgroundColorIfNeeded}>

<View style={highlightedBackgroundColorIfNeeded} dataSet={{isEditing: !!draftMessage}}>

What alternative solutions did you explore? (Optional)

We have a few alternatives here.

Delay the auto scroll to bottom here using InteractionManager, let the focus happens first.

App/src/components/FlatList/index.tsx

Lines 128 to 130 in 66112d1

	if (mvcpAutoscrollToTopThresholdRef.current != null && scrollOffset <= mvcpAutoscrollToTopThresholdRef.current) {
	scrollToOffset(0, true, false);
	}

Delay the focus using InteractionManager,

App/src/libs/focusComposerWithDelay/index.ts

Lines 30 to 38 in 66112d1

	Promise.all([ComposerFocusManager.isReadyToFocus(), isWindowReadyToFocus()]).then(() => {
	if (!textInput) {
	return;
	}
	textInput.focus();
	if (forcedSelectionRange) {
	setTextInputSelection(textInput, forcedSelectionRange);
	}
	});

OR we can wait for the scroll event to ends, but the scroll end event actually just use setTimeout 250ms after there is no onScroll event received anymore.

App/src/components/InvertedFlatList/index.tsx

Lines 75 to 83 in 66112d1

	if (lastScrollEvent.current) {
	scrollEndTimeout.current = setTimeout(() => {
	if (lastScrollEvent.current !== timestamp) {
	return;
	}
	// Scroll has ended
	lastScrollEvent.current = null;
	onScrollEnd();
	}, 250);

We can manually scroll when the edit composer is focused, but reportScrollManager.scrollToIndex is currently only made for native because in web, we don't want to scroll the edit composer to bottom when editing it.

App/src/pages/home/report/ReportActionItemMessageEdit.tsx

Lines 521 to 530 in 66112d1

	onFocus={() => {
	setIsFocused(true);
	if (textInputRef.current) {
	ReportActionComposeFocusManager.editComposerRef.current = textInputRef.current;
	}
	InteractionManager.runAfterInteractions(() => {
	requestAnimationFrame(() => {
	reportScrollManager.scrollToIndex(index, true);
	});
	});

App/src/hooks/useReportScrollManager/index.ts

Lines 11 to 18 in 66112d1

	const scrollToIndex = useCallback(
	(index: number, isEditing?: boolean) => {
	if (!flatListRef?.current || isEditing) {
	return;
	}
	flatListRef.current.scrollToIndex({index, animated: true});
	},

So, what we can do is to only scroll if it's the last action.

reportScrollManager.scrollToIndex(index, isLastVisibleReportAction);

We can get the last visible report action inside getFirstVisibleReportActionID. We will rename it to getFirstAndLastVisibleReportActionID

App/src/pages/home/report/ReportActionsList.tsx

Line 452 in 66112d1

const firstVisibleReportActionID = useMemo(() => ReportActionsUtils.getFirstVisibleReportActionID(sortedReportActions, isOffline), [sortedReportActions, isOffline]);

function getFirstAndLastVisibleReportActionID(sortedReportActions: ReportAction[] = [], isOffline = false): [string, string] {
    if (!Array.isArray(sortedReportActions)) {
        return ['', ''];
    }
    const sortedFilterReportActions = sortedReportActions.filter((action) => !isDeletedAction(action) || (action?.childVisibleActionCount ?? 0) > 0 || isOffline);
    return sortedFilterReportActions.length > 1 ? [
        sortedFilterReportActions.at(sortedFilterReportActions.length - 2)?.reportActionID ?? '-1',
        sortedFilterReportActions.at(0)?.reportActionID ?? '-1',
    ] : ['', ''];
}

Then, pass isLastVisibleReportAction just like isFirstVisibleReportAction

App/src/pages/home/report/ReportActionsList.tsx

Lines 531 to 532 in 66112d1

	isFirstVisibleReportAction={firstVisibleReportActionID === reportAction.reportActionID}
	shouldUseThreadDividerLine={shouldUseThreadDividerLine}

If we want to always scroll the edit composer to bottom (specifically to put it above the main composer), then we can remove the isEditing here.

App/src/hooks/useReportScrollManager/index.ts

Lines 11 to 17 in 66112d1

	const scrollToIndex = useCallback(
	(index: number, isEditing?: boolean) => {
	if (!flatListRef?.current || isEditing) {
	return;
	}
	flatListRef.current.scrollToIndex({index, animated: true});

[adelekennedy]

I think this issue may be a dupe (thank you @shahinyan11!) closing in favor if this one as it already has a proposal

[raza-ak]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Pressing up arrow to edit the previous message, there is a bad overlap with the recipients local time.

What is the root cause of that problem?

Enabling edit mode triggers a slight upward movement of the scrollbar, which shifts the layout and results in an overlap between the message editing area and the recipient's local time display.

What changes do you think we should make in order to solve the problem?

A useEffect hook was implemented to automatically scroll to the bottom after the component finishes rendering in edit mode. This ensures that the edit section is fully visible and prevents any overlap with the recipient’s local time.
Following changes will fix the issue:

Video:

380152981-e3cb759d-fbbe-4ec1-862f-39a7a989edbb.mp4

[melvin-bot]

@strepanier03 Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~021851399834739628625

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @Ollyws (External)

[strepanier03]

Repro'd pretty easily. This is mostly just a polish project as it doesn't impact the customer's ability to use the product nor drive forward other projects.

[melvin-bot]

@strepanier03, @Ollyws Huh... This is 4 days overdue. Who can take care of this?

[Ollyws]

Will get to this one tomorrow morning.

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[Ollyws]

@bernhardoj's proposal LGTM.
🎀👀🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @rlinoz, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[bernhardoj]

PR is ready

cc: @Ollyws

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.62-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix chat list isn't scrolled to bottom when edit message using arrow up key #52090

If no regressions arise, payment will be issued on 2024-11-22. 🎊

For reference, here are some details about the assignees on this issue:

• @Ollyws requires payment through NewDot Manual Requests
• @bernhardoj requires payment through NewDot Manual Requests

[melvin-bot]

@Ollyws @strepanier03 @Ollyws The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]