Skip to content

feat(money): swap bottom-bar Money icon for Dollar glyph (MUSD-773)#29813

Merged
Kureev merged 9 commits into
mainfrom
kureev/MUSD-773
May 7, 2026
Merged

feat(money): swap bottom-bar Money icon for Dollar glyph (MUSD-773)#29813
Kureev merged 9 commits into
mainfrom
kureev/MUSD-773

Conversation

@Kureev
Copy link
Copy Markdown
Contributor

@Kureev Kureev commented May 6, 2026

Description

Replaces the legacy Bank icon used by the Money tab in the bottom navigation bar with the new Dollar glyph from the Figma spec (MUSD-773), in both selected (filled) and unselected (outline) states.

  • Adds two new SVG assets: dollar.svg (outlined circle + dollar sign) and dollar-filled.svg (solid circle with dollar-sign cutout).
  • Both SVGs use currentColor for stroke and fill so the Icon component can tint them per theme — they pick up the correct active/inactive color in both light and dark mode, matching the convention used by the other tab bar icons (Home/HomeFilled, MetamaskFoxOutline/MetamaskFoxFilled).
  • The filled variant uses fill-rule="evenodd" so the dollar sign is a transparent cutout through the colored circle, showing the tab-bar background through it (avoids hardcoded light/dark color pairs).
  • Wires the new icons in TabBar.constants.ts (inactive → IconName.Dollar) and TabBar.tsx (FILLED_ICONS map → IconName.DollarFilled).
  • Adds Dollar and DollarFilled to the local IconName enum and asset map by hand to avoid the side-effect rewrite that yarn generate-icons would have done to the rest of the enum.

Changelog

CHANGELOG entry: null

Related issues

Fixes: MUSD-773

Manual testing steps

Feature: Money tab uses the new Dollar icon in both states and themes

  Scenario: Money tab is unselected in light mode
    Given the app is in light mode
    And the user is not on the Money tab
    Then the Money tab icon is the outlined Dollar glyph
    And its stroke and dollar sign use the inactive icon tint

  Scenario: Money tab is unselected in dark mode
    Given the app is in dark mode
    And the user is not on the Money tab
    Then the Money tab icon is the outlined Dollar glyph
    And its stroke and dollar sign use the inactive icon tint

  Scenario: Money tab is selected in light mode
    Given the app is in light mode
    When the user taps the Money tab
    Then the Money tab icon switches to the filled Dollar glyph
    And the circle paints in the active icon tint
    And the dollar sign is transparent, showing the tab-bar background through it

  Scenario: Money tab is selected in dark mode
    Given the app is in dark mode
    When the user taps the Money tab
    Then the Money tab icon switches to the filled Dollar glyph
    And the circle paints in the active icon tint
    And the dollar sign is transparent, showing the tab-bar background through it

Screenshots/Recordings

Before

After

image image

Pre-merge author checklist

Performance checks (if applicable)

  • I've tested on Android
  • I've tested with a power user scenario
  • I've instrumented key operations with Sentry traces for production performance metrics

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Note

Low Risk
Low risk UI-only change that swaps icon assets and enum entries; main risk is any missed references to the removed/renamed IconName values causing build-time errors.

Overview
Updates the bottom navigation Money tab to use new Musd/MusdFilled icon assets instead of the legacy bank glyph, wiring them through the tab bar icon maps.

Extends the icon system with new IconName entries and SVG assets, and updates Portfolio-related tab UI/tests to use IconName.PieChart in place of the removed IconName.Portfolio (including HomepageDiscoveryTabs).

Reviewed by Cursor Bugbot for commit ee491bb. Bugbot is set up for automated code reviews on this repo. Configure here.

Replaces the legacy Bank icon used for the Money tab in both selected and
unselected states with the new Dollar / DollarFilled glyphs from the Figma
spec. Both SVGs are theme-aware via currentColor, so the icon picks up the
correct active/inactive tint in light and dark modes; the filled variant
uses an even-odd cutout so the dollar sign shows the tab-bar background
through it.
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@Kureev Kureev self-assigned this May 6, 2026
@github-actions github-actions Bot added the size-S label May 6, 2026
@Kureev Kureev marked this pull request as ready for review May 6, 2026 17:24
@Kureev Kureev requested a review from a team as a code owner May 6, 2026 17:24
Copy link
Copy Markdown
Contributor

@brianacnguyen brianacnguyen left a comment

Choose a reason for hiding this comment

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

can you update these with the musd and musd-filled icons provided?

Comment thread app/component-library/components/Icons/Icon/Icon.types.ts Outdated
@Kureev
Copy link
Copy Markdown
Contributor Author

Kureev commented May 6, 2026

Updated icons:
image
image

image image

@Kureev Kureev requested a review from brianacnguyen May 6, 2026 21:12
@Kureev Kureev enabled auto-merge May 6, 2026 21:12
brianacnguyen
brianacnguyen previously approved these changes May 6, 2026
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit c938a48. Configure here.

Comment thread app/component-library/components/Icons/Icon/Icon.types.ts Outdated
@Kureev Kureev requested a review from brianacnguyen May 6, 2026 22:37
brianacnguyen
brianacnguyen previously approved these changes May 6, 2026
brianacnguyen
brianacnguyen previously approved these changes May 6, 2026
@Matt561
Copy link
Copy Markdown
Contributor

Matt561 commented May 7, 2026

@cursoragent monitor CI and fix failures that appear. Begin by fixing the failing unit test then fix the tsc lint errors.

Keep fix commits separate

@cursor
Copy link
Copy Markdown
Contributor

cursor Bot commented May 7, 2026

The CI failures are both coming from the same root cause: HomepageDiscoveryTabs.tsx still references IconName.Portfolio (which no longer exists), which breaks lint:tsc and causes HomepageDiscoveryTabs.test.tsx to crash while rendering Icon.

Next I’m going to update HomepageDiscoveryTabs.tsx to use the new icon enum (IconName.PieChart), then rerun yarn lint:tsc and the failing unit test file to confirm CI should go green.

Open in Web Open in Cursor 

@cursor cursor Bot requested a review from a team as a code owner May 7, 2026 02:28
@Matt561 Matt561 requested a review from brianacnguyen May 7, 2026 02:44
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeMoney, SmokeWalletPlatform
  • Selected Performance tags: None (no tests recommended)
  • Risk Level: low
  • AI Confidence: 82%
click to see 🤖 AI reasoning details

E2E Test Selection:
The PR makes the following changes:

  1. New icons added: Musd and MusdFilled SVG icons added to the icon library (Icon.types.ts, Icon.assets.ts, new SVG files).

  2. Icon rename: IconName.Portfolio renamed to IconName.PieChart - the underlying SVG asset (pie-chart.svg) is unchanged, only the enum name changed. All usages have been updated (confirmed no remaining IconName.Portfolio references in .ts/.tsx files).

  3. TabBar.tsx & TabBar.constants.ts: The Money tab icon changed from IconName.Bank / IconName.Bank to IconName.MusdFilled / IconName.Musd (filled/outline states). This is a visual-only change to the bottom navigation bar's Money tab icon.

  4. HomepageDiscoveryTabs.tsx: The "Portfolio" discovery tab now uses IconName.PieChart instead of IconName.Portfolio - purely a rename, same SVG asset.

  5. Test files: Unit test files updated to reflect the PortfolioPieChart rename.

Impact Assessment:

  • SmokeMoney: Directly affected - the Money tab in the bottom TabBar now shows a different icon (Musd instead of Bank). Tests that navigate to the Money/Card/Ramps screens via the TabBar should verify the icon renders correctly.
  • SmokeWalletPlatform: The HomepageDiscoveryTabs Portfolio tab icon changed. This component is embedded in the Wallet home screen (Trending/Discovery tabs area). Tests covering the wallet home and discovery tabs should verify the icon renders.
  • TabBar navigation: The TabBar is used by virtually all E2E tests, but the change is purely visual (icon swap) and doesn't affect navigation logic, routing, or tab selection behavior. No functional regression risk for other test suites.
  • No functional changes: No logic, routing, state management, or API changes. Pure icon/visual updates.

The IconName.Portfolio removal is safe - confirmed no remaining usages in the codebase. The new Musd/MusdFilled icons are properly registered in all required places (types, assets, SVG files).

Performance Test Selection:
These are purely visual/icon changes - swapping icon names and adding new SVG icons. No performance-sensitive code paths are affected: no state management changes, no list rendering changes, no data fetching, no controller changes, no app startup changes. The icon swap in TabBar and HomepageDiscoveryTabs has negligible performance impact.

View GitHub Actions results

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 7, 2026

@Kureev Kureev added this pull request to the merge queue May 7, 2026
Merged via the queue into main with commit 8136f4f May 7, 2026
95 of 96 checks passed
@Kureev Kureev deleted the kureev/MUSD-773 branch May 7, 2026 04:12
@github-actions github-actions Bot locked and limited conversation to collaborators May 7, 2026
@metamaskbotv2 metamaskbotv2 Bot added the release-7.77.0 Issue or pull request that will be included in release 7.77.0 label May 7, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-7.77.0 Issue or pull request that will be included in release 7.77.0 size-S team-earn

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants