Show reportID in report details panel#57942
Conversation
|
@QichenZhu 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] |
| filterLogs: 'Registros de filtrado', | ||
| network: 'La red', | ||
| reportID: 'ID del informe', | ||
| longID: 'ID largo', |
There was a problem hiding this comment.
Is the translation confirmed? Do you need me to post a message in the Slack channel?
There was a problem hiding this comment.
That would be great as I haven't had access to Slack yet. Thank you.
Reviewer Checklist
Screenshots/VideosAndroid: Nativeandroid-native.mp4Android: mWeb Chromeandroid-web.mp4iOS: Nativeios-native.mp4iOS: mWeb Safariios-web.mp4MacOS: Chrome / Safarimac-web.movMacOS: Desktopmac-desktop.mov |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Just updated.
|
Fixed this by disabling text selection on mobile chrome. |
|
Thanks for the updates. |
|
I’m seeing different feedback after a tap/click across platforms: We’re not intentionally introducing these behaviors, and I’m not sure if it’s technically possible to make them consistent across platforms. I’d like to check with @Expensify/design to see if any of these feedback types look problematic. |
I believe these are platforms specific behaviors and we shouldn't mess with them. As long as the text is copied, I think that's fine. Can we confirm that on iOS browser and on desktop that the text is indeed copied? |
Yeah I also agree with this point. I think there's not so much we can do about it. |
Thanks for your insight!
Yes, it is. Verified by pasting elsewhere. |
|
Thanks for confirming. cc @Expensify/design for visibility. Otherwise I say we move on 👍 |
| import type {TextProps} from './Text'; | ||
|
|
||
| type TextWithCopyProps = TextProps & { | ||
| copyText: string; |
There was a problem hiding this comment.
Just a few suggestions to meet the coding standards:
- Add a
/** block comment */above thecopyTextprop. - Add a comment to describe what
TextWithCopydoes, given that we already haveCopyTextToClipboardandPressableWithDelayToggle. - Add a comment to explain why we disable selection in Android Chrome.
- Remove
textRefif it’s not used. We can add it back if needed in the future. - Don't write
getBase62ReportID(Number(report.reportID))and[styles.flexRow, styles.alignItemsCenter]more than once.
Let me know if you disagree. Thanks for your effort on this, @mkzie2!
Agree with that! |
|
I agree as well! Let's keep movin' |
|
@Expensify/design 🫣 don't murder me here, but since there is absolutely zero indication of any interaction here... Do you all think it would be better to open a popover with a |
Thank you! Reported in Slack: Context menu slides in from edge of window |
|
Updated to not show the pointer cursor: Screen.Recording.2025-03-27.at.23.02.37.mov |
|
Looks good to me 👍 |
|
Same 👍 |
|
Me three 👍 |
| const popoverAnchor = useRef<View>(null); | ||
| const styles = useThemeStyles(); | ||
|
|
||
| const toggleCopyContextMenu = (event: GestureResponderEvent | MouseEvent) => { |
There was a problem hiding this comment.
The word "toggle" is misleading. Please rename the function to reflect its actual behavior.
There was a problem hiding this comment.
Excerpt from PR Author Checklist:
I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
I think toggle here is fine. But I updated it to open anyway.
|
@mkzie2 Could you please change "press" to "right click or long press" in the test steps and re-upload the broken videos/screenshots? Thanks! |
Updated |
Thanks for the update, but the QA steps are still not quite right.
|
|
Oops I'm sorry 🙇 Updated |
|
@rafecolton Could you or someone from the team verify the Spanish translation? Thanks! |
rafecolton
left a comment
There was a problem hiding this comment.
LGTM. Asked here (internal only) for confirmation on the translation, will merge once received
|
Translation confirmed |
|
✋ 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/rafecolton in version: 9.1.24-2 🚀
|
|
🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.1.24-10 🚀
|
1 similar comment
|
🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.1.24-10 🚀
|
|
Coming from issue #60668 where the expense report identification footer is displayed in the details pane of a group chat, we decided that these numbers should only be shown on expense and invoice reports. |



Explanation of Change
Fixed Issues
$ #57722
PROPOSAL: #57722 (comment)
Tests
Offline tests
None
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)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.2025-03-26.at.14.29.42.mov
Android: mWeb Chrome
Screen.Recording.2025-03-31.at.18.36.04.mov
iOS: Native
Screen.Recording.2025-03-31.at.18.33.46.mov
iOS: mWeb Safari
Screen.Recording.2025-03-31.at.18.34.21.mov
MacOS: Chrome / Safari
Screen.Recording.2025-03-31.at.18.32.56.mov
MacOS: Desktop
Screen.Recording.2025-03-31.at.18.32.31.mov