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

Remove Save button on default rooms settings #8146

Closed
kevinksullivan opened this issue Mar 14, 2022 · 15 comments
Closed

Remove Save button on default rooms settings #8146

kevinksullivan opened this issue Mar 14, 2022 · 15 comments
Assignees
Labels
Daily KSv2 Engineering Reviewing Has a PR in review

Comments

@kevinksullivan
Copy link
Contributor

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. Join defaultRooms beta
  2. See default room - either #announce or #admins
  3. Click room avatar
  4. Click settings

Expected Result:

There should be no Save button on the room name, and the field should be clearly non-editable (i.e. no text box around it)

Actual Result:

The field looks editable and has a Save button

Platform:

  • Web
  • iOS
  • Android
  • Desktop App
  • Mobile Web

Notes/Photos/Videos:
image

Slack conversation: https://expensify.slack.com/archives/C02MW39LT9N/p1646421753395089?thread_ts=1646419035.253859&cid=C02MW39LT9N

View all open jobs on GitHub

@kevinksullivan kevinksullivan added AutoAssignerTriage Auto assign issues for triage to an available triage team member Daily KSv2 labels Mar 14, 2022
@MelvinBot
Copy link

Triggered auto assignment to @kadiealexander (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

@MelvinBot MelvinBot removed the AutoAssignerTriage Auto assign issues for triage to an available triage team member label Mar 14, 2022
@kevinksullivan
Copy link
Contributor Author

CC @Expensify/design @trjExpensify want to make sure you're all good with this change too!

@trjExpensify
Copy link
Contributor

Yep agreed, as you can't edit the name of a default room.

@shawnborton
Copy link
Contributor

cc @parasharrajat I think we were working together on this kind of input recently? Anyways I agree in that we should get rid of the save button, but I think I would also use a style more like the bottom example in terms of how to display this since it is read-only:

image

@kadiealexander kadiealexander removed their assignment Mar 14, 2022
@MelvinBot
Copy link

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

@parasharrajat
Copy link
Member

parasharrajat commented Mar 14, 2022

Yeah, So the Attached screenshot on the issue details is old. New Style is already merged but not yet available on PROD.

To reflect the read-only input, we have a grayish background color and the rest of the input is the same. This is the same for all inputs in the App.

But instead of trying to keep the same input design as the last one on this image, I suggest we use the normal Header and details section style.

image

So Image will have Room Name header and underneath #announce as content. Like this
image

@shawnborton
Copy link
Contributor

I think I disagree with your comment about the header style. I know it's still a bit of a work in progress, but we are actively trying to update our inputs to the style that I shared above.

@nickmurray47
Copy link
Contributor

@parasharrajat if you were working on a similar issue, do you mind linking me the PR?

Correct me if I'm wrong it sounds like the update here will not be for the "Save" button, but just around the header style.

@shawnborton
Copy link
Contributor

Yeah apologies for the confusion, I kind of distracted from the original post here. I think we need to remove the Save button, and then furthermore I am suggesting that we change the style of the input when it is read-only.

@shawnborton
Copy link
Contributor

Looking at a recent mockup, I think all of these small headers can be removed as they are redundant:
image

The bottom "Workspace" should look like the disabled input style I am suggesting in my post above.

@nickmurray47
Copy link
Contributor

got it, seems like a small enough issue that I can just whip up a quick PR and will tag you in it for style check when it's ready @shawnborton

@shawnborton
Copy link
Contributor

Sounds great, thanks!

@MelvinBot
Copy link

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

@MelvinBot
Copy link

@nickmurray47 Eep! 4 days overdue now. Issues have feelings too...

@MelvinBot
Copy link

@nickmurray47 Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Daily KSv2 Engineering Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests

8 participants