#45951 & #45958: Implement <ApprovalWorkflowSection /> component and Enhance toLocaleOrdinal to support string ordinals#46562
Conversation
…e-mansion-labs/expensify-app-fork into Guccio163/45951_implementApprovalWorkflowSection
…-fork into Guccio163/45951_implementApprovalWorkflowSection
…1_implementApprovalWorkflowSection
…-fork into Guccio163/45951_implementApprovalWorkflowSection
|
PR is ready for review from C+; I'm OOO until the end of the week, so any change before that will be handled by @blazejkustra, good luck!👋 |
|
@shubham1206agra 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] |
|
@blazejkustra @Guccio163 One question. Does #45892 help improve this PR in some way? |
|
@shubham1206agra function from #45892 is directly used by this PR, we decided to link them together to speed up the process and not to block the #45951 |
|
@shubham1206agra I merged the newest main for you 🚀 |
|
@blazejkustra I am getting this error on toggling Approval section. |
|
|
||
| {approvalWorkflow.approvers.map((approver, index) => ( | ||
| <View key={approver.email}> | ||
| <View style={{height: 16, width: 1, backgroundColor: theme.border, marginLeft: 19}} /> |
There was a problem hiding this comment.
@blazejkustra Can you please explain me the choice of style here (especially marginLeft)? And can we use styles here.
There was a problem hiding this comment.
marginLeft is set to 19 to match the designs @shubham1206agra:

Is this connected to this PR? 🤔 |
|
This is generally feeling good. cc @Expensify/design - I added you both to the beta in case you want to test the adhoc build above. You might need to clear cache from Settings > Troubleshoot. Just confirming, do we want to truncate after two lines here? cc @JmillsExpensify |
The screen you are seeing is still the old Approver screen, meaning it will be handled in a future PR. Although, I thought it should be highligthed only when hovering over the item like in other selection lists in the app: Screen.Recording.2024-08-02.at.20.05.48.movIs using a selected BG color for the selected approver a novelty in the app or is it used somewhere? @shawnborton |
…-fork into Guccio163/45951_implementApprovalWorkflowSection
Reviewer Checklist
Screenshots/Videos |
| import Text from './Text'; | ||
|
|
||
| type ApprovalWorkflowSectionProps = { | ||
| approvalWorkflow: ApprovalWorkflow; |
There was a problem hiding this comment.
Please add /** */ style comments to each prop
There was a problem hiding this comment.
Just added, please take a quick look if they're okay :)
|
✋ 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/tgolen in version: 9.0.17-0 🚀
|
|
🚀 Deployed to production by https://github.com/marcaaron in version: 9.0.17-2 🚀
|
2 similar comments
|
🚀 Deployed to production by https://github.com/marcaaron in version: 9.0.17-2 🚀
|
|
🚀 Deployed to production by https://github.com/marcaaron in version: 9.0.17-2 🚀
|
| // eslint-disable-next-line react/no-array-index-key | ||
| <View key={`approver-${approver.email}-${index}`}> | ||
| <View style={styles.workflowApprovalVerticalLine} /> | ||
| <MenuItem |
There was a problem hiding this comment.
We didn't add numberOfLines which led to the email content overflowing to the next line.
| title={translate('workflowsExpensesFromPage.title')} | ||
| style={styles.p0} | ||
| titleStyle={styles.textLabelSupportingNormal} | ||
| descriptionTextStyle={styles.textNormalThemeText} |
There was a problem hiding this comment.
[BZ checklist] coming from #63121, line height for descriptionTextStyle is not accurate which caused this minor ui issue










Details
This PR adds a view of more than the default approval workflows; The buttons should be presable, but their functionalities are yet to be implemented. This PR is UI-focused.
UI with enabled

canUseWorkflowsAdvancedApprovalshould look like this:Meanwhile, account without

canUseWorkflowsAdvancedApprovalpermission should remain unchanged:Fixed Issues
$ #45951
$ #45958
PROPOSAL:
Tests
Right now, canUseWorkflowsAdvancedApproval permission is switched off, to see the new layout, make function
src/libs/Permissions.tsx/canUseWorkflowsAdvancedApprovalreturntrue.Open
WorkspaceWorkflowPage.tsxand check if page looks like this:Check if every section redirects to
ROUTES.WORKSPACE_WORKFLOWS_APPROVALS_EDITAdd approvals button's update involved only UI, it's functionality will be added in [CRITICAL] [Advanced Approval Workflows] Implement <WorkspaceWorkflowsApprovalsCreatePage /> for Workflow Creation #45954
Offline tests
N/A
QA Steps
N/A
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))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.mov
Android: mWeb Chrome
mWeb-android.mov
iOS: Native
ios.mov
iOS: mWeb Safari
mWeb-ios.mov
MacOS: Chrome / Safari
web.mov
MacOS: Desktop
desktop.mov