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.

Sign up for GitHub

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

Jump to bottom

Fix up workspace routes #3763

Merged
merged 5 commits into from
Jun 29, 2021
Merged

Fix up workspace routes #3763

merged 5 commits into from
Jun 29, 2021

Conversation

marcaaron
Copy link
Contributor

@marcaaron marcaaron commented Jun 25, 2021 •
edited
Loading

cc @parasharrajat we didn't really implement this 100% correctly as the routes need to be hooked up to the policy

Details

  • Hooks the workspace sidebar up to the chosen policy
  • Enables tapping on that policy in /settings

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/168732

Tests

  1. Create a new workspace
    2021-06-25_12-47-21
    2021-06-25_12-47-34
    2021-06-25_12-47-40
  2. Verify you are redirected to the workspace/:policyID/card route + the sidebar has the workspace name
  3. Close the modal
  4. Navigate to /settings by clicking on your avatar
  5. Verify the workspace appears in /settings/
  6. Tap on the workspace name
    2021-06-25_12-48-32
  7. Verify you are brought back to the workspace/:policyID/card route

QA Steps

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Mobile Web

Desktop

iOS

Android

@marcaaron marcaaron self-assigned this Jun 25, 2021
@parasharrajat
Copy link
Member

Sorry, didn't know about that part.

@marcaaron marcaaron marked this pull request as ready for review June 25, 2021 22:33
@marcaaron marcaaron requested a review from a team as a code owner June 25, 2021 22:33
@MelvinBot MelvinBot requested review from MariaHCD and removed request for a team June 25, 2021 22:34
MariaHCD
MariaHCD reviewed Jun 28, 2021
View reviewed changes
Copy link
Contributor

@MariaHCD MariaHCD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Just one thing I noticed - after creating the workspace, and then dismissing the WorkspaceCardPage modal, the NewWorkspacePage is still shown:

Expensify cash

@marcaaron
Copy link
Contributor Author

Looks great! Just one thing I noticed - after creating the workspace, and then dismissing the WorkspaceCardPage modal, the NewWorkspacePage is still shown:

Nice, agree that seems wrong.

@marcaaron
Copy link
Contributor Author

Updated

MariaHCD
MariaHCD approved these changes Jun 29, 2021
View reviewed changes
Copy link
Contributor

@MariaHCD MariaHCD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎊

@MariaHCD MariaHCD merged commit 5cde3ca into main Jun 29, 2021
@MariaHCD MariaHCD deleted the marcaaron-navToWorkspace branch June 29, 2021 06:12
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

This was referenced Jun 29, 2021
Closed
Closed
@OSBotify
Copy link
Contributor

🚀 Deployed to staging in version: 1.0.74-1🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MariaHCD MariaHCD MariaHCD approved these changes

Assignees

@marcaaron marcaaron

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@marcaaron @parasharrajat @OSBotify @MariaHCD