ref(events): Migrate ContextIcon to platformicons#115701
ref(events): Migrate ContextIcon to platformicons#115701priscilawebdev wants to merge 9 commits into
Conversation
Replace the legacy sentry-logos mapping behind ContextIcon with platformicons while keeping the existing component API intact. Add coverage for alias translation and unknown-icon fallback behavior so follow-up cleanup can safely remove the old asset dependency. Refs DE-1262 Co-Authored-By: Codex <codex@openai.com>
github-actions
Bot
added
the
Scope: Frontend
📊 Type Coverage Diff✅ No new type safety issues introduced. Coverage: 93.53% |
Drop the 38 sentry-logos assets only referenced by the legacy ContextIcon mapping and inline the helper logic now that platformicons resolves names directly.
Preserves the prior worker-specific brand from logoCloudflareWorker.
…are" This reverts commit 6e02313.
- Return null from getLogoImage instead of a logoUnknown URL sentinel, so callers compare against null rather than a build-produced string. - Pass format=lg to PlatformIcon so shaped device icons (iPhone, iPad, Apple TV/Watch, Android phone/tablet, Switch) render their device outline instead of just the brand glyph. - Map google -> google instead of android, restoring the Google logo for Pixel devices. - Bump platformicons to ^9.5.0. Refs DE-1262
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 920b101. Configure here.
|
|
||
| return <img height={size} width={size} css={extraCass} src={imageName} />; | ||
| return ( | ||
| <PlatformIcon platform={platformIconName ?? 'default'} size={size} format="lg" /> |
There was a problem hiding this comment.
String size value passed where number expected
High Severity
SvgIcon.ICON_SIZES[providedSize] returns a string like '32px', but PlatformIcon's size prop expects a number. Every other PlatformIcon usage in the codebase passes a numeric value (e.g. size={16}, size={32}). The old code passed this string to an <img> tag's height/width which tolerates strings, but PlatformIcon will not handle '32px' correctly as a numeric size. The value needs to be parsed to a number first (similar to how utils.tsx uses parseInt(iconSize, 10) for UserAvatar).
Reviewed by Cursor Bugbot for commit 920b101. Configure here.
There was a problem hiding this comment.
not a real bug...it accepts strings too
priscilawebdev
changed the base branch from
master
to
priscila/chore/bump-platformicons-9.5.0
…ebdev/ref/de-1262-migrate-contexticon-to-platformicons
…icon-to-platformicons
1 participant
Migrate
ContextIconto renderPlatformIconinstead of the legacysentry-logosasset map.This keeps the existing
ContextIconAPI in place for current callers whilemoving the implementation onto
platformiconsand translating the oldcontext icon aliases to supported platform ids. It also preserves the
existing unknown-icon sentinel behavior that event context consumers use to
decide when not to render an icon.
closes https://linear.app/getsentry/issue/DE-1262/migrate-contexticon-to-platformicons