Conversation
|
🚧 @rlinoz has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ae77a7db72
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@mkhutornyi 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] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8fa2f91180
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| didLayout.current = true; | ||
|
|
||
| markOpenReportEnd(report); | ||
| markOpenReportEnd(report, {warm: true}); |
There was a problem hiding this comment.
Do not end open-report span warm during skeleton mount
This onLayout handler now always closes ManualOpenReport with warm: true, but this same FlatListWithScrollKey is mounted even when showReportActionsLoadingState is true and the list is empty (it renders ReportActionsListLoadingSkeleton via ListEmptyComponent). In that loading state, layout can fire before the cold-marking effect and permanently close the span as warm, which misclassifies cold report opens in telemetry.
Useful? React with 👍 / 👎.
cc: @tgolen @luacmartins to make sure this is what you meant.
Explanation of Change
Starts finishing the
ManualOpenReportmetric with a newwarmtag. The definition is:warm:
ManualOpenReportfinished without needing to wait for an API callcold:
ManualOpenReportfinished when we displayed the loading skeleton, because we would need an API callFixed Issues
$ #83332
PROPOSAL:
Tests
https://expensify.sentry.io/explore/traces/?environment=adhoc&field=id&field=span.name&field=span.description&field=span.duration&field=transaction&field=timestamp&field=tags%5Bis_warm%2Cnumber%5D&project=4510228107427840&query=has%3Atags%5Bis_warm%2Cnumber%5D&statsPeriod=7d
Offline tests
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