[Release 2] [Domain control] Add DomainMemberDetialsPage RHP#79358
Conversation
…page' into feature/kuba-nowakowski/add_domain_members_details_page # Conflicts: # src/languages/en.ts # src/libs/Navigation/AppNavigator/ModalStackNavigators/index.tsx # src/libs/Navigation/linkingConfig/RELATIONS/DOMAIN_TO_RHP.ts # src/libs/Navigation/types.ts # src/pages/domain/Members/DomainMembersPage.tsx
…feature/kuba-nowakowski/add_domain_members_details_page
…feature/kuba-nowakowski/add_domain_members_details_page # Conflicts: # src/pages/domain/Members/DomainMembersPage.tsx
…page' into feature/kuba-nowakowski/add_domain_members_details_page
…page' into feature/kuba-nowakowski/add_domain_members_details_page # Conflicts: # src/pages/domain/Admins/DomainAdminDetailsPage.tsx
…n_members_details_page
Code Review SummaryI've reviewed PR #79358 and identified several issues: Critical Issues1. Inconsistent import formatting in DomainAdminDetailsPage.tsx 2. Unused imports in DomainAdminDetailsPage.tsx 3. MembersDetailsPage.tsx - Incorrect prop usage 4. BaseDomainMemberDetailsComponent.tsx - Unused type export Recommendations
|
Additional Code Quality FindingsDomainAdminDetailsPage.tsxLine 1: Import formatting inconsistency - missing spaces in curly braces MembersDetailsPage.tsxLine 4: Unused import - MemberDetailsMenuItem type is imported but never used BaseDomainMemberDetailsComponent.tsxLine 32: Unused type definition - MemberDetailsMenuItem is defined but not used in component Suggested Fixes
|
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.
|
| const {translate, formatPhoneNumber} = useLocalize(); | ||
| const icons = useMemoizedLazyExpensifyIcons(['Info'] as const); | ||
|
|
||
| // eslint-disable-next-line rulesdir/no-inline-useOnyx-selector |
There was a problem hiding this comment.
For all eslint disablings, there should be comment explaining why.
Can we move selector function to the top of the file?
There was a problem hiding this comment.
no we can't, we need the accountID
There was a problem hiding this comment.
I'll add the comment
There was a problem hiding this comment.
I feel like we discussed this in the past and there was a way to achieve this somehow 🤔
There was a problem hiding this comment.
|
@situchan review addressed! |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppScreen.Recording.2026-01-13.at.6.57.17.PM.moviOS: mWeb SafariMacOS: Chrome / Safariweb.mov |
|
There are still failing checks |
src/libs/Navigation/AppNavigator/ModalStackNavigators/index.tsx
Outdated
Show resolved
Hide resolved
…n_members_details_page
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 951678c14e
ℹ️ 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".
|
Bug: member profile avatar briefly shows and disappears after opening profile page Screen.Recording.2026-01-13.at.6.47.28.PM.movEdit: maybe not bug because members list is sorted alphabetically. The order changed because "Hidden" name is replaced with real one. |
I think those two will be resolved once the backend changes are deployed |
|
Just checking, is this spelling error (
|
should be corrected 😄 |
mountiny
left a comment
There was a problem hiding this comment.
Thanks, non blocking comments
| const {translate, formatPhoneNumber} = useLocalize(); | ||
| const icons = useMemoizedLazyExpensifyIcons(['Info'] as const); | ||
|
|
||
| // eslint-disable-next-line rulesdir/no-inline-useOnyx-selector |
There was a problem hiding this comment.
I feel like we discussed this in the past and there was a way to achieve this somehow 🤔
| const {translate, formatPhoneNumber} = useLocalize(); | ||
| const icons = useMemoizedLazyExpensifyIcons(['Info'] as const); | ||
|
|
||
| // eslint-disable-next-line rulesdir/no-inline-useOnyx-selector |
There was a problem hiding this comment.
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.3.1-0 🚀
|
|
🚀 Deployed to production by https://github.com/francoisl in version: 9.3.1-1 🚀
|


Explanation of Change
This PR adds the member details RHP and unifies it with
DomainAdminDetailsPage(initial PR)
Fixed Issues
$ #78124
PROPOSAL:
Tests
/domain/<domainAccountID>/members/domain/<domainAccountID>/adminsOffline tests
QA Steps
/domain/<domainAccountID>/members/domain/<domainAccountID>/adminsPR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand 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
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Screen.Recording.2026-01-12.at.16.13.24.mov