feat: Add Vacation delegate view for the delegate #78432
Conversation
|
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 |
melvin-bot
bot
requested review from
JmillsExpensify,
flaviadefaria,
heyjennahay,
joekaufmanexpensify and
trjExpensify
There was a problem hiding this comment.
Pull request overview
This PR adds a vacation delegate view for users who are currently acting as delegates for other members. When a user is set as a vacation delegate by others, they cannot set their own vacation delegate, and instead see a list of all members who have delegated to them.
Key Changes:
- Added
delegatorForfield toVacationDelegatetype to track users delegating to the current user - Implemented conditional rendering in StatusPage to show delegator list when user has active delegations
- Added translations for the "cannot set vacation delegate" message across all supported languages
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/types/onyx/VacationDelegate.ts | Added delegatorFor field to track array of delegator emails |
| src/pages/settings/Profile/CustomStatus/StatusPage.tsx | Added conditional UI to display delegators list or vacation delegate settings based on delegation status |
| src/languages/en.ts | Added English translation for "cannot set vacation delegate" message |
| src/languages/es.ts | Added Spanish translation |
| src/languages/de.ts | Added German translation |
| src/languages/fr.ts | Added French translation |
| src/languages/it.ts | Added Italian translation |
| src/languages/ja.ts | Added Japanese translation |
| src/languages/nl.ts | Added Dutch translation |
| src/languages/pl.ts | Added Polish translation |
| src/languages/pt-BR.ts | Added Portuguese (Brazil) translation |
| src/languages/zh-hans.ts | Added Chinese (Simplified) translation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
Weird, Melvin bot added many reviewers |
@samranahm You missed the link to the Slack message in the checklist |
|
@samranahm Please fill in the QA steps as well |
| {hasActiveDelegations ? ( | ||
| <View> | ||
| <Text style={[styles.mh5, styles.mb4]}>{translate('statusPage.cannotSetVacationDelegate')}</Text> | ||
| {vacationDelegate?.delegatorFor?.map((delegatorEmail) => { |
There was a problem hiding this comment.
I suggest refactoring this block into renderDelegatorList() as shown, adding a null guard and simplifying variable names for readability.
Example:
const renderDelegatorList = () => {
if (!vacationDelegate?.delegatorFor) {
return null;
}
return vacationDelegate.delegatorFor.map((delegatorEmail) => {
const delegatorDetails = getPersonalDetailByEmail(delegatorEmail);
const formattedLogin = formatPhoneNumber(delegatorDetails?.login ?? '');
const displayLogin = formattedLogin || delegatorEmail;
return (
<MenuItem
key={delegatorEmail}
title={delegatorDetails?.displayName ?? displayLogin}
description={displayLogin}
avatarID={delegatorDetails?.accountID ?? CONST.DEFAULT_NUMBER_ID}
icon={delegatorDetails?.avatar ?? Expensicons.FallbackAvatar}
iconType={CONST.ICON_TYPE_AVATAR}
numberOfLinesDescription={1}
containerStyle={[styles.pr2, styles.mt1]}
interactive={false}
/>
);
});
};
....
{renderDelegatorList()}
There was a problem hiding this comment.
Yes, These changes indeed make the component cleaner and more maintainable. I’ll refactor it accordingly.
|
@huult Please take a look. |
2 participants
Explanation of Change
We're adding the Vacation Delegate view for delegates on the Status page.
Fixed Issues
$ #78037
PROPOSAL: #78037 (comment)
Tests
Precondition: User A is not a delegate for anyone.
Steps
Offline tests
QA Steps
// 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.Will add the Screenshots/Videos of remaining platforms once the BE update deployed.
Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS.Chrome.mp4