Skip to content

Conversation

@armenzg
Copy link
Member

@armenzg armenzg commented Jun 4, 2024

No description provided.

@armenzg armenzg self-assigned this Jun 4, 2024
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jun 4, 2024
const {oneOtherIssueEvent} = useTraceTimelineEvents({
event,
});
const readyToShow = !isLoading && !isError;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This condition is not really necessary as you will see below.

{isRelatedIssuesEnabled && readyToShow && oneOtherIssueEvent === undefined && (
<TraceLink event={event} />
)}
{isRelatedIssuesEnabled && oneOtherIssueEvent && (
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If oneOtherIssueEvent is defined, we know that the data was fetched and that there was no error.

return (
<StyledDataSection>
<GroupEventCarousel group={group} event={event} projectSlug={project.slug} />
{isRelatedIssuesEnabled && readyToShow && oneOtherIssueEvent === undefined && (
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving this block closer to <TraceTimeline> since they would be shown together.

@armenzg armenzg marked this pull request as ready for review June 4, 2024 13:46
@armenzg armenzg requested a review from a team as a code owner June 4, 2024 13:46
@codecov
Copy link

codecov bot commented Jun 4, 2024

Bundle Report

Changes will decrease total bundle size by 37 bytes ⬇️

Bundle name Size Change
app-webpack-bundle-array-push 27.92MB 37 bytes ⬇️

@codecov
Copy link

codecov bot commented Jun 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.94%. Comparing base (0ef7972) to head (d82a1bb).
Report is 3 commits behind head on master.

Current head d82a1bb differs from pull request most recent head c0f4b30

Please upload reports for the commit c0f4b30 to get more accurate results.

Additional details and impacted files
@@             Coverage Diff             @@
##           master   #71999       +/-   ##
===========================================
+ Coverage   56.90%   77.94%   +21.04%     
===========================================
  Files        6562     6572       +10     
  Lines      292314   292600      +286     
  Branches    50471    50519       +48     
===========================================
+ Hits       166330   228057    +61727     
+ Misses     121241    58292    -62949     
- Partials     4743     6251     +1508     
Files Coverage Δ
...atic/app/views/issueDetails/groupEventCarousel.tsx 82.95% <ø> (ø)
static/app/views/issueDetails/groupEventHeader.tsx 100.00% <100.00%> (ø)

... and 1996 files with indirect coverage changes

@armenzg armenzg merged commit 2777216 into master Jun 4, 2024
@armenzg armenzg deleted the fix/related_issues/armenzg branch June 4, 2024 15:50
armenzg added a commit that referenced this pull request Jun 4, 2024
When fetching events, `oneOtherIssueEvent` is `undefined`, thus, we need
to wait until we're certain that the fetch has been completed to prevent
rendering twice or the wrong state.

I introduced this problem in #71999.
@github-actions github-actions bot locked and limited conversation to collaborators Jun 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants