[TS migration] Migrate ProfileAvatarWithIndicator.js component to TypeScript#40090
Conversation
fabioh8010
left a comment
There was a problem hiding this comment.
@kubabutkiewicz LGTM! Please include test steps and screenshots!
|
@getusha could you prioritize a review here? We would like to get this merged until tomorrow as this is one of the last TS PRs remaining! |
|
Switched reviewers per https://expensify.slack.com/archives/C02NK2DQWUX/p1712864609143049 |
Reviewer Checklist
Screenshots/VideosAndroid: NativeScreen.Recording.2024-04-12.at.1.38.10.AM.movAndroid: mWeb ChromeScreen.Recording.2024-04-12.at.1.25.38.AM.moviOS: NativeScreen.Recording.2024-04-12.at.1.22.46.AM.moviOS: mWeb SafariScreen.Recording.2024-04-12.at.1.22.06.AM.movMacOS: Chrome / SafariScreen.Recording.2024-04-12.at.1.19.07.AM.movMacOS: DesktopScreen.Recording.2024-04-12.at.1.20.42.AM.mov |
Sorry guys, i thought i replied to this. 😄 🙏 |
allroundexperts
left a comment
There was a problem hiding this comment.
Looks good. One minor comment, but I think we haven't enforced the useOnyx yet, so depending on the priority, we can let it go.
|
It's cool that rory got assigned here. He actually would be proposing the |
|
@fabioh8010 @allroundexperts @roryabraham replaced |
| const [isLoadingOnyxValue] = useOnyx(ONYXKEYS.IS_LOADING_APP); | ||
| const isLoading = isLoadingOnyxValue ?? true; |
There was a problem hiding this comment.
Note: This piece of code mimics the old behavior (withOnyx + default value assigment to isLoading prop).
…s-migration/ProfileAvatarWithIndicator
|
🚀 Deployed to staging by https://github.com/roryabraham in version: 1.4.63-0 🚀
|
|
🚀 Deployed to production by https://github.com/mountiny in version: 1.4.63-21 🚀
|
Details
Fixed Issues
$ #40089
Tests
Offline tests
QA Steps
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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop