[$500] Web - Workspace - Workspace created in offline mode is not grayed out on Choose a workspace page

[kbecciv]

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.36-0 Reproducible in staging?: y Reproducible in production?: n 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: Applause - Internal Team Slack conversation:

Action Performed:

1. Go offline
2. Create new workspace
3. Navigate to the Choose a workspace page

Expected Result:

Newly created workspace in offline should be grayed out

Actual Result:

Workspace created in offline mode is not grayed out

Workaround:

Unknown

Platforms:

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

• [ ] Android: Native
• [ ] Android: mWeb Chrome
• [ ] iOS: Native
• [ ] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/93399543/b8c5d263-54e4-4a30-be6b-998dbd8719c5

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0124a63474e95d7dc5
• Upwork Job ID: 1753546948780208128
• Last Price Increase: 2024-02-02

[github-actions[bot]]

:wave: 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.

[melvin-bot[bot]]

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

[kbecciv]

We think that this bug might be related to #wave8-collect-admins CC @zanyrenney

[francoisl]

Sounds somewhat similar to https://github.com/Expensify/App/issues/35702.

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[ZhenjaHorbach]

Proposal

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

Web - Workspace - Workspace created in offline mode is not grayed out on Choose a workspace page

What is the root cause of that problem?

The main problem with issue is that we don't pass pending action We can't use OfflineWithFeedback without pending action

https://github.com/Expensify/App/blob/6a889f1fc78a99c35b75503e7f4409860673b4bc/src/pages/WorkspaceSwitcherPage.tsx#L125-L142

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

To fix this issue we can update this function and add a new field

pendingAction: policy?.pendingFields?.generalSettings || policy?.pendingAction

I add pendingFields?.generalSettings because when we update the general setting we add pending action there

But I think we can update optimistic data and add pendingAction field

https://github.com/Expensify/App/blob/d122676c28e15c08e8a4d30c0838b9396fdb9a83/src/libs/actions/Policy.ts#L828-L855

Plus we have problem with OptionRow since we get crash app since we use Hoverable and PressableWithFeedback inside OfflineWithFeedback

https://github.com/Expensify/App/blob/664626c3600065c1d8ee407fe5b8dcadf6ec5622/src/components/OptionRow.tsx#L163-L205

To fix this we need delete Hoverable and use only PressableWithFeedback(In any case we only need hovered state )

Or move OfflineWithFeedback inside Hoverable or PressableWithFeedback

What alternative solutions did you explore? (Optional)

NA

[Tony-MK]

Proposal

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

Web - Workspace - Workspace created in offline mode is not grayed out on choosing a workspace page.

What is the root cause of that problem?

In the WorkspaceOverviewPage, the pendingAction for the OfflineWithFeedback component only checks for pendingFields.generalSettings. That is why it only dims when the title is edited because pendingFields.generalSettings is "update".

https://github.com/Expensify/App/blob/3ab4e6e12d9ff38e69493321f0d31f032481b373/src/pages/workspace/WorkspaceOverviewPage.js#L109

https://github.com/Expensify/App/blob/3ab4e6e12d9ff38e69493321f0d31f032481b373/src/pages/home/sidebar/PressableAvatarWithIndicator.js#L59-L61

Also, the SidebarUtils.getOptionData function does not check for report.pendingAction and has the value "add" when the policy was created.

Thus, the OptionRowLHNData sidebar component won't dim. https://github.com/Expensify/App/blob/3ab4e6e12d9ff38e69493321f0d31f032481b373/src/libs/SidebarUtils.ts#L297

https://github.com/Expensify/App/blob/3ab4e6e12d9ff38e69493321f0d31f032481b373/src/libs/Navigation/AppNavigator/createCustomBottomTabNavigator/BottomTabBar.tsx#L66-L67

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

For the WorkspaceOverviewPage, we should modify the pendingAction {lodashGet(policy, 'pendingFields.generalSettings') || lodashGet(policy, 'pendingAction')} of the OfflineWithFeedback component here and if required for the AvatarWithImagePicker over here.

For example :

https://github.com/Expensify/App/blob/3ab4e6e12d9ff38e69493321f0d31f032481b373/src/pages/workspace/WorkspaceOverviewPage.js#L100

pendingAction={lodashGet(policy, 'pendingFields.generalSettings') || lodashGet(policy, 'pendingAction')}

Alternatively, we could wrap the WorkspacePageWithSections component with a OfflineWithFeedback component. Then, use the policy.pendingAction for the pendingAction prop. Finally, set shouldDisableOpacity for the AvatarWithImagePicker and the inner OfflineWithFeedback component if it is present policy.pendingAction to prevent double dimming.

Lastly, for the SidebarUtils.getOptionData function, introduce report.pendingAction in the OR condition which determines result.pendingAction alongside report.pendingFields.addWorkspaceRoom and `report.pendingFields.createChat.

What alternative solutions did you explore? (Optional)

Create a pendingFields attribute for the policy in the onyx and introduce it WorkspaceOverviewPage and SidebarUtils.getOptionData. It will function like createChat and be used like pendingAction in the earlier solution.

[mountiny]

Demoting, not a blocker

[melvin-bot[bot]]

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

[mananjadhav]

@mountiny @tylerkaraszewski I just reviewed an offline issue for Delete Workspace. Both the issues need different changes, but there might be an overlap such as changing the wrapping order.

But are we planning to consolidate all the new Workspace list offline issues in one? or we can work on these separately?

[mountiny]

I think we can work on this in one issue, create and delete workspace offline and ensure the feedback is correct.

[mananjadhav]

Cool. Tracking it here.

[ZhenjaHorbach]

I think we can work on this in one issue, create and delete workspace offline and ensure the feedback is correct.

Sorry. But these are completely different tickets that are in no way related to each other Different root causes, different components, etc.)

[hayata-suenaga]

@ZhenjaHorbach could you kindly explain why do you think these issues have separate root causes?

[Tony-MK]

I don't mind combining the two issues, but I agree with @ZhenjaHorbach.

@hayata-suenaga, If I am not wrong, the other issue deals with changing the WorkspacesListPage.tsx due to the ordering and styling of the components surrounding WorkspaceListRow.

While this issue occurs because the policy.pendingAction, "add", is not used in the pendingAction prop for the OfflineWithFeedback components of WorkspaceOverviewPage and OptionRowLHNData.

[hayata-suenaga]

okay that makes sense. then let's handle these issues separately. @mananjadhav please select a proposal when you have time 🙇

[mananjadhav]

@Tony-MK's proposal looks good to me, as it covers all cases for add workspace.

🎀 👀 🎀 C+ reviewed.

[melvin-bot[bot]]

Current assignee @tylerkaraszewski is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[ZhenjaHorbach]

@Tony-MK's proposal looks good to me, as it covers all cases for add workspace.

🎀 👀 🎀 C+ reviewed.

Sorry, I don't mind your decision But does it fix the problem in this issue? Issue related to fix Choose a workspace page (This is WorkspaceSwitcherPage ) In turn, the selected proposal makes changes in a completely different place and doesn't fix the main issue

[hayata-suenaga]

@mananjadhav could you check @ZhenjaHorbach's comment?

[mananjadhav]

I'll check the comment and the proposal again to confirm.

[Tony-MK]

Hey all,

I have to say about this https://github.com/Expensify/App/issues/35732#issuecomment-1930697090 which I think should be part of the expected result.

In turn, the selected proposal makes changes in a completely different place

I believe that this is because my proposal included handling the add pending action for the WorkspaceOverviewPage.

Issue related to fix Choose a workspace page (This is WorkspaceSwitcherPage )

Secondly, it also handles the pending action for the WorkspaceSwitcherPage and the Sidebar when the user is on the home screen.

Finally, I believe my solution was less complicated and regression-prone than changing Onyx's data.

@Tony-MK's https://github.com/Expensify/App/issues/35732#issuecomment-1924993724 looks good to me, as it covers all cases for add workspace.

Therefore, my proposal should fix all the cases as @mananjadhav stated in this https://github.com/Expensify/App/issues/35732#issuecomment-1930686323

I can provide the test branch if you want to review it.

However, I am happy with the C+'s and engineer's decisions.

Thanks.

[ZhenjaHorbach]

Hey all,

I have to say about #35732 (comment)

In turn, the selected proposal makes changes in a completely different place

I believe this because my proposal included handling the pending action for the WorkspaceOverviewPage.

Issue related to fix Choose a workspace page (This is WorkspaceSwitcherPage )

Secondly, it also handles the pending action for the WorkspaceSwitcherPage and the Sidebar when the user is on the home screen.

Finally, my solution was less complicated and regression-prone than changing Onyx's data.

@Tony-MK's #35732 (comment) looks good to me, as it covers all cases for add workspace.

Therefore, my proposal should fix all the as @mananjadhav stated in this #35732 (comment)

I can provide the test branch if you want to review it.

However, I am happy with the C+'s and engineer's decisions.

In my proposal I do not indicate that we are obliged to change the onyx's data

And I don't mind if your proposition is better ) It’s just not clear how your proposal fixes WorkspaceSwitcherPage Although this issue requires it

[Tony-MK]

To fix this issue we can update this function and add a new field

pendingAction: policy?.pendingFields?.generalSettings || policy?.pendingAction

I add pendingFields?.generalSettings because when we update the general setting we add pending action there

But I think we can update optimistic data and add pendingAction field

https://github.com/Expensify/App/blob/d122676c28e15c08e8a4d30c0838b9396fdb9a83/src/libs/actions/Policy.ts#L828-L855

@ZhenjaHorbach, then it seems I did not understand this part, before mentioning the Hoverable. Could you elaborate further?

It’s just not clear how your proposal fixes WorkspaceSwitcherPage

The SidebarUtils.getOptionData function also handles the add pending action for the WorkspaceSwitcherPage.

[ZhenjaHorbach]

To fix this issue we can update this function and add a new field

pendingAction: policy?.pendingFields?.generalSettings || policy?.pendingAction

I add pendingFields?.generalSettings because when we update the general setting we add pending action there

But I think we can update optimistic data and add pendingAction field

https://github.com/Expensify/App/blob/d122676c28e15c08e8a4d30c0838b9396fdb9a83/src/libs/actions/Policy.ts#L828-L855

@ZhenjaHorbach, then it seems I did not understand this part, before mentioning the Hoverable. Could you elaborate further?

It’s just not clear how your proposal fixes WorkspaceSwitcherPage

The SidebarUtils.getOptionData function also handles the add pending action for the WorkspaceSwitcherPage.

We don't use SidebarUtils.getOptionData inside WorkspaceSwitcherPage )

This is an idea to update, but I have already proposed an option without Onyx changes

[Tony-MK]

We don't use SidebarUtils.getOptionData inside WorkspaceSwitcherPage )

@ZhenjaHorbach, we do. It works. It will also handle the home screen.

Also, I think we need to fix the WorkspaceOverviewPage as well.

[ZhenjaHorbach]

We don't use SidebarUtils.getOptionData inside WorkspaceSwitcherPage )

@ZhenjaHorbach, we do. It works. It will also handle the home screen.

Also, I think we need to fix the WorkspaceOverviewPage as well.

In WorkspaceSwitcherPage we use only OptionRow instead of OptionRowLHNData (And here we use SidebarUtils.getOptionData) So any changes related to SidebarUtils.getOptionData will not cause any effect on OptionRow

Sorry, if I'm wrong, but can you double-check ?) Because I think your changes won't change anything in WorkspaceSwitcherPage

[Tony-MK]

It should work because I have tested it.

However, let's hear the C+'s decision.

[mananjadhav]

Folks I was finishing up some PRs, and I'll be focusing on this one today. Please bear with me for one day.