fix: fallback avatar shown in reports page when RHP is open and page is not in focus#67470
fix: fallback avatar shown in reports page when RHP is open and page is not in focus#67470Julesssss merged 2 commits intoExpensify:mainfrom
Conversation
|
I have added screen recording of 3 platforms (android mWeb, IOS mWeb and MacOS chrome) where page refresh is possible. Moreover, this issue is only visible in MacOS chrome as RHP for other 2 platforms covers the whole screen. For the other 3 native platforms, page refresh/reload is not possible, so couldn't add screen recordings for them. @c3024 |
| // Trigger once on mount (e.g., on page reload), when RHP is open and screen is not focused | ||
| useEffect(() => { | ||
| handleFocusOrMount(); | ||
| // eslint-disable-next-line |
There was a problem hiding this comment.
Let us specify the exact lint rule disabled here and move the comment inside.
| // Trigger once on mount (e.g., on page reload), when RHP is open and screen is not focused | |
| useEffect(() => { | |
| handleFocusOrMount(); | |
| // eslint-disable-next-line | |
| useEffect(() => { | |
| clearTransactionsAndSetHashAndKey(); | |
| // Trigger once on mount (e.g., on page reload when RHP is open and screen is not focused) | |
| // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps |
| setCurrentSearchHashAndKey(hash, searchKey); | ||
| }, [hash, searchKey, clearSelectedTransactions, setCurrentSearchHashAndKey]), | ||
| ); | ||
| const handleFocusOrMount = useCallback(() => { |
There was a problem hiding this comment.
| const handleFocusOrMount = useCallback(() => { | |
| const clearTransactionsAndSetHashAndKey = useCallback(() => { |
Lets us change this function name to what it does like the PR author checklist suggests:
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)
|
@c3024 I have fixed the PR comments. Could you pls review once you are free. |
|
This issue is preventing me from testing the PR. I will complete review once that is resolved. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridApprefreshAndroid.movAndroid: mWeb ChromerefreshAndroidmWeb1.movrefreshAndroidmWeb.moviOS: HybridApprefreshiOS.moviOS: mWeb SafarirefreshiOSmWeb.movMacOS: Chrome / SafarirefreshChrome.movMacOS: DesktoprefreshDesktop.mov |
|
🚀 Deployed to staging by https://github.com/Julesssss in version: 9.1.89-1 🚀
|
|
🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.1.89-21 🚀
|
Explanation of Change
Fixed Issues
$ #67039
PROPOSAL: $ #67039 (comment)
Tests
Offline tests
Same as tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand 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
Android: mWeb Chrome
android.web.mp4
iOS: Native
iOS: mWeb Safari
ios.web.mp4
MacOS: Chrome / Safari
web.chrome.desktop.mp4
MacOS: Desktop