[$500] Report gets blank after resizing

[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: 1.4.0.3
Reproducible in staging?: y
Reproducible in production?: y
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: @shubham1206agra
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1700236291136659

Action Performed:

(watch the vid before you try to reproduce, it'll be more clear)

1. Open any report in full size
2. Go to small screen view
3. Press back header
4. Go to large view (>800) again

Expected Result:

All the messages should be loading

Actual Result:

Report becomes blank

Workaround:

unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Recording.243.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0195fdcb43d1c84e8f
• Upwork Job ID: 1725605698649042944
• Last Price Increase: 2023-11-24
• Automatic offers:
• alitoshmatov | Reviewer | 27909873
• tienifr | Contributor | 27909877

[melvin-bot]

Triggered auto assignment to @mallenexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

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

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

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

[mallenexpensify]

Was able to reproduce after watching vid, def something we want t ofix. I feel like a version of this bug has existed before, I couldn't find any GHs for it though.

[tienifr]

Proposal

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

Report becomes blank

What is the root cause of that problem?

The getRehydratedState here is run before the useWindowDimensions hook render again, leading to the isSmallScreenWidth is still true when getRehydratedState is run while it actually should be false. This causes the Central Pane not to be added when resizing the screen and the report is blank.

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

In here, calculate the isSmallScreenWidth by using the Dimensions.get('window').width inside getRehydratedState to get the latest value.

const windowWidth = Dimensions.get('window').width;
const isSmallScreenWidth = windowWidth <= variables.mobileResponsiveWidthBreakpoint;

The logic is the same as the existing isSmallScreenWidth calculation here, just that it uses current value of the window dimension instead of using a hook, and should be used in cases like this the isSmallScreenWidth needs to be retrieved immediately without delay of a hook render. The logic can be extracted to a getIsSmallScreenWidth method for reuse.

Then we can clean up the existing getIsSmallScreenWidth here in web since it's not used any more

What alternative solutions did you explore? (Optional)

NA

[barros001]

Proposal

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

When on a report, and switching screen to the small view (mobile), clicking back on the report (which takes you to LHN), and then switching back to desktop view, central panel will be empty. The same happens if you load the app on mobile view (which will land you on LHN) and expanding the window to desktop size, no central panel will be loaded.

What is the root cause of that problem?

The root problem is is the sequence in which things are rendered. The app is structured this way (very simplified)

<NavigatorRoot>
  <AuthScreens>
    <Navigator>...</Navigator>
  </AuthScreens>
</NavigatorRoot>

When you switch from mobile to desktop (and vice versa), these components would all re-render because they all relay on isSmallScreenWidth, which comes from useWindowDimensions hook. Normally things would happen in the correct sequence, but when switching views, things get out of order. Here's why:

App/src/libs/Navigation/NavigationRoot.js

Lines 74 to 80 in 3f7e66a

	useEffect(() => {
	if (!navigationRef.isReady() || !props.authenticated) {
	return;
	}
	// We need to force state rehydration so the CustomRouter can add the CentralPaneNavigator route if necessary.
	navigationRef.resetRoot(navigationRef.getRootState());
	}, [isSmallScreenWidth, props.authenticated]);

When isSmallScreenWidth, this hook gets executed which in turn will call navigationRef.resetRoot(navigationRef.getRootState());. That will trigger getRehydratedState which will look for isSmallScreenWidth. This all happens before <Navigation> gets a chance to re-render and update the value of isSmallScreenWidth. For that reason getRehydratedState will have a stale value and it will not add the central panel.

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

We need to delay calling navigationRef.resetRoot(navigationRef.getRootState()); until everything have been re-rendered. To accomplish that we have two options:

1. Move this useEffect into createCustomStackNavigator.js:

We can remove the use effect from NavigatorRoot and add to createCustomStackNavigator as follows:

    useEffect(() => {
        if (!navigationRef.isReady()) {
            return;
        }
        // We need to force state rehydration so the CustomRouter can add the CentralPaneNavigator route if necessary.
        navigationRef.resetRoot(navigationRef.getRootState());
    }, [isSmallScreenWidth]);

That will bring the logic closer to where it affects while making sure it only calls resetRoot when we have the correct value for isSmallScreenWidth.

2. Delay calling resetRoot:

    useEffect(() => {
        if (!navigationRef.isReady() || !props.authenticated) {
            return;
        }
        // We need to force state rehydration so the CustomRouter can add the CentralPaneNavigator route if necessary.
        setTimeout(() => {
            navigationRef.resetRoot(navigationRef.getRootState());
        }, 0);
    }, [isSmallScreenWidth, props.authenticated]);

Wrapping the call to resetRoot in a setTimeout with a 0 delay will just let javascript process everything in the queue first, which means it will let react re-render everything before going back to it. This solves the problem and the central panel is added back.

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[derekops333]

Proposal

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

Report becomes blank when switching from desktop to mobile view, pressing back button and then going back to the desktop view

What is the root cause of that problem?

getRehydratedState function inside the CustomRouter is not called immediately after we switch from mobile to desktop (and other way around)

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

In the ResponsiveStackNavigator, we should add simple useEffect that is triggered when isSmallScreenWidth is changed and call navigation.reset(state); This will manually call getRehydratedState every time we have a transition desktop-mobile or mobile-desktop transition.

[tsa321]

@mallenexpensify I think similar to this bug: #28354

[ayazalavi]

Proposal

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

Upon resize windows from large -> small -> large size in width, central pane navigator doesnt gets loaded.

What is the root cause of that problem?

Root cause of the problem is that CustomRouter below is not getting rehydrated based on screen size changes

App/src/libs/Navigation/AppNavigator/createCustomStackNavigator/CustomRouter.js

Line 65 in 4df083c

function CustomRouter(options) {

the variable getting used to monitor screen size change is react state variable

App/src/libs/Navigation/AppNavigator/createCustomStackNavigator/index.js

Line 50 in 4df083c

const {isSmallScreenWidth} = useWindowDimensions();

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

Instead of entirely depending upon getRehydratedState for adding central pane we can use getStateForAction which can be triggered based on certain condition e.g. screen size changed

So basically as soon as screen size changes we can call following code in

App/src/libs/Navigation/AppNavigator/createCustomStackNavigator/index.js

Line 49 in 4df083c

function ResponsiveStackNavigator(props) {

navigation.dispatch({
            type: 'SCREEN_CHANGED',
            // Additional payload or parameters
          });

and in CustomRouter add something like this besides getRehydratedState

App/src/libs/Navigation/AppNavigator/createCustomStackNavigator/CustomRouter.js

Lines 65 to 70 in 3f7e66a

	function CustomRouter(options) {
	const stackRouter = StackRouter(options);
	return {
	...stackRouter,
	getRehydratedState(partialState, {routeNames, routeParamList}) {

 getStateForAction(state, action, options) {
            // eslint-disable-next-line default-case
            switch (action.type) {
                case "SCREEN_CHANGED": {
                    if (!isAtLeastOneCentralPaneNavigatorInState(state)) {
                        // If we added a route we need to make sure that the state.stale is true to generate new key for this route
                        // eslint-disable-next-line no-param-reassign
                        Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(getTopMostReportIDFromRHP(state)));
                    }
                    return state                    
                }
            }
            return stackRouter.getStateForAction(state, action, options);
          },

As per my testing this fixes the issue.

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[alitoshmatov]

It is the same as #28354. @tienifr, since you worked on previous issue and have proposal here, can explain why this issue resurfaced.

[mallenexpensify]

Thanks @tsa321 for the link.
Also tagging @narefyev91 and @arosiclair who worked on

• [HOLD for payment 2023-10-16] [$500] Report screen becomes empty when resizing to big size from LHN #28354

[ayazalavi]

Hey @alitoshmatov, about the issue you brought up: first, we tried sorting it out with useWindowDimensions, but it still caused some hiccups. The trouble really roots in our reliance on getRehydratedState, which is an internal callback in React Navigation. This callback kicks off the addition of a central pane, but we can't directly call it ourselves. So, for any tweaks we need after navigation loads, we've gotta find a different way.

I suggested using another approach to reach that central pane when state changes. I'd love to hear your take on this. @narefyev91 and @arosiclair, your thoughts matter too. What do you all think?

[derekops333]

Hey @alitoshmatov, about the issue you brought up: first, we tried sorting it out with useWindowDimensions, but it still caused some hiccups. The trouble really roots in our reliance on getRehydratedState, which is an internal callback in React Navigation. This callback kicks off the addition of a central pane, but we can't directly call it ourselves. So, for any tweaks we need after navigation loads, we've gotta find a different way.

I suggested using another approach to reach that central pane when state changes. I'd love to hear your take on this. @narefyev91 and @arosiclair, your thoughts matter too. What do you all think?

My solution does manually call getRehydratedState on screen size change (mobile-desktop-mobile). It resets the state (but actually keeping the same state parameter) which basically just re-runs getRehydratedState

[tienifr]

It is the same as #28354. @tienifr, since you worked on previous issue and have proposal here, can explain why this issue resurfaced.

My fixed PR is created so long ago, I also tried to figure out why this issue resurfaced but there was no outcome. I see all proposals point out the same RCA: The getRehydratedState is run before the useWindowDimensions hook render again, that why I strongly believe it's the correct RCA.

What do you think? @alitoshmatov

[derekops333]

@m-natarajan are there any updates related to this issue?

[alitoshmatov]

What do you think? @alitoshmatov

Okay, I also couldn't find why this issue occurred again. The RCA you provided is correct.

Reviewing proposals.

[melvin-bot]

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

[shubham1206agra]

Proposal

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

Report gets blank after resizing

What is the root cause of that problem?

The getRehydratedState here is run before the useWindowDimensions hook render again, leading to the isSmallScreenWidth is still true when getRehydratedState is run while it actually should be false. This causes the Central Pane not to be added when resizing the screen and the report is blank.

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

In here, calculate the isSmallScreenWidth by using the Dimensions.get('window').width inside getRehydratedState to get the latest value.

const windowWidth = Dimensions.get('window').width;
const isSmallScreenWidth = windowWidth <= variables.mobileResponsiveWidthBreakpoint;

The above code is for the web platforms. For native devices, the isSmallScreenWidth is always true from #27004 to fix the iPad layout for the time being as we don't officially fully support iPad / tablet devices.

The logic is the same as the existing isSmallScreenWidth calculation here, just that it uses current value of the window dimension instead of using a hook, and should be used in cases like this the isSmallScreenWidth needs to be retrieved immediately without delay of a hook render. The logic can be extracted to a getIsSmallScreenWidth method for reuse.

Then we can clean up the existing getIsSmallScreenWidth here in web since it's not used any more

What alternative solutions did you explore? (Optional)

NA

[shubham1206agra]

@shubham1206agra You mentioned race condition, can you elaborate on that if you think that was the root cause of iPad issue

@alitoshmatov
Nope, this is just iPad behavior got out of sync with useWindowDimensions.

I can raise the new PR with original solution with minor fixes for iPad. (This is mostly 2-3 lines addition).

[mountiny]

@alitoshmatov I also believe you might have to try on larger screen ipad

[melvin-bot]

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

• fix: 31503 Report gets blank after resizing #32422

If no regressions arise, payment will be issued on 2024-01-24. 🎊

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

• @alitoshmatov requires payment automatic offer (Reviewer)
• @shubham1206agra requires payment (Needs manual offer from BZ)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@alitoshmatov] The PR that introduced the bug has been identified. Link to the PR:
• [@alitoshmatov] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@alitoshmatov] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@alitoshmatov] Determine if we should create a regression test for this bug.
• [@alitoshmatov] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@mallenexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[mallenexpensify]

This can be closed... right?
Above I see payment has already been made and, if there was a regression (which it looks like there was) it was for an unsupported device, iPad, so I don't think we'd reduce pay for that

[shubham1206agra]

@mallenexpensify We reverted the change so its still broken. @alitoshmatov needs to review the proposal here to fix this again. So can you reopen this again?

[dangrous]

Yep, you're right, reopening as we reverted the PR that fixed it.

We should determine whether @tienifr and @alitoshmatov should work to fix (again) since they were the original assignees, or whether we want to open it up again.

Thanks!

[alitoshmatov]

I am a little bit confused. @shubham1206agra your proposal is exactly the same as @tienifr 's, Are you suggesting that reimplementing this solution is correct approach. If yes, I think I and @tienifr can reimplement this easily since it is the same old solution. I am happy to review it without any payment.

[shubham1206agra]

Reimplementing with little changes. But let's wait for Ideal Nav to stabilize to check the repro of the issue.

Edit - Looks like the issue will be fixed. Lets wait for the Ideal Nav to hit production.

[alitoshmatov]

@shubham1206agra That looks like a plan, I do agree with holding this issue since this issue is an edge case

[mallenexpensify]

Posted in the Wave 8 room. to check

[mallenexpensify]

I'm going to close this. Coming from the internal slack discussion

We don’t officially support re-sizing to my knowledge. We had a bunch of issues open in the repo recently and closed them. I don’t think this is a priority right now.

If/when we want to address resizing we'll be able to easily find this issue cuz resizing is in the title.