Added workspace Settings Modal #3602

parasharrajat · 2021-06-15T23:34:06Z

@marcaaron @roryabraham Please review. I believe needs improvements.

Details

Fixed Issues

Fixes #3566

Tests

QA Steps

Tested On

Screenshots

Web

Overall design

workspace.mp4

When user is on Private Domain

When user Is on public Domain

When the user has card Enabled

Final Web | Desktop

workspace-w.mp4

Mobile Web

workspace-m.mp4

iOS

Android

workspace-a.mp4

shawnborton · 2021-06-15T23:38:28Z

I know this is a WIP but going to start leaving some design feedback here so you have it. If you take a look at the original mockups, I think the goal is to have the workspace appear as a modal on wide screens - similar to how viewing an attachment looks. On smaller screens, we basically just repeat the same navigation pattern as the chat portion of the app where we have a menu (the left list of chats) where you can navigate to a specific page (aka a chat).

There's a couple of other small tweaks I am noticing - the background color on the right side should be white, there should be a small border between the left and the right pane (on desktop), and the "Borton Enterprises" area should basically reuse the exact styling when you view your own profile and see an avatar + name underneath.

Also cc @michelle-thompson to follow along for this one too.

parasharrajat · 2021-06-15T23:44:51Z

I am not sure what would be the Mobile behavior for this. But I have used back type for the drawer which behaves like stack and Workspace Card page slides from right above the drawer.

Q: Please let me know what would be mobile look and behavior of this?

The page will connect to ONYXKEYS.USER and look at the isOnPublicDomain property

Here the property is wrong but I improvised.

So we have three different conditions.

If the user is on a public domain, the “Get Started” button will be hidden and some alternate copy will be shown.
If the user is on a non-public domain, and the domain is already provisioned for the expensify card, then this will display a link for configuring cards that will link to OldDot. (expensify.com.dev/domain_companycards)
In all other cases (if they have no Verified Business Account ["VBA"], or if they have any number of VBAs), this will display a button that says “Get Started”. The button will navigate the user to http://.../bankAccount/new.

Questions:

Where will we show the link? what would be the link text? What will happen to Get Started Button? Color of the Link? Will it be underlined? This (expensify.com.dev/domain_companycards) domain does not lead anywhere?
How to check if the user has no Verified Business Account or they have any number of VBAs ?

parasharrajat · 2021-06-15T23:45:27Z

Thanks, @shawnborton I was about to tag you. Please look at the above questions.

src/languages/en.js

marcaaron

Looks like a really great start to me overall. I left mostly small comments and non blockers but there are a few requests. 🚀

src/ROUTES.js

src/languages/en.js

src/libs/Navigation/AppNavigator/AuthScreens.js

src/libs/Navigation/AppNavigator/WorkspaceSettingsDrawerNavigator.js

src/libs/Navigation/AppNavigator/fullscreenModalcardStyleInterpolator.js

src/libs/Navigation/linkingConfig.js

src/pages/workspace/WorkspaceCard.js

src/styles/styles.js

parasharrajat · 2021-06-16T00:39:46Z

Please help me with these questions @marcaaron. #3602 (comment)

marcaaron · 2021-06-16T01:01:14Z

I am not sure what would be the Mobile behavior for this. But I have used back type for the drawer which behaves like stack and Workspace Card page slides from right above the drawer. Q: Please let me know what would be mobile look and behavior of this?

This behavior makes sense to me, but perhaps @shawnborton has another idea. My feeling is that we can start with this and switch it up later, but probably design has a better idea than me of what would be best.

Where will we show the link? what would be the link text? What will happen to Get Started Button? Color of the Link? Will it be underlined?

I'll let @shawnborton take those.

This (expensify.com.dev/domain_companycards) domain does not lead anywhere?

This should be https://expensify.com/domain_companycards and not have the .dev (which is for local use)

How to check if the user has no Verified Business Account or they have any number of VBAs ?

I think the language here is perhaps confusing and there is no need to check whether they have "no VBA" or "any number of VBAs" since that would cover all possible scenarios 😅

So, I think the relevant information we need is just:

Is the user on a public domain
Is that domain already configured to use the Expensify card

cc @MitchExpensify @kevinksullivan to make sure that last assumption is correct

shawnborton · 2021-06-16T17:00:17Z

I am not sure what would be the Mobile behavior for this. But I have used back type for the drawer which behaves like stack and Workspace Card page slides from right above the drawer. Q: Please let me know what would be mobile look and behavior of this?

I think it would behave just like navigating to a chat from the home screen.

Where will we show the link? what would be the link text? What will happen to Get Started Button? Color of the Link? Will it be underlined?

Apologies as I am not following here - what is the link that you referencing?

kevinksullivan · 2021-06-16T17:03:49Z

So, I think the relevant information we need is just:

Is the user on a public domain
Is that domain already configured to use the Expensify card

@marcaaron this is correct, so we do not have to check whether a VBA exists or not. Just if the user is on a public domain and if not, whether that domain has an enabled expensify card already.

Public domain

"Copy: In order to use the Expensify Card you must use your company's private domain. Go ahead and add your private email address as a secondary login [here]."
[here] will link to https://www.expensify.com/settings?param={%22section%22:%22account%22}

Non-Public Domain, Expensify Card already enabled

show this page, where "manage cards" links to https://www.expensify.com/domain_companycards

Non-Public Domain, Expensify Card not enabled

Where will we show the link? what would be the link text? What will happen to Get Started Button? Color of the Link? Will it be underlined? This (expensify.com.dev/domain_companycards) domain does not lead anywhere?

How to check if the user has no Verified Business Account or they have any number of VBAs ?

…pace

parasharrajat · 2021-06-16T20:45:41Z

@roryabraham How to check if domain is already provisioned for the expensify card or Expensify Card already enabled ?

parasharrajat · 2021-06-16T21:04:52Z

@roryabraham
Lols. I am thrilled. How can I guess the exactly correct key without even looking at the data or knowing context. I used isCardEnabled which is correct. I can also say that you chose this variable name 🕵️

…pace

parasharrajat · 2021-06-22T20:45:43Z

@shawnborton I checked the profile page Avatar size and font size for the name but that seems smaller in comparison to the mockups here. Their font size is 17 and the avatar size is 80x80. What are the values here?

parasharrajat · 2021-06-22T21:04:30Z

@marcaaron I assume that the Close button on both sidebar and Card page should close the whole workspace Modal and the back arrow should just shift back in history. I have used HeaderWithbackbutton Component and it does not seem to work well. The back arrow and close button on the sidebar do not do anyting and the Close button on Arrow sometimes closes the modal and sometimes takes us back to the Report Page. I would like to ask what should I do here.

I am thinking custom handler for the card page back button to OpenDrawer. Not sure about the dismissModal.

marcaaron · 2021-06-22T21:14:54Z

I assume that the Close button on both sidebar and Card page should close the whole workspace Modal

That's correct.

and the back arrow should just shift back in history

No, I don't think it would change the history. We will want to just open the drawer for the new drawer navigator. You can take a look at how we handle this for the ReportView + sidebar here e.g.

https://github.com/Expensify/Expensify.cash/blob/2c831b06e70b19a932165921416741d8b5561d56/src/pages/home/ReportScreen.js#L90-L93
https://github.com/Expensify/Expensify.cash/blob/2c831b06e70b19a932165921416741d8b5561d56/src/libs/Navigation/Navigation.js#L46-L56

But let me know if this doesn't work and I can help figure out alternatives.

parasharrajat · 2021-06-23T18:55:59Z

#3602 (comment)

@shawnborton Any input? Thanks.

parasharrajat · 2021-06-23T19:52:06Z

@marcaaron
So I tried navigation options. But I am not able to figure out how to navigate back to the main drawer. If I call dismissModal on the Close button on Sidebar. It does not do anything. It keeps me in the WorkspaceSidebar. These nested navigators are very confusing.

I did

function navigate(route = ROUTES.HOME) {
    if (route === ROUTES.HOME || route === ROUTES.WORKSPACE) {
        if (isLoggedIn) {
            openDrawer();
            return;
        }

        // If we're navigating to the signIn page while logged out, pop whatever screen is on top
        // since it's guaranteed that the sign in page will be underneath (since it's the initial route).
        navigationRef.current.dispatch(StackActions.pop());
        return;
    }
    ......

added check for workspace route here. Code is updated to reflect that.

marcaaron · 2021-06-23T20:08:39Z

Will pull down the branch in a bit and see if I can think of anything.

parasharrajat · 2021-06-23T20:44:47Z

@michelle-thompson Could you please help me deciding this #3602 (comment)?

marcaaron · 2021-06-23T21:00:17Z

@parasharrajat it seems like what's happening here is that we do not remove the WorkspaceSettings screen when we call dismissModal() because the call to goBack() will open the drawer and then an additional call to openDrawer() is made which will do nothing since it's already open.

I believe this is because those methods will refer to the drawer navigator that is on the top of the stack. So, what we need to do is:

Check to see if we have a drawer when dismissing the modal
Pop the screen instead
Don't call goBack() or openDrawer() in this case since we should return the user back to whatever is sitting underneath the second drawer navigator.

diff --git a/src/libs/Navigation/Navigation.js b/src/libs/Navigation/Navigation.js
index 55a222d15..1cab26ba4 100644
--- a/src/libs/Navigation/Navigation.js
+++ b/src/libs/Navigation/Navigation.js
@@ -78,6 +78,11 @@ function dismissModal(shouldOpenDrawer = false) {

     // This should take us to the first view of the modal's stack navigator
     navigationRef.current.dispatch((state) => {
+        // If this is a nested drawer navigator then we must pop this screen
+        if (state.type === 'drawer') {
+            return StackActions.pop();
+        }
+
         // If there are multiple routes then we can pop back to the first route
         if (state.routes.length > 1) {
             return StackActions.popToTop();

I tested this change and I think it will work, but let me know what you think.

parasharrajat · 2021-06-23T21:08:28Z

That makes sense to me. let me try.

parasharrajat · 2021-06-23T21:32:34Z

That worked like a charm. Thanks for the help. Code pushed. Waiting for Final Design related questions. It seems that I have spent most of the time waiting for getting answers related to designs.

parasharrajat · 2021-06-23T23:19:57Z

@shawnborton It slipped from my mind. I will be needing those icons on the left sidebar. I know that this was not part of original task but thought it can be combined so I made the sidebar as well.
Thank you.

shawnborton · 2021-06-24T00:11:09Z

I think the people icon already exists as we use it in the Create menu for Groups.

Here is the Expensify Card icon: expensifycard.svg.zip

shawnborton · 2021-06-24T00:13:35Z

@parasharrajat regarding your comment above about the avatar and font size, I think ideally we use the same avatar and font size as this view here:

marcaaron

Just a few comments mostly looking great to me.

marcaaron · 2021-06-24T00:23:03Z

src/libs/Navigation/Navigation.js

@@ -43,7 +43,7 @@ function goBack() {
 * @param {String} route
 */
 function navigate(route = ROUTES.HOME) {
-    if (route === ROUTES.HOME) {
+    if (route === ROUTES.HOME || route === ROUTES.WORKSPACE) {


NAB, when we are navigating to /workspace we are assuming that we're already navigated to /workspace/card or some other route. We should maybe add a comment to clarify what's happening here and the intended behavior e.g.

If someone tries to navigate to /workspace when the workspace drawer is not open the home drawer will toggle. In this case, we should navigate to a sub-route of workspace instead e.g. /workspace/card

Actually, this also kind of makes me think that it might be incorrect to have this check in general right?

We already established that calling Navigation.goBack() will toggle the drawer. So maybe we can do that instead of navigating to /workspace when we want to toggle the workspace sidebar open?

But when we will navigate over the WorkspaceSetting Modal, then go back will switch those screens, Let's say first we got Card page then people page. So on the people page, goback should take us to Card apge.

@marcaaron Please tell me if I need to do these changes as well. Otherwise, PR is ready for review.

I'm not really following as there's no way to access the the /workspace/people from /workspace/card, but it's not a blocker for me just something that seems weird / confusing.

Ok. I think it's better than we can adjust that later when we implement other pages like /workspace/people.

src/libs/Navigation/Navigation.js

src/pages/workspace/WorkspaceSidebar.js

src/pages/workspace/WorkspaceCardPage.js

src/pages/workspace/WorkspaceSidebar.js

src/pages/workspace/WorkspaceCardPage.js

marcaaron

LGTM, great work @parasharrajat ! 🙇

OSBotify · 2021-06-24T17:42:19Z

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

OSBotify · 2021-06-25T18:47:02Z

🚀 Deployed to staging in version: 1.0.73-4🚀

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

OSBotify · 2021-06-29T18:58:08Z

🚀 Deployed to production in version: 1.0.74-0🚀

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

parasharrajat added 6 commits June 16, 2021 02:04

new: added workspace settings route

77f69b9

added workspace drawer

35a41c2

workspace sidebar and styling for drawer

2e6e92a

added workspace card page

9df04bc

fix: overlay & refactor

0898f77

pull latest changes

759fcdf

parasharrajat requested a review from a team as a code owner June 15, 2021 23:34

parasharrajat changed the title ~~Added workspace Settings Modal~~ [WIP] Added workspace Settings Modal Jun 15, 2021

MelvinBot requested review from Beamanator and removed request for a team June 15, 2021 23:34

small changes

8b049d9

marcaaron self-requested a review June 15, 2021 23:58

marcaaron assigned parasharrajat Jun 15, 2021

parasharrajat commented Jun 16, 2021

View reviewed changes

src/languages/en.js Outdated Show resolved Hide resolved

marcaaron requested changes Jun 16, 2021

View reviewed changes

Merge branch 'main' of github.com:Expensify/Expensify.cash into works…

2839caf

…pace

parasharrajat added 4 commits June 17, 2021 02:37

workspace card cleanup

051b5dc

adjusted screen options for modals

a4e1ea5

refactor Drawer options

7f2dd52

refactor Workspace Card page

4c320f7

parasharrajat requested a review from marcaaron June 16, 2021 22:03

parasharrajat added 2 commits June 22, 2021 23:04

Merge branch 'main' of github.com:Expensify/Expensify.cash into works…

38b7801

…pace

rename var

7c146a9

fix issues

6f6666c

Pulled latest changes

6d9bda1

update

9242841

fix navigation

ba310bc

marcaaron requested changes Jun 24, 2021

View reviewed changes

marcaaron reviewed Jun 24, 2021

View reviewed changes

src/pages/workspace/WorkspaceCardPage.js Show resolved Hide resolved

updated UI and Small refactor

3a13450

marcaaron approved these changes Jun 24, 2021

View reviewed changes

marcaaron merged commit aee074b into Expensify:main Jun 24, 2021

This was referenced Jun 25, 2021

Deploy Checklist: Expensify.cash 2021-06-15 #3593

Closed

Deploy Checklist: Expensify.cash 2021-06-25 #3759

Closed

parasharrajat deleted the workspace branch November 20, 2023 13:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added workspace Settings Modal #3602

Added workspace Settings Modal #3602

parasharrajat commented Jun 15, 2021 •

edited

Loading

shawnborton commented Jun 15, 2021

parasharrajat commented Jun 15, 2021

parasharrajat commented Jun 15, 2021

marcaaron left a comment

parasharrajat commented Jun 16, 2021

marcaaron commented Jun 16, 2021

shawnborton commented Jun 16, 2021

kevinksullivan commented Jun 16, 2021 •

edited

Loading

parasharrajat commented Jun 16, 2021

parasharrajat commented Jun 16, 2021

parasharrajat commented Jun 22, 2021

parasharrajat commented Jun 22, 2021

marcaaron commented Jun 22, 2021

parasharrajat commented Jun 23, 2021 •

edited

Loading

parasharrajat commented Jun 23, 2021 •

edited

Loading

marcaaron commented Jun 23, 2021

parasharrajat commented Jun 23, 2021

marcaaron commented Jun 23, 2021

parasharrajat commented Jun 23, 2021

parasharrajat commented Jun 23, 2021 •

edited

Loading

parasharrajat commented Jun 23, 2021

shawnborton commented Jun 24, 2021

shawnborton commented Jun 24, 2021

marcaaron left a comment

marcaaron Jun 24, 2021

marcaaron Jun 24, 2021

parasharrajat Jun 24, 2021

parasharrajat Jun 24, 2021

marcaaron Jun 24, 2021

parasharrajat Jun 24, 2021

marcaaron left a comment

OSBotify commented Jun 24, 2021

OSBotify commented Jun 25, 2021

OSBotify commented Jun 29, 2021

Added workspace Settings Modal #3602

Added workspace Settings Modal #3602

Conversation

parasharrajat commented Jun 15, 2021 • edited Loading

Details

Fixed Issues

Tests

QA Steps

Tested On

Screenshots

Web

Overall design

When user is on Private Domain

When user Is on public Domain

When the user has card Enabled

Final Web | Desktop

Mobile Web

iOS

Android

shawnborton commented Jun 15, 2021

parasharrajat commented Jun 15, 2021

parasharrajat commented Jun 15, 2021

marcaaron left a comment

Choose a reason for hiding this comment

parasharrajat commented Jun 16, 2021

marcaaron commented Jun 16, 2021

shawnborton commented Jun 16, 2021

kevinksullivan commented Jun 16, 2021 • edited Loading

Non-Public Domain, Expensify Card not enabled

parasharrajat commented Jun 16, 2021

parasharrajat commented Jun 16, 2021

parasharrajat commented Jun 22, 2021

parasharrajat commented Jun 22, 2021

marcaaron commented Jun 22, 2021

parasharrajat commented Jun 23, 2021 • edited Loading

parasharrajat commented Jun 23, 2021 • edited Loading

marcaaron commented Jun 23, 2021

parasharrajat commented Jun 23, 2021

marcaaron commented Jun 23, 2021

parasharrajat commented Jun 23, 2021

parasharrajat commented Jun 23, 2021 • edited Loading

parasharrajat commented Jun 23, 2021

shawnborton commented Jun 24, 2021

shawnborton commented Jun 24, 2021

marcaaron left a comment

Choose a reason for hiding this comment

marcaaron Jun 24, 2021

Choose a reason for hiding this comment

marcaaron Jun 24, 2021

Choose a reason for hiding this comment

parasharrajat Jun 24, 2021

Choose a reason for hiding this comment

parasharrajat Jun 24, 2021

Choose a reason for hiding this comment

marcaaron Jun 24, 2021

Choose a reason for hiding this comment

parasharrajat Jun 24, 2021

Choose a reason for hiding this comment

marcaaron left a comment

Choose a reason for hiding this comment

OSBotify commented Jun 24, 2021

OSBotify commented Jun 25, 2021

OSBotify commented Jun 29, 2021

parasharrajat commented Jun 15, 2021 •

edited

Loading

kevinksullivan commented Jun 16, 2021 •

edited

Loading

parasharrajat commented Jun 23, 2021 •

edited

Loading

parasharrajat commented Jun 23, 2021 •

edited

Loading

parasharrajat commented Jun 23, 2021 •

edited

Loading