Skip to content

feat: use the shop c mark as the credits icon - #464

Merged
juanmahidalgo merged 2 commits into
masterfrom
feat/credits-icon-shop-c
Aug 5, 2026
Merged

feat: use the shop c mark as the credits icon#464
juanmahidalgo merged 2 commits into
masterfrom
feat/credits-icon-shop-c

Conversation

@juanmahidalgo

Copy link
Copy Markdown
Contributor

The credits balance still rendered the old glyph — a multi-layer gradient bloom. The current mark for credits is the hexagonal C, which the shop already ships.

Changes

  • Icon/CreditsIcon now draws the hexagonal C, in currentColor.
  • Removed NavbarCreditsIcon from Navbar/icons.tsx — the old bloom, dead code (not referenced anywhere, and Navbar/icons is not exported from the package). Leaving it in would keep the retired glyph one import away.

Because CreditsIcon is shared, this updates every surface where the glyph stands for the currency in one go: the navbar credits chip, the user menu balances, and CreditsToggle.

Notes

currentColor instead of a baked palette. The old mark carried its own gradients and so looked the same on every surface it landed on; the C inherits the text colour of whatever chip it sits in — lavender (#A0ABFF) in the navbar credits chip, off-white in the shop credits chip. That is what lets one glyph serve both without a per-surface variant.

The notification illustrations are untouched. Icon/Notifications/CreditsIcon is a separate 48px artwork used by the credits notification rows, not a currency mark.

The creditsBalance and shopCreditsBalance chips now share the glyph, distinguished by colour. No consumer passes both at once today (the marketplace passes creditsBalance, the shop passes shopCreditsBalance), and they are one currency, so this is intended rather than a collision to design around.

Test plan

  • npm run format, npm run lint, npm run lint:package-json, npm run build, npm test (37 passing)
  • Check the mark in Storybook (Icons stories) at default and fontSize="large"
  • Confirm the navbar chip, the user menu and the credits toggle all pick up the C

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Aug 5, 2026

Copy link
Copy Markdown

Deploying ui2 with  Cloudflare Pages  Cloudflare Pages

Latest commit: cc4fb20
Status: ✅  Deploy successful!
Preview URL: https://f73562ce.ui2-423.pages.dev
Branch Preview URL: https://feat-credits-icon-shop-c.ui2-423.pages.dev

View logs

@decentraland-bot decentraland-bot left a comment

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.

Review — PR #464

Verdict: Approve

Clean, well-scoped change. The old multi-layer gradient bloom is replaced with the hexagonal C badge that the shop already uses, and the dead NavbarCreditsIcon is removed. No P0 or P1 issues.

What I checked

  • Correctness of the new CreditsIconviewBox="0 0 20 21" is placed before {...props}, following MUI convention (set a default, let the consumer override). The explicit fill="none" on the hexagon path is necessary because MUI's SvgIcon applies fill: currentColor via CSS on the root <svg> — without it the hexagon would render as a solid shape instead of a stroked outline. The C letter path correctly uses fill="currentColor". Both paths match the existing ShopCreditsIcon artwork.

  • Dead-code removal is safeNavbarCreditsIcon was defined and exported from icons.tsx but never imported by Navbar.tsx, never re-exported from Navbar/index.ts, and never listed in the package barrel (src/index.ts). No downstream consumer in the Decentraland org imports it. Removal is a no-op for every consumer.

  • No public API breakageCreditsIcon keeps the same signature (SvgIconProps). Internal consumers (Navbar.tsx, CreditsToggle.tsx, ManaBalances.tsx) pass sx/fontSize but never viewBox, so the artwork swap is transparent. The marketplace's CreditsIcon is a separate image import, unrelated to this component.

  • Security — No secrets, no XSS vectors (no <script>, event handlers, <foreignObject>, or external references in the SVG), no unsafe React patterns. Clean.

P2 findings (non-blocking)

[P2] Orphaned JSDoc comment (src/components/Navbar/icons.tsx, line 248)

The comment /** Inline credits icon sized for the navbar (20×20). */ documented the now-deleted NavbarCreditsIcon. It currently floats above the unrelated chain-icon section comment, which reads oddly. Consider removing it.


Reviewed by Jarvis 🤖 · Requested by juanmahidalgo via GitHub

@@ -246,42 +246,6 @@ const BellIcon = memo(function BellIcon(props: IconProps) {
})

/** Inline credits icon sized for the navbar (20x20). */

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.

[P2] This JSDoc documented the deleted NavbarCreditsIcon and is now orphaned — it sits above the chain-icon section comment. Consider removing it.

Suggested change
/** Inline credits icon sized for the navbar (20x20). */
/** Chain icon components keyed by chainId. Uses the same SVGs as Icon/Chains/ but without MUI SvgIcon wrapper. */

@juanmahidalgo

Copy link
Copy Markdown
Contributor Author

Follow-up on the preview: the two C marks side by side came from the MarketplaceWithManaInNavbar story passing both credits props at once. Nothing can reach that state — creditsBalance is the marketplace's MANA-pegged expiring credits and shopCreditsBalance is the shop's USD ones, and no app passes both.

Dropped creditsBalance from that story so it shows one chip, and renamed it ShopWithManaInNavbar since with shop credits it is no longer a marketplace example. MarketplaceExample still covers the marketplace chip (with its expiry tooltip), so the marketplace is unaffected.

@decentraland-bot decentraland-bot left a comment

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.

Review Summary

Clean, well-scoped change that replaces the old gradient bloom credits icon with the hexagonal C badge across all shared surfaces. The currentColor approach is a good design decision — it lets the glyph adapt to each surface's text colour rather than carrying a baked palette.

What I checked

  • Public API surface: CreditsIcon is publicly exported via the Icon barrel, but only the rendered SVG changes — the component name and SvgIconProps interface are unchanged. Not a breaking change.
  • Dead code removal: NavbarCreditsIcon was defined and exported in Navbar/icons.tsx but never imported anywhere (Navbar.tsx uses ShopCreditsIcon and the MUI-wrapped CreditsIcon). Navbar/icons.tsx is not re-exported from the package entry point, so removing it has zero consumer impact.
  • Downstream consumers: The marketplace (decentraland/marketplace) imports its own local CreditsIcon SVG (../../images/icon-credits.svg), not the one from @dcl/ui2. No cross-repo breakage.
  • Internal consumers: CreditsToggle, ManaBalances, and Navbar all import CreditsIcon by name — the import paths and named export are unchanged, so all still work.
  • Story fix: Removing creditsBalance from the ShopWithManaInNavbar story eliminates an impossible state (no app passes both creditsBalance and shopCreditsBalance).
  • Security: No concerns — purely visual SVG changes, no user input, no auth, no secrets.

Findings

[P2] Orphaned JSDoc commenticons.tsx line 248: The comment /** Inline credits icon sized for the navbar (20x20). */ documented the now-deleted NavbarCreditsIcon. It now sits orphaned above the chain icons section. Consider removing it. (Also noted in a previous inline comment.)

[P2] Duplicated SVG paths — The hexagonal C paths are duplicated between Icon/CreditsIcon (MUI SvgIcon wrapper) and Navbar/ShopCreditsIcon (plain <svg>). The JSDoc documents this as deliberate (MUI boundary) with the note "Change one and change the other." This is a reasonable trade-off — extracting shared path constants would add complexity for minimal gain. The warning comment is sufficient.

Verdict

No P0 or P1 issues. The two P2s are minor housekeeping items that don't block merge. Approved.


Reviewed by Jarvis 🤖 · Requested by juanmahidalgo via GitHub

@@ -246,42 +246,6 @@ const BellIcon = memo(function BellIcon(props: IconProps) {
})

/** Inline credits icon sized for the navbar (20x20). */

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.

[P2] This JSDoc comment (/** Inline credits icon sized for the navbar (20x20). */) documented the now-deleted NavbarCreditsIcon and is orphaned — it sits directly above the chain-icon section comment. Consider removing it.

Suggested change
/** Inline credits icon sized for the navbar (20x20). */
/** Chain icon components keyed by chainId. Uses the same SVGs as Icon/Chains/ but without MUI SvgIcon wrapper. */

@juanmahidalgo
juanmahidalgo merged commit 80943f5 into master Aug 5, 2026
4 checks passed
@juanmahidalgo
juanmahidalgo deleted the feat/credits-icon-shop-c branch August 5, 2026 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants