[HOLD #90204 ] Add workspace rooms list rows#91056
Conversation
…ski/feature/workspace-rooms-list
…re/workspace-rooms-list
- Use styles.flex3/flex2/flex1 instead of inline {flex: N} so column widths share one source of truth between header and row
- Move column header into FlatList ListHeaderComponent
- Use textLabelSupporting for column titles
- Replace per-row PERSONAL_DETAILS_LIST subscription with getPersonalDetailsByIDs
Filter archived/hidden rooms, derive room name from report attributes, and exclude hidden participants from the member count. Lift personal-details lookup to the page and pass per-row data into WorkspaceRoomsListItem. Switch FlatList to FlashList, disable the placeholder Create button, add the persistent row background, hover state, 8px row gap, and the column flex constants shared between the header and rows.
Tell useOnyx to invalidate its selector cache when policyID changes instead of relying on selector reference identity, matching the SuggestionMention pattern.
Use the existing styles.flex1/flex2/flex3 helpers directly in the header and row instead of wrapping them in a separate columnFlex module.
- Extract policy chat rooms selector to src/selectors/Report.ts and add unit tests - Extract list header into a dedicated ListHeader component - Add JSDoc to WorkspaceRoomsRow and WorkspaceRoomsListItemProps
|
Initial review took place on the fork PR: software-mansion-labs#338 |
|
@ikevin127 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] |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3c5d404a9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (!report || report.policyID !== policyID) { | ||
| continue; | ||
| } | ||
| if (!isChatRoom(report) && !isPolicyExpenseChat(report)) { |
There was a problem hiding this comment.
Because this selector explicitly admits isPolicyExpenseChat(report), any workspace member expense chat that is already in Onyx for this policy will be rendered as a row on the Workspace Rooms page and navigate like a room. Those chats are not rooms — the existing isChatRoom() helper is the repo's room predicate and it intentionally does not include policy expense chats (src/libs/ReportUtils.ts:1770). This will pollute the Rooms table for users who have opened policy expense chats in the workspace.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This isn't technically true. We only send the ones that are accessible by the user and we want to see the policy expense chats.
However, @dylanexpensify should admins/auditors viewing Rooms should see every member's expense chat listed?
There was a problem hiding this comment.
However, @dylanexpensify should admins/auditors viewing Rooms should see every member's expense chat listed?
IMO we should surface any rooms these users have access to, including expense chats. We can leverage filtering/sorting to make it more digestible for admins down the road. Thoughts?
| import CONST from '@src/CONST'; | ||
| import type {PersonalDetails, Report} from '@src/types/onyx'; | ||
|
|
||
| const ROW_MIN_HEIGHT = 64; |
There was a problem hiding this comment.
Is there not a norm CONST for rows? why is it hardcoded here?
There was a problem hiding this comment.
Good catch, I fixed it
|
🚧 @grgia has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
| onPress: () => void; | ||
| }; | ||
|
|
||
| function WorkspaceRoomsListItem({report, roomName, ownerDetails, ownerDisplayName, memberCount, onPress}: WorkspaceRoomsListItemProps) { |
There was a problem hiding this comment.
@sumo-slonik we should be reusing SelectionList/ Table pattern like the other pages. See WorkspaceMembersPage
We wouldn't need a new WorkspaceRoomsListItem component
There was a problem hiding this comment.
At first I implemented it the same as the workspace list (there’s no selection list there either), because I thought it looked quite similar to that screen, but I’ve already refactored the code so that it now uses a selection list here as well.
|
@grgia I was auto-assigned for C+ review, checking the parent issue looks like this PR is for @shubham1206agra to review - I'll unsubscribe from the issue, tag me in case my review is needed 🙌 
|
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
sumo-slonik
force-pushed
the
kuba-nowakowski/feature/workspace-rooms-list
branch
from
5d21048 to
d0be423
Compare
|
As for the mocks, I think it would give us something like this: |
|
Looks good! |
|
For the table, can we please try to follow the pattern from these (not yet merged) PRs? Mainly:
|
|
Stoked another table is getting migrated!! 🎉❤️ |
|
@sumo-slonik let me know if you have any questions about this, we can chat tmr in the office if you need! |
It is currently disabled because his functionality will only be added in the next PR, which is waiting for this one to be merged. |
Once I resolve all the comments, I’ll be happy to discuss it. Unfortunately, I’m out of Kraków this week. |
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx ts-node ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
|
@shawnborton @grgia Screen.Recording.2026-05-22.at.10.44.39.mov |
|
The gap between the avatar and the label should be 12px Can we also give Members column a fixed width since it should never be super long? Perhaps something like 80px. |
Of course, I’m taking care of it. |
|
It should already be fixed. Screen.Recording.2026-05-22.at.11.30.08.mov |
|
Members column seems too wide, and the spacing is uneven between these two |
|
Let me know when this is ready for a fresh build |
|
@shawnborton Do you have any other suggestions on what I should improve? 
|
JS00001
left a comment
JS00001
left a comment
There was a problem hiding this comment.
Table changes look good to me. A lot of your changes (The 'width' prop for columns, the 'label' prop for the searchbar, the additional styling for the search bar, etc) are similar to changes being made here: #90204
So you may need to fix conflicts if that PR merges before this one
I agree, in that case I’ll wait until that one is merged, resolve the conflicts with it, and then we’ll proceed with merging this |
|
Yup, visually that's looking good to me. For mobile, if we don't have a value for the Created by column, we should just omit that label entirely. |
|
@sumo-slonik Can we put a rush on this? As we want to finish this as soon as possible. |
@shubham1206agra From my side, I’m happy to do everything I can to get this done as quickly as possible, but as agreed in the message above with @JS00001 , we were supposed to wait until the second table migration is ready to avoid any conflicts. However, everything here seems to be ready, so maybe we can merge it sooner in that case. |
|
@sumo-slonik would you add HOLD and the link to the PR we're holding on to the title or description? |
sumo-slonik
changed the title
8 participants
Explanation of Change
Adds rows to the
WorkspaceRoomsPagetable. Behind theworkspaceRoomsPagebeta.Fixed Issues
$ #89596
Tests
workspaceRoomsPagebeta enabled.Offline tests
Unnecessary
QA Steps
Same as tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
MacOS: Chrome / Safari
Screen.Recording.2026-05-18.at.14.27.22.mov