Repository navigation
feat: policy distance rate page - #37717
Conversation
…icy-distance-rates-page
| }; | ||
|
|
||
| const editRate = () => { | ||
| // Navigation.navigate(ROUTES.WORKSPACE_EDIT_DISTANCE_RATE.getRoute(route.params.policyID, rateID)); |
There was a problem hiding this comment.
Yes, this page is mostly accessible by devs and QAs. This comment and other ones are indicators of what's to come in next PRs
…icy-distance-rates-page
| isVisible={isWarningModalVisible} | ||
| onClose={() => setIsWarningModalVisible(false)} | ||
| > | ||
| <View style={[styles.notSoFastPopoverWrapper]}> |
There was a problem hiding this comment.
Why is there such a specific style here? notSoFastPopoverWrapper seems like it's ultra specific, even though this modal is a pretty generic looking component.
There was a problem hiding this comment.
We can reuse existing ConfirmModal,this is what it'll look like. wdyt?
There was a problem hiding this comment.
That would be ideal, yup. Maybe we can change the copy slightly so we don't get an orphan either, cc @trjExpensify
There was a problem hiding this comment.
Swapped to ConfrimModal component and removed extra styles Ive added
Same question, the modal looks fine from my side. i assumed it's going to be handled later alongside the core functionalities. |
| <ButtonWithDropdownMenu<WorkspaceDistanceRatesBulkActionType> | ||
| shouldAlwaysShowDropdownMenu | ||
| pressOnEnter | ||
| customText={`${selectedDistanceRates.length} ${translate('workspace.distanceRates.selected')}`} |
There was a problem hiding this comment.
NAB: we can do something like this
Line 1818 in 7b51c3a
| isVisible={isWarningModalVisible} | ||
| onClose={() => setIsWarningModalVisible(false)} | ||
| > | ||
| <View style={[styles.notSoFastPopoverWrapper]}> |
There was a problem hiding this comment.
We can reuse existing ConfirmModal,this is what it'll look like. wdyt?
getusha
left a comment
getusha
left a comment
There was a problem hiding this comment.
LGTM & Works well other than the requests above.
|
Cool, @MrMuzyk let's get the comments above addressed and resolve conflicts. This is close! |
|
I think I've fixed all that had to be fixed now. You can have a look again :) |
|
@getusha are you available to continue your review here? |
luacmartins
left a comment
luacmartins
left a comment
There was a problem hiding this comment.
LGTM and tests well!
@luacmartins @MrMuzyk i think this is not fixed yet, is this going to be handled in the other PR? |
|
I think that's fine for now given that those actions don't actually work yet. Let's leave that for a follow up of when we actually implement that functionality! |
|
@luacmartins looks like this was merged without a test passing. Please add a note explaining why this was done and remove the |
|
there's a known issue with these tests https://expensify.slack.com/archives/C01GTK53T8Q/p1709911998098289 |
|
@MrMuzyk let's address this issue in the next PR:
|
|
✋ 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 production by https://github.com/luacmartins in version: 1.4.50-5 🚀
|
7 participants
Details
Created new page to display Distance Rates
Fixed Issues
$ #37510
PROPOSAL:
Tests
Same as QA steps
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodWaiting for Copylabel for a copy review on the original GH to get the correct copy.STYLE.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 so 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-native.mp4
Android: mWeb Chrome
android-chrome.mp4
iOS: Native
ios-native.mp4
iOS: mWeb Safari
ios-safari.mp4
MacOS: Chrome / Safari
web.mp4
MacOS: Desktop
desktop.mp4