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] |
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.
Exclude policy expense chats from the rooms list
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! 🧪🧪
|
|
@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.
|
5d21048 to
d0be423
Compare
|
It should already be fixed. Screen.Recording.2026-05-22.at.11.30.08.mov |
|
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
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? |
# Conflicts: # src/selectors/Report.ts
|
@shubham1206agra It’s all yours |
| tableMethods: {updateSearchString}, | ||
| } = useTableContext(); | ||
|
|
||
| const inputLabel = label ?? translate('workspace.companyCards.findCard'); |
There was a problem hiding this comment.
lets just require a label, we shouldnt fallback to 'find card'
| import Log from '@libs/Log'; | ||
|
|
||
| function openPolicyRoomsPage(policyID: string) { | ||
| if (!policyID) { |
There was a problem hiding this comment.
Is this an issue that you ran into? Is this guard necessary?
There was a problem hiding this comment.
Added just in case it'd be needed later, but I can drop it this is inside a workspace flow so policyID should always be set.
There was a problem hiding this comment.
It was just implemented the same way as in openWorkspaceRoomsPage
| [policyID], | ||
| ); | ||
|
|
||
| const rooms: WorkspaceRoomRowData[] = (policyReports ?? []) |
There was a problem hiding this comment.
would it be better to make this the selector instead of filtering later on?
There was a problem hiding this comment.
I'll move the it into policyChatRoomsSelector and update the tests
| import type {Report} from '@src/types/onyx'; | ||
|
|
||
| describe('policyChatRoomsSelector', () => { | ||
| const R = ONYXKEYS.COLLECTION.REPORT; |
There was a problem hiding this comment.
nab can we give this a meaningful name please?
@sumo-slonik Can you please address this? |
- Require label prop on TableSearchBar; drop the Find card fallback - Move archived filter into policyChatRoomsSelector and update tests - Drop redundant policyID guard in openPolicyRoomsPage - Rename const R to REPORT_KEY_PREFIX in ReportSelectorTest - Omit Created by label on narrow layout when owner is missing
I personally do! |
In that case, I’ll do it that way |
|
@shubham1206agra I think that all comments have already been resolved here |






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