Skip to content

ref(replay): Refactor Replay Details>Console tab to use the new *Frames types #53660

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 28, 2023

Conversation

ryan953
Copy link
Member

@ryan953 ryan953 commented Jul 26, 2023

Refactored to use the *Frame types.

Also, we are now also rendering custom breadcrumbs in the console tab! see screen shot and redux.action rows.

Note Before After
Console Breadcrumbs SCR-20230726-mcrc SCR-20230726-mcsc
Custom breadcrumbs n/a SCR-20230726-miiq

relates to #47991

@ryan953 ryan953 requested a review from a team as a code owner July 26, 2023 21:15
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jul 26, 2023
@@ -144,18 +127,35 @@ const ConsoleLog = styled('div')<{
`;

const ICONS = {
[BreadcrumbLevelType.ERROR]: <IconFire size="xs" />,
[BreadcrumbLevelType.WARNING]: <IconWarning size="xs" />,
[BreadcrumbLevelType.ERROR]: (
Copy link
Member

Choose a reason for hiding this comment

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

Why did we change the error icon from "fire" to "close"?

Copy link
Member

Choose a reason for hiding this comment

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

+1 curious about this too

Copy link
Member Author

Choose a reason for hiding this comment

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

So i was thinking that Sentry Errors (in the timeline and bredcrumbs, etc) are using the Fire icon already. And these rows are not errors, so it should be different. Also chrome console uses the x and tringle things:
SCR-20230728-khdj

Comment on lines -84 to -88
{breadcrumbHasIssue(breadcrumb) ? (
<IssueLinkWrapper>
<ViewIssueLink breadcrumb={breadcrumb} />
</IssueLinkWrapper>
) : null}
Copy link
Member

Choose a reason for hiding this comment

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

Do we no longer link to the issue now? I liked that link to differentiate the sentry error from console error.

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 was here from when we showed issues inside the console. But this is dead code now that we've moved issues/errors into the Errors tab.

@@ -144,18 +127,35 @@ const ConsoleLog = styled('div')<{
`;

const ICONS = {
[BreadcrumbLevelType.ERROR]: <IconFire size="xs" />,
[BreadcrumbLevelType.WARNING]: <IconWarning size="xs" />,
[BreadcrumbLevelType.ERROR]: (
Copy link
Member

Choose a reason for hiding this comment

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

+1 curious about this too

@ryan953 ryan953 enabled auto-merge (squash) July 28, 2023 20:15
@ryan953 ryan953 merged commit 28655d6 into master Jul 28, 2023
@ryan953 ryan953 deleted the ryan953/replay-ref-console-frames branch July 28, 2023 20:16
@github-actions github-actions bot locked and limited conversation to collaborators Aug 13, 2023
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