-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[CP-stag] Removing admins title from payers screen #80933
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
Conversation
Code Review SummaryI've reviewed PR #80933. This is a straightforward change that removes the "Admins" section title from the payers screen. ✅ Overall AssessmentThe change looks good and accomplishes the stated goal. The removal of the section title is clean and minimal. 💡 Minor SuggestionsWhile not blocking, you may want to consider:
The code change itself is correct and complete. Nice work keeping the change minimal and focused! ✨ |
|
@chiragsalian Does this require C+ review? I'm available to review now. |
|
sure @akinwale, feel free to review. If possible can you check if this change breaks anything else. Im currently trying to confirm the same. I dont think it does so far. |
|
oh i think we need the title if there are multiple admins 👀 |
|
Tweaked the code a little to show "Admins" title if there are multiple admins. |
Reviewer Checklist
Screenshots/Videos |
akinwale
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
[CP-stag] Removing admins title from payers screen (cherry picked from commit 83b573d) (cherry-picked to staging by francoisl)
|
🚀 Cherry-picked to staging by https://github.com/francoisl in version: 9.3.11-10 🚀
|
|
🚀 Cherry-picked to staging by https://github.com/francoisl in version: 9.3.11-13 🚀
|
|
🚀 Cherry-picked to staging by https://github.com/francoisl in version: 9.3.11-16 🚀
|






Explanation of Change
Before,

with no admins
now,

With no admins
With admins

Fixed Issues
$ #80856
PROPOSAL:
Tests
Offline tests
QA Steps
Same as #80856
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR 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