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

[HOLD for payment 2024-12-20] [$250] Web - Workspace - Central pane gets blank after resizing and clicking back icon #53395

Closed
1 of 8 tasks
vincdargento opened this issue Dec 2, 2024 · 22 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@vincdargento
Copy link

vincdargento commented Dec 2, 2024

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.69-4
Reproducible in staging?: Yes
Reproducible in production?: No
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): N/A
Issue reported by: Applause Internal Team

Action Performed:

  1. Go to https://staging.new.expensify.com/
    and log in
  2. Create a workspace if there is no any
  3. Navigate to the account settings and select the workspace
  4. Resize the window so the LHN of workspace is no longer visible
  5. Click on the back icon
  6. Return to full screen

Expected Result:

Workspace profile is displayed in the central pane

Actual Result:

The central pane is blank. The same happens with other workspace options e.g. Members, Categories

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

bug.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021863651912898483255
  • Upwork Job ID: 1863651912898483255
  • Last Price Increase: 2024-12-02
Issue OwnerCurrent Issue Owner: @mallenexpensify
@vincdargento vincdargento added DeployBlockerCash This issue or pull request should block deployment Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 2, 2024
Copy link

melvin-bot bot commented Dec 2, 2024

Triggered auto assignment to @mallenexpensify (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.

Copy link

melvin-bot bot commented Dec 2, 2024

Triggered auto assignment to @puneetlath (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

Copy link

melvin-bot bot commented Dec 2, 2024

💬 A slack conversation has been started in #expensify-open-source

@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Dec 2, 2024
Copy link
Contributor

github-actions bot commented Dec 2, 2024

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@vincdargento
Copy link
Author

vincdargento commented Dec 2, 2024

@mallenexpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors.

@puneetlath
Copy link
Contributor

Re-sizing doesn't feel like a primary flow that is blocker-worthy to me. Going to demote.

@puneetlath puneetlath added External Added to denote the issue can be worked on by a contributor and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels Dec 2, 2024
@melvin-bot melvin-bot bot changed the title Web - Workspace - Central pane gets blank after resizing and clicking back icon [$250] Web - Workspace - Central pane gets blank after resizing and clicking back icon Dec 2, 2024
@puneetlath puneetlath added the Daily KSv2 label Dec 2, 2024
Copy link

melvin-bot bot commented Dec 2, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 2, 2024
Copy link

melvin-bot bot commented Dec 2, 2024

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

@nkdengineer
Copy link
Contributor

nkdengineer commented Dec 2, 2024

Edited by proposal-police: This proposal was edited at 2024-12-02 18:44:37 UTC.

Proposal

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

The central pane is blank. The same happens with other workspace options e.g. Members, Categories

What is the root cause of that problem?

when we resize on a screen that have a small screen width, when going from the list workspace to the detail workspace, the route will be WORKSPACE_INITIAL
and when resizing again, the route is still WORKSPACE_INITIAL so the central pane side will not be displayed

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

We can add a useEffect in WorkspaceInitialPage and add dependency shouldUseNarrowLayout

In case shouldUseNarrowLayout = false but route is still WORKSPACE_INITIAL then we will automatically navigate to WORKSPACE_PROFILE

What alternative solutions did you explore? (Optional)

@mallenexpensify
Copy link
Contributor

@ntdiary , 👀 on @nkdengineer 's proposal above plz. I was able to reproduce, will add to #quality as low.

@huult
Copy link
Contributor

huult commented Dec 3, 2024

Proposal

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

Central pane gets blank after resizing and clicking back icon

What is the root cause of that problem?

When we resize to a mobile screen and click the back arrow button, it navigates from Workspace_Profile to Workspace_Initial and removes /profile from the URL.

Screen.Recording.2024-12-03.at.12.15.48.mov

When we then resize to a desktop screen, we remain on Workspace_Initial, which causes this issue to occur.

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

To resolve this issue, when we resize to a desktop screen, we must update the screen from Workspace_Initial to Workspace_Profile.

We able use that logic to apply for this solution some thing like that:

if (!isNarrowLayout) {
if (state.routes.length === 1 && state.routes.at(0)?.name === SCREENS.WORKSPACE.INITIAL) {

//src/pages/workspace/WorkspaceInitialPage.tsx#L159
    const isNarrowLayout = getIsNarrowLayout();
    useEffect(() => {
        if (isNarrowLayout || activeRoute !== SCREENS.WORKSPACE.INITIAL) {
            return;
        }
        Navigation.navigate(ROUTES.WORKSPACE_PROFILE.getRoute(policyID));
         // or
        // Navigation.navigate(ROUTES.WORKSPACE_INITIAL.getRoute(policyID));
    }, [activeRoute, isNarrowLayout, policyID]);

Note: isNarrowLayout can be used for something like this

We must check that activeRoute !== SCREENS.WORKSPACE.INITIAL to avoid always redirecting to Workspace_Profile when resizing.

POC
Screen.Recording.2024-12-03.at.12.26.01.mov

What alternative solutions did you explore? (Optional)

@ntdiary
Copy link
Contributor

ntdiary commented Dec 3, 2024

under review. :)

@CyberAndrii
Copy link
Contributor

For some reason after #49937 this function is no longer getting called on resize

getRehydratedState(partialState: StackState, {routeNames, routeParamList, routeGetIdList}: RouterConfigOptions): PlatformStackNavigationState<ParamListBase> {
adaptStateIfNecessary(partialState);
const state = stackRouter.getRehydratedState(partialState, {routeNames, routeParamList, routeGetIdList});
return state;
},

It should be triggered by this line

navigationRef.resetRoot(navigationRef.getRootState());

@FitseTLT
Copy link
Contributor

FitseTLT commented Dec 3, 2024

Can be related to #53445

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Dec 3, 2024
@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Dec 13, 2024
@melvin-bot melvin-bot bot changed the title [$250] Web - Workspace - Central pane gets blank after resizing and clicking back icon [HOLD for payment 2024-12-20] [$250] Web - Workspace - Central pane gets blank after resizing and clicking back icon Dec 13, 2024
Copy link

melvin-bot bot commented Dec 13, 2024

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

@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Dec 13, 2024
Copy link

melvin-bot bot commented Dec 13, 2024

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

If no regressions arise, payment will be issued on 2024-12-20. 🎊

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

  • @ntdiary requires payment through NewDot Manual Requests

Copy link

melvin-bot bot commented Dec 13, 2024

@ntdiary @mallenexpensify @ntdiary 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]

Copy link

melvin-bot bot commented Dec 18, 2024

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@ntdiary
Copy link
Contributor

ntdiary commented Dec 19, 2024

BugZero Checklist:

  • [Contributor] Classify the bug:
Bug classification

Source of bug:

  • 1a. Result of the original design (eg. a case wasn't considered)
  • 1b. Mistake during implementation
  • 1c. Backend bug
  • 1z. Other:

Where bug was reported:

  • 2a. Reported on production (eg. bug slipped through the normal regression and PR testing process on staging)
  • 2b. Reported on staging (eg. found during regression or PR testing)
  • 2d. Reported on a PR
  • 2z. Other:

Who reported the bug:

  • 3a. Expensify user
  • 3b. Expensify employee
  • 3c. Contributor
  • 3d. QA
  • 3z. Other:
  • [Contributor] 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: https://github.com/Expensify/App/pull/49937/files#r1892224061

  • [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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: N/A

  • [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

    No need, as we have added one automated navigation test.

Regression Test Proposal Template
  • [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

    Link to issue:

Regression Test Proposal

Precondition:

Test:

Do we agree 👍 or 👎

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Dec 20, 2024
Copy link

melvin-bot bot commented Dec 20, 2024

Payment Summary

Upwork Job

  • Reviewer: @ntdiary owed $250 via NewDot

BugZero Checklist (@mallenexpensify)

  • I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
  • I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1863651912898483255/hired)
  • I have paid out the Upwork contracts or cancelled the ones that are incorrect
  • I have verified the payment summary above is correct

@mallenexpensify
Copy link
Contributor

Contributor+: @ntdiary owed $250 via NewDot

No need, as we have added one automated navigation test.

Not creating a test case.

Thx!

@JmillsExpensify
Copy link

$250 approved for @ntdiary

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
Status: Done
Development

No branches or pull requests

9 participants