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

New Feature: Add a dedicated Workspaces page in settings #9971

Closed
trjExpensify opened this issue Jul 18, 2022 · 74 comments
Closed

New Feature: Add a dedicated Workspaces page in settings #9971

trjExpensify opened this issue Jul 18, 2022 · 74 comments
Assignees
Labels
Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff NewFeature Something to build that is a new item. Reviewing Has a PR in review Waiting for copy User facing verbiage needs polishing

Comments

@trjExpensify
Copy link
Contributor

trjExpensify commented Jul 18, 2022

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Deliverables

  1. Add Workspaces as a menu item on the Avatar > Settings page, incl. a preview of 2 workspace avatars right-aligned on the row
  2. Add a new page and move the list of workspaces to that page
  3. Add a New workspace green footer button to the bottom of that new page
  4. Remove New workspace from the three dots overflow menu that's accessed from a workspace (leave Delete workspace there)

Platform:

  • To be implemented consistently across all platforms

Notes/Photos/Videos:

image

Figma file

Expensify/Expensify Issue URL: N/A
Issue reported by: @trjExpensify
(Internal) Slack conversation: https://expensify.slack.com/archives/C038T8LKKV5/p1657879133021399

View all open jobs on GitHub

CC: @shawnborton @JmillsExpensify as you may be interested in following along.

@trjExpensify trjExpensify added Engineering Daily KSv2 NewFeature Something to build that is a new item. labels Jul 18, 2022
@melvin-bot
Copy link

melvin-bot bot commented Jul 18, 2022

Triggered auto assignment to @thienlnam (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@trjExpensify
Copy link
Contributor Author

I'll keep myself assigned for the CM portion. Passing this on to engineering first to triage.

@shawnborton
Copy link
Contributor

Love it!

@thienlnam
Copy link
Contributor

This will have to be on hold for API refactors (specifically for Policy_Create and Policy_Delete) cc @chiragsalian who is working on those -> currently held on Pattern B

After that, the rest of the items can be completed by an external contributor / or internal to make it quicker to correspond to design and UI items

Putting internal / on hold for now for these internal issues

https://github.com/Expensify/Expensify/issues/215184
https://github.com/Expensify/Expensify/issues/215181

@thienlnam thienlnam added Weekly KSv2 Internal Requires API changes or must be handled by Expensify staff and removed Daily KSv2 labels Jul 18, 2022
@thienlnam thienlnam changed the title New Feature: Add a dedicated Workspaces page in settings [HOLD API] New Feature: Add a dedicated Workspaces page in settings Jul 18, 2022
@trjExpensify
Copy link
Contributor Author

Ah cool, fair enough!

@melvin-bot melvin-bot bot added the Overdue label Jul 27, 2022
@thienlnam
Copy link
Contributor

Still holding

@melvin-bot melvin-bot bot removed the Overdue label Jul 28, 2022
@chiragsalian
Copy link
Contributor

Discussed 1:1 with jack. Taking over the issue.

@melvin-bot melvin-bot bot added the Overdue label Aug 12, 2022
@chiragsalian
Copy link
Contributor

The refactors all got unblocked and since those are the priority i'll first finish the API refactors and then get to this.

@melvin-bot melvin-bot bot removed the Overdue label Aug 15, 2022
@chiragsalian chiragsalian added Monthly KSv2 and removed Weekly KSv2 labels Aug 15, 2022
@trjExpensify
Copy link
Contributor Author

Wahoo, nice! 😍

@JmillsExpensify
Copy link

Ya, seriously! This is a nice improvement.

@roryabraham
Copy link
Contributor

If there's only one workspace, will we keep the same UI we have today?

@shawnborton
Copy link
Contributor

@NickTooker this help article about the free plan workspace might be helpful for you.

We basically need a title and a little blurb to help inform and encourage users.

@NickTooker
Copy link

NickTooker commented Oct 17, 2022

Perfect! Thanks, @shawnborton - back shortly!

@NickTooker
Copy link

For the title - I'm not sure we need to change much from the title in the current mock. I think this is exactly what we want the user to do.

For the text - I am pulling from the help article that Shawn provided (which was great!). I think we want to convey what the workspace does, and that it needs to be set-up in order to access it's features. Let me know you initial thoughts and we can tweak this together!

Create a Workspace
Workspaces are a one-stop shop for managing your settings, reimbursing receipts, managing cards, sending invoices, paying bills, and more! Create a new Workspace to access all available features!

@shawnborton
Copy link
Contributor

Nice, thanks Nick! What do you think about slightly trimming it down and omitting the !?

image

@NickTooker
Copy link

I think that looks great and reads much better. Thumbs up from my end!

@chiragsalian
Copy link
Contributor

chiragsalian commented Oct 17, 2022

awesome, thanks guys, i'll update the code.

@chiragsalian
Copy link
Contributor

While working on the red brick road testing for another issue i just remembered that with the changes here whats best for showing the red dot. Currently on production its like this,
image

Should we just show the red dot on the left of the avatars here?
image

Thoughts @shawnborton?

@shawnborton
Copy link
Contributor

That makes sense to me - though i think I would put the red dot to the right of the avatars so that the red dot is always in the same spot near the arrow?

@chiragsalian
Copy link
Contributor

Actually coming back to this, so shawn is it possible to provide mockups of how you want this to look with and without the red dot. Ideally, I would like things to be in the same place but I wanted to confirm the expectation. Will the avatars move a little left when there is a red dot and then move back more right when there is no red dot? Or were you thinking of static positions?

@shawnborton
Copy link
Contributor

Will the avatars move a little left when there is a red dot and then move back more right when there is no red dot?

This is exactly what I was thinking. Happy to provide a mock if needed, but I think with flexbox you would just add the red dot inbetween avatars and the right arrow, all vertically middle-aligned.

@chiragsalian
Copy link
Contributor

okay sounds good, i'll try that out.

@chiragsalian
Copy link
Contributor

okay, how does this look. Can you check if the alignments are good. Two screenshots, one without redbrick and one with,

Web - Without red brick,
image

Web - With red brick,
image

Mobile - Without Red brick
image

Mobile - With Red brick
image

Let me know if this looks good. The screenshot includes badge i.e., 0.00 next to Payments to confirm the alignment of workspaces and that was the same. I wasn't sure if it was, if i need to move it around let me know. Or if anything else should be adjusted.

@shawnborton
Copy link
Contributor

I think that looks pretty good to me! Perhaps the badge/red dot could be 4px closer to the arrow though?

@jamesdeanexpensify
Copy link
Contributor

I was looking back at the proposed copy for this screen, and I'm wondering if the suggestion below is still accurate and a bit more succinct? Assuming mostly admins will create workspaces, at least to start...

Create a new Workspace
Workspaces are where you'll chat with your team, reimburse expenses, issue cards, send invoices, pay bills, and more -- all in one place.

@chiragsalian
Copy link
Contributor

Sure, I moved the red dot 4px closer to the arrow. It looks like this,
image

Cool. I'm not sure of the copy so I'll let @NickTooker weigh in while i build that screen.

@shawnborton
Copy link
Contributor

Sweet, that looks great to me - thanks Chirag!

@chiragsalian
Copy link
Contributor

Shawn, does this look alright for the empty workspace page?
image

I feel like the subtitle is too close to the title, but i haven't changed it and just used the default values from the existing component. Let me know if you prefer I leave it like this or would prefer me to add some padding/margin after the title "Create a workspace".

Additionally, I'm waiting on the final copy before i ask for Spanish translation.

@shawnborton
Copy link
Contributor

I agree with you, let's add maybe 4 or 8 more pixels in between title and body text to that main component.

@chiragsalian
Copy link
Contributor

Done, this is margin-bottom 8,
Web -
image

Mobile -
image

I've updated the component vs having a different style just for this one. I think its fine since previous usages of the same component look fine to me,
image

@shawnborton
Copy link
Contributor

Nice, that looks good to me.

@chiragsalian
Copy link
Contributor

Moved the copy discussion to slack to maybe finalize it with more speed.

@JmillsExpensify
Copy link

Woo! This looks awesome. Great work.

@chiragsalian
Copy link
Contributor

Okay, I took some time fighting with the styles to get things consistent on Mobile/Web without any errors and testing took a bit of time too. Anyway, I finished testing, added screenshots, and just placed the code for review.

@chiragsalian chiragsalian added the Reviewing Has a PR in review label Oct 21, 2022
@melvin-bot melvin-bot bot closed this as completed Oct 27, 2022
@trjExpensify
Copy link
Contributor Author

Niceeee! Thanks, Chirag! ❤️

@aldo-expensify
Copy link
Contributor

@chiragsalian I think the red dot may be missing for the "Workspaces" entry in the settings menu:

Screen.Recording.2022-10-31.at.4.18.06.PM.mov

We may want to replace this line:

if (hasPolicyMemberError(policyMemberList) || hasCustomUnitsError(policy)) {

by

if (hasPolicyMemberError(policyMemberList) || hasCustomUnitsError(policy) || hasPolicyError(policy)) {

The detail with this fix is that it will make a red dot appear next to the workspace, and I think it shouldn't be there:

image

so.. the better fix I think would be to replace the line here:

.find(policy => PolicyUtils.getPolicyBrickRoadIndicatorStatus(policy, this.props.policyMembers))

by

.find(policy => PolicyUtils.getPolicyBrickRoadIndicatorStatus(policy, this.props.policyMembers) || PolicyUtils.hasPolicyError(policy))

image

@aldo-expensify
Copy link
Contributor

aldo-expensify commented Oct 31, 2022

Sorry, I think it is better if I just create a new GH issue for this and ping Chirag in case he wants to do it.

Update: created issue here: #12327 and draft PR: #12329

@chiragsalian
Copy link
Contributor

I think the red dot may be missing for the "Workspaces" entry in the settings menu:

That's odd. We tested the red dot for the Workspaces in the settings menu. I guess we must've missed a scenario since I tested it against members error and not an individual workspace error. Thanks for the catch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff NewFeature Something to build that is a new item. Reviewing Has a PR in review Waiting for copy User facing verbiage needs polishing
Projects
None yet
Development

No branches or pull requests

9 participants