-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Enable copy action for travel details #58246
base: main
Are you sure you want to change the base?
Conversation
@dannymcclain @ntdiary One of you needs to 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] |
I think the videos are looking pretty good, but I'm having a hard time seeing the whole flow because the videos are pretty tightly cropped (time-wise). Would you mind uploading videos that show the entire flow more easily? Some of these desktop options are looking pretty narrow - @Expensify/design do we have a minimum width we usually set for these? Lastly, we definitely don't want the menu item to change width once it changes from "Copy to clipboard" to "Copied". |
Video for full flow: Screen.Recording.2025-03-12.at.07.22.03-recording.mov |
@ntdiary Do you have any hint about retaining popover width even when the inner content changes? For report action context menu, because there're other menu items with larger width, when |
Totally agree that we should have a min-width. When I made the component, the smallest one I found was |
I don't know that we've established one - so I say let's make a new rule! What if we do something like 340px since it's divisible by 4? :) But I definitely agree that having a min-width here would be nice to avoid the size jumping. |
Explanation of Change
Enable copy feature for travel details:
Fixed Issues
$ #57800
PROPOSAL: #57800 (comment)
Tests
Copy to clipboard
item showsCopy to clipboard
Offline tests
None
QA Steps
Copy to clipboard
item showsCopy to clipboard
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
Screen.Recording.2025-03-12.at.02.11.01.mov
iOS: Native
Screen.Recording.2025-03-12.at.02.06.55.mov
iOS: mWeb Safari
Screen.Recording.2025-03-12.at.02.08.29.mov
MacOS: Chrome / Safari
Screen.Recording.2025-03-12.at.07.22.03-recording.mov
MacOS: Desktop
Screen.Recording.2025-03-12.at.02.07.32.mov