Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.55k stars 2.89k forks source link

[HOLD #11768] [$1000] Hide create a workspace page when app is loading #12128

Closed chiragsalian closed 1 year ago

chiragsalian commented 2 years ago

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Go to http://localhost:8080/settings/workspaces in the URL while being logged out.
  2. Log into the app.

Expected Result:

Workspace should show a loader and show workspaces when ready

Actual Result:

The create a workspace screen flashes for a second before showing the workspaces

https://user-images.githubusercontent.com/25876548/197101377-e61a926a-bb24-461d-a79e-30fce8136ea4.mov

Workaround:

Can the user still use Expensify without this being fixed? - Yes. this is a polish to improve experience.

Platform:

Where is this issue occurring?

Version Number: Reproducible in staging?: Y Reproducible in production?: N Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Notes/Photos/Videos: Any additional supporting documentation Expensify/Expensify Issue URL: Issue reported by: Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01b610ee831855a28a
  • Upwork Job ID: 1636765010047090688
  • Last Price Increase: 2023-03-17
chiragsalian commented 1 year ago

Low priority so haven't gotten to it yet

chiragsalian commented 1 year ago

Low priority so haven't gotten to it yet

chiragsalian commented 1 year ago

Low priority so haven't gotten to it yet

mountiny commented 1 year ago

@chiragsalian would you be interested on outsourcing this to @narefyev91 from Callstack?

Milos213 commented 1 year ago

Proposal

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

Empty workspace page shows before loading workspaces after login

What is the root cause of that problem?

https://github.com/Expensify/App/blob/bcdcfe595ecdcf5d45efadcc92e18c058bba76cc/src/pages/workspace/WorkspacesListPage.js#L185-L195 If workspaces are empty (no matter really empty or not loaded yet), we always show BlockingView with empty workspace title and subtitle.

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

We can early return FullScreenLoadingIndicator in render() if _.isEmpty(this.props.policies) = true. This condition should be enough to check if workspaces are loaded or not because at least 1 policy (Concierge) should exist for created account. This is working well even for new accounts.

MelvinBot commented 1 year ago

📣 @Milos213! 📣

Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:

  1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.

Screen Shot 2022-11-16 at 4 42 54 PM

Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>
MelvinBot commented 1 year ago

@chiragsalian Whoops! This issue is 2 days overdue. Let's get this updated quick!

MelvinBot commented 1 year ago

@chiragsalian Huh... This is 4 days overdue. Who can take care of this?

chiragsalian commented 1 year ago

whoops, my bad, i lost track of this one. I should have made this external ages ago. Just briefly tested and its still an issue.

Unassigning myself since I'm OOO for a bit but it should be great for someone external to address. Thanks for the proposal @Milos213 but I'll let the assigned C+ and engineer review it.

MelvinBot commented 1 year ago

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

MelvinBot commented 1 year ago

Triggered auto assignment to @adelekennedy (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

MelvinBot commented 1 year ago

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

MelvinBot commented 1 year ago

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

s77rt commented 1 year ago

@Milos213 Thanks for the proposal. But I think it's hacky to have the (free) workspaces coupled with the rest of the policies. Also looks like the issue is spread throughout the app and not just workspaces (e.g. http://localhost:8080/settings/payments) so I think we are looking for a generic solution here.

0xmiros commented 1 year ago

These kinds of issues are already on hold for navigation reboot: https://github.com/Expensify/App/issues/12428 https://github.com/Expensify/App/issues/12717 https://github.com/Expensify/App/issues/12719

MelvinBot commented 1 year ago

Looks like something related to react-navigation may have been mentioned in this issue discussion.

As a reminder, please make sure that all proposals are not workarounds and that any and all attempt to fix the issue holistically have been made before proceeding with a solution. Proposals to change our DeprecatedCustomActions.js files should not be accepted.

Feel free to drop a note in #expensify-open-source with any questions.

s77rt commented 1 year ago

Thanks @0xmiroslav It makes sense to hold. @Gonals What do you think?

s77rt commented 1 year ago

Not overdue. @Gonals ^

s77rt commented 1 year ago

@MelvinBot It's not overdue. Bad bot

adelekennedy commented 1 year ago

@Gonals I think we're pending your input above!

mountiny commented 1 year ago

Putting this on hold for navigation

s77rt commented 1 year ago

Still on hold

adelekennedy commented 1 year ago

on hold

s77rt commented 1 year ago

Same ^

s77rt commented 1 year ago

Still on hold...

s77rt commented 1 year ago

Same ^

MelvinBot commented 1 year ago

@s77rt, @Gonals, @adelekennedy Whoops! This issue is 2 days overdue. Let's get this updated quick!

MelvinBot commented 1 year ago

@s77rt, @Gonals, @adelekennedy Whoops! This issue is 2 days overdue. Let's get this updated quick!

s77rt commented 1 year ago

Still on hold

adelekennedy commented 1 year ago

on hold

Gonals commented 1 year ago

@MariaHCD, tagging you on this one as I'm going on parental leave! Thanks!

MelvinBot commented 1 year ago

@MariaHCD, @s77rt, @adelekennedy Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

MariaHCD commented 1 year ago

Looks like we're holding on the navigation reboot. Downgrading priority to weekly!

adelekennedy commented 1 year ago

@MariaHCD should we downgrade this further? This issues been open for 6+ months - I'm not sure if we should downgrade, close or keep it as is until we resolve the navigation issues

MariaHCD commented 1 year ago

I see there's progress being made on the navigation reboot so let's downgrade the priority further and pick this back up again.

s77rt commented 1 year ago

Still on hold.

mountiny commented 1 year ago

This is fixed by the refactor, there is no reporter so I am closing this