[VIP-Travel] Create Trip Room Preview #38808
Conversation
src/CONST.ts
Outdated
| @@ -613,6 +614,7 @@ const CONST = { | |||
| LIMIT: 50, | |||
| // OldDot Actions render getMessage from Web-Expensify/lib/Report/Action PHP files via getMessageOfOldDotReportAction in ReportActionsUtils.ts | |||
| TYPE: { | |||
| ACTION_TRIPPREVIEW: 'ACTION_TRIPPREVIEW', | |||
There was a problem hiding this comment.
NAB
| ACTION_TRIPPREVIEW: 'ACTION_TRIPPREVIEW', | |
| ACTION_TRIP_PREVIEW: 'ACTION_TRIP_PREVIEW', |
There was a problem hiding this comment.
I'll adjust it once the backend is ready to be sure that this value is correct
|
|
||
| const dateInfo = DateUtils.getFormattedDateRange(new Date(basicTripInfo.startDate.iso8601), new Date(basicTripInfo.endDate.iso8601)); | ||
|
|
||
| const getDisplayAmount = (): string => { |
There was a problem hiding this comment.
I would use useMemo here.
| // pendingAction={iouReport?.pendingFields?.preview} | ||
| shouldDisableOpacity={!!(action.pendingAction ?? action.isOptimisticAction)} |
There was a problem hiding this comment.
Why the pendingAction is commented out here?
| onLongPress={(event) => showContextMenuForReport(event, contextMenuAnchor, chatReportID, action, checkIfContextMenuActive)} | ||
| shouldUseHapticsOnLongPress | ||
| style={[styles.flexRow, styles.justifyContentBetween, styles.reportPreviewBox, styles.cursorDefault]} | ||
| role="button" |
There was a problem hiding this comment.
| role="button" | |
| role={CONST.ROLE.BUTTON} |
|
|
||
| const reservations: Reservation[] = TripReservationUtils.getReservationsFromTripTransactions(tripTransactions); | ||
|
|
||
| const dateInfo = DateUtils.getFormattedDateRange(new Date(basicTripInfo.startDate.iso8601), new Date(basicTripInfo.endDate.iso8601)); |
There was a problem hiding this comment.
It might be worth putting into useMemo.
src/styles/index.ts
Outdated
| tripDescriptionMargin: { | ||
| marginBottom: 2, | ||
| }, |
There was a problem hiding this comment.
Is this value valid? We usually have multiples of 4.
NAB: Do we need to create a new style with only one value? Shouldn't we just use styles.mb1 or something like this?
There was a problem hiding this comment.
Following this design, I've created a new style where the gap value is 2
There was a problem hiding this comment.
I think I would be fine using a gap of 4px since that technically adheres to the system better. And then this way we don't need to write a super specific style.
src/types/onyx/Transaction.ts
Outdated
| @@ -209,6 +244,8 @@ type Transaction = OnyxCommon.OnyxValueWithOfflineFeedback< | |||
|
|
|||
| /** Indicates transaction loading */ | |||
| isLoading?: boolean; | |||
|
|
|||
|
In designs here is a
|
src/libs/DateUtils.ts
Outdated
| } | ||
| if (isSameMonth(date1, date2)) { | ||
| // Dates in the same month and year, differ by days | ||
| return `${format(date1, 'MMM d')} - ${format(date2, 'd')}`; |
There was a problem hiding this comment.
In designs, we don't have spaces between the days. We should confirm the other formats as well.
| return `${format(date1, 'MMM d')} - ${format(date2, 'd')}`; | |
| return `${format(date1, 'MMM d')}-${format(date2, 'd')}`; |
src/libs/ReportActionsUtils.ts
Outdated
| // @TODO: isTripRoomPreview condition has been added to make the TripRoomPreview component visible. | ||
| // Remove it when the reportAction.message array is not empty for this type, then !isDeleted will be true. |
There was a problem hiding this comment.
I think that it should always be visible
Done :) |
|
@WojtekBoman is it possible for our team to QA this? I noticed that section is not filled out |
|
|
@shubham1206agra could you please take a quick look / retest since there's been commits since your final review |
|
@grgia You are good to merge here. |
|
✋ 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/grgia in version: 1.4.86-0 🚀
|
1 similar comment
|
🚀 Deployed to staging by https://github.com/grgia in version: 1.4.86-0 🚀
|
|
🚀 Deployed to staging by https://github.com/grgia in version: 1.4.86-0 🚀
|
|
@grgia The QA team is encountering issues with this PR. Should we log the issue for the error message? Recording.6251.mp4 |
|
@AndrewGable @stitesExpensify we tried with travel beta account and see the same issue we had in this PR Recording.1999.mp4 |
|
🚀 Deployed to production by https://github.com/AndrewGable in version: 9.0.0-9 🚀
|
Details
This PR introduces a new component
TripRoomPreview. It's available for testing only for accounts with thespotnanaTravelbetaDesigns: https://www.figma.com/file/rOyUSHbLLyp64emz6WeoPW/Travel?type=design&node-id=154-14373&mode=design&t=xqcJ31MMIiDxsY0J-4
Fixed Issues
$ #37826
PROPOSAL: N/A
Tests
If some data is not loaded, please open the linked report first, this problem will be fixed on the backend side.
Offline tests
QA Steps
Same as tests
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
Screen.Recording.2024-05-29.at.17.35.27.mov
Android: mWeb Chrome
Screen.Recording.2024-05-29.at.17.49.04.mov
iOS: Native
Screen.Recording.2024-05-29.at.17.58.26.mov
iOS: mWeb Safari
Screen.Recording.2024-05-29.at.18.03.22.mov
MacOS: Chrome / Safari
Screen.Recording.2024-05-29.at.17.36.32.mov
MacOS: Desktop
Screen.Recording.2024-05-29.at.17.29.22.mov