Open dragnoir opened 1 week ago
@eVoloshchak Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]
Some screenshots:
Groupe:
Room:
Admin room:
Deprecated group:
Long workspace name:
Long group name:
long room name:
Archived room:
@shawnborton you may want to review this!
For archived rooms, I think they should look identical to room names that can't be edited, like so:
No need to create a separate design style for archived rooms in my opinion. cc @Expensify/design for vis.
Otherwise will spin up a test build now.
:test_tube::test_tube: Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! :test_tube::test_tube: | Android :robot: | iOS :apple: |
---|---|---|
https://ad-hoc-expensify-cash.s3.amazonaws.com/android/40858/index.html | https://ad-hoc-expensify-cash.s3.amazonaws.com/ios/40858/index.html | |
Desktop :computer: | Web :spider_web: | |
https://ad-hoc-expensify-cash.s3.amazonaws.com/desktop/40858/NewExpensify.dmg | https://40858.pr-testing.expensify.com | |
:eyes: View the workflow run that generated this build :eyes:
Test build is looking pretty good!
Testing now and reporting a few things:
For workspace chats, we should also update the way we display the name:
From our mocks:
Note that we want to update the workspace room avatar, but I think we can handle that in a different PR.
For archived rooms, I think they should look identical to room names that can't be edited
Archived rooms don't belong to Workspaces. Do I have just to remove the part where the workspace is? "In....."
Archived rooms are indeed on workspaces, for example:
For workspace chats, we should also update the way we display the nam
for this, updates as we have on groups?
Workspace name: My WorkspaceName
So the thing with workspace rooms is that they have a different title depending if the Submitter or Admin is viewing them.
Submitter sees:
Admin currently sees:
So maybe something like this makes sense?
Thoughts @Expensify/design?
I think the "Workspace" label will be double, top and bottom
Yeah, that's why I am suggesting we just reuse "Room name" - see my mock above
Also I don't think you can rename those, so there wouldn't be an arrow on the right.
Looks good to me Shawn & I agree.
And here's a quick mock of an archived room for reference. Look right to you @shawnborton?
Love it, that looks perfect to me.
That looks great from my end as well
@shawnborton any idea how can I simulate the "Submitter View" for a workspace?
Create a workspace by another user then submit a report to it by my account?
Yup, create a workspace as an "Admin" and then invite an "Employee" to your workspace. Then from the employee's perspective, submit an expense to the workspace.
Admin View
Submitter View
I think that's looking good. cc @JmillsExpensify - just letting you know we're getting a head start on the room name styles here, though I know we might have more changes coming with your doc.
@shawnborton what about invoices? 🤔
@eVoloshchak pls if we can close this ASAP, daily updates and conflicts 😁
In that case, I think we can drop the "In Invoices" and just say "Invoices"
Also in talking with @JmillsExpensify, I think we want it to say "Name" instead of "Room name" for workspace chats and invoice chats.
Details
This PR move the Group Chat "Group/Room name" field out of Settings and into Report Details
Fixed Issues
$ https://github.com/Expensify/App/issues/40262 PROPOSAL: https://github.com/Expensify/App/issues/40262#issuecomment-2058957823
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
![image](https://github.com/Expensify/App/assets/12425932/71072929-a973-46b9-8ef7-ac347e583291)Android: mWeb Chrome
![image](https://github.com/Expensify/App/assets/12425932/51e20ad2-932f-4a98-a5b3-a1e24a842c27)iOS: Native
![image](https://github.com/Expensify/App/assets/12425932/b5922d91-e821-4505-bc01-177b67694e7b)iOS: mWeb Safari
![image](https://github.com/Expensify/App/assets/12425932/96e75d89-7653-424b-ad5b-0ad01c0b3700)MacOS: Chrome / Safari
![image](https://github.com/Expensify/App/assets/12425932/a976e83b-f665-44f6-b86e-5b0a32266a5a)MacOS: Desktop
![image](https://github.com/Expensify/App/assets/12425932/8a3236f9-6cc0-4202-b6c3-0b2f3f94a48f)