Skip to content

ref(dashboards): Clean up RevisionListItem logic and naming#114161

Merged
skaasten merged 6 commits intomasterfrom
shaunkaasten/dain-1583-revisionlistitemjsx-cleanups
Apr 28, 2026
Merged

ref(dashboards): Clean up RevisionListItem logic and naming#114161
skaasten merged 6 commits intomasterfrom
shaunkaasten/dain-1583-revisionlistitemjsx-cleanups

Conversation

@skaasten
Copy link
Copy Markdown
Contributor

@skaasten skaasten commented Apr 28, 2026

Small readability refactors to RevisionListItem and typeSelector — no behavior changes.

  • Add mappers and a new component function for rendering different widget diff statuses
  • rename of the widget display map constant

skaasten and others added 4 commits April 28, 2026 10:14
…up in WidgetDiffCard

Collapse two separate if/else chains for statusLabel and tagVariant into a single STATUS_MAP record. The map is exhaustive by type, so adding a new status value will surface a TypeScript error.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…hain

Replace a 4-level ternary in RevisionListItem with a dedicated RevisionDiffBody
component that uses early returns. Also pre-computes isError from the two
separate error conditions so the inline compound expression is gone.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Export RevisionDiffBody so it can be rendered directly in stories. Add
loading and error state stories that were previously impossible to show
without mocking hooks.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Rename to follow ALL_CAPS convention for module-level constants and make
the connection to the DisplayType enum explicit.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@linear-code
Copy link
Copy Markdown

linear-code Bot commented Apr 28, 2026

@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Apr 28, 2026
@skaasten skaasten marked this pull request as ready for review April 28, 2026 14:38
@skaasten skaasten requested a review from a team as a code owner April 28, 2026 14:38
Copy link
Copy Markdown
Contributor

@DominikB2014 DominikB2014 left a comment

Choose a reason for hiding this comment

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

🧹 nice cleanup, easier to read/follow imo

Comment on lines +262 to +264
added: {label: t('Added'), variant: 'success'},
removed: {label: t('Removed'), variant: 'danger'},
modified: {label: t('Modified'), variant: 'warning'},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There should be a type we can reuse for variant?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Found it - thanks!

isError: boolean;
snapshot: DashboardDetails | undefined;
}) {
if (isAnyLoading) return <LoadingIndicator />;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should we just do isLoading as the prop? When your in the context of this component, i don't think the keyword Any is relevant right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yep, that's better 👍

Rename isAnyLoading prop to isLoading on RevisionDiffBody — the Any
qualifier is not meaningful from the component's perspective. Use TagVariant
from sentry/utils/theme instead of an inline union for the STATUS_MAP variant
field.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@skaasten skaasten merged commit ac1dbe7 into master Apr 28, 2026
65 checks passed
@skaasten skaasten deleted the shaunkaasten/dain-1583-revisionlistitemjsx-cleanups branch April 28, 2026 15:16
cleptric pushed a commit that referenced this pull request May 5, 2026
Small readability refactors to `RevisionListItem` and `typeSelector` —
no behavior changes.

* Add mappers and a new component function for rendering different
widget diff statuses
* rename of the widget display map constant

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants