[No QA] Add reasonAttributes to ActivityIndicator in image-loading contexts#82927
Conversation
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
@bernhardoj 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] |
| context: 'PDFView', | ||
| shouldRequestPassword, | ||
| isPasswordInvalid, | ||
| shouldAttemptPDFLoad, |
There was a problem hiding this comment.
Looks like these are unrelated to why the loading shows
There was a problem hiding this comment.
Yeah, you're right.
But in this case we don't have specific prop to tell why the component did render. While adding these I just thought it would add some context, like "User did something and then loading stuck"
heyjennahay
left a comment
There was a problem hiding this comment.
Code change only. Product approval not required
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safariw.mp4 |
|
🚀 Deployed to staging by https://github.com/youssef-lr in version: 9.3.26-0 🚀
|
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.3.26-8 🚀
|
Explanation of Change
Wires
reasonAttributesthrough toActivityIndicatorin eight image-loading contexts so that Sentry skeleton spans carry structured metadata about why the loading indicator is visible. This is the first proof-of-concept batch for issue #81683, covering:Lightbox,ReceiptPreview,AvatarCropModal,PlaidCardFeedIcon,HeaderWithBackButton(download + rotate),LoadingIndicator(prop passthrough),PDFView, andPDFThumbnail.Each usage passes a
contextstring identifying the component, plus relevant boolean state variables (e.g.isImageLoaded,isLoading,isDownloading) so engineers can distinguish long-running spans in Sentry and identify infinite-skeleton root causes.Fixed Issues
$ #81683, #79653
PROPOSAL:
Tests
Offline tests
N/A
QA Steps
Same as tests
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))npm run compress-svg)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