Skip to content

fix wallet balance in fiat - #165

Merged
bordalix merged 1 commit into
masterfrom
hide_balance_fiat
Oct 15, 2025
Merged

bordalix merged 1 commit into
masterfrom
hide_balance_fiat

Conversation

@bordalix

@bordalix bordalix commented Oct 15, 2025 •

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • New Features

    • Balance supports multiple display modes: sats, fiat, or both.
    • Dynamic unit labels reflect the selected currency (e.g., fiat code or SATS).
    • Option to show a secondary balance line when both currencies are enabled.
  • UI

    • Primary balance now follows the selected display preference.
    • Cleaner presentation with adaptive units and optional second row for the alternate currency.

@coderabbitai

coderabbitai Bot commented Oct 15, 2025 •

Copy link
Copy Markdown
Contributor

Walkthrough

Balance.tsx now supports multiple currency display modes by importing CurrencyDisplay, computing main/other balances and units based on configuration, and conditionally rendering a second row when CurrencyDisplay is Both. The primary display switched from a hardcoded SATS label to dynamic units, without changing public interfaces.

Changes

Cohort / File(s) Summary
Balance display modes
src/components/Balance.tsx
Added import of CurrencyDisplay; computed mainBalance/otherBalance and mainUnit/otherUnit from CurrencyDisplay and config.fiat; introduced showBoth flag; replaced satsBalance and static SATS label with dynamic mainBalance/mainUnit; conditionally renders second row for otherBalance/otherUnit when display mode is Both.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant U as User
  participant B as Balance Component
  participant CFG as Config
  participant T as CurrencyDisplay

  U->>B: Render()
  B->>CFG: Read fiat currency
  B->>T: Read display mode
  alt display = Fiat
    B->>B: mainBalance = fiat<br/>mainUnit = fiat code
  else display = Sats
    B->>B: mainBalance = sats<br/>mainUnit = SATS
  else display = Both
    B->>B: mainBalance = primary<br/>otherBalance = secondary<br/>mainUnit/otherUnit set
  end
  B-->>U: Render primary row (mainBalance, mainUnit)
  opt display = Both
    B-->>U: Render secondary row (otherBalance, otherUnit)
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title “fix wallet balance in fiat” directly reflects the changes made to correct and enhance the fiat display in the Balance component, and it concisely captures a core aspect of the update without introducing unrelated details.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch hide_balance_fiat

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying wallet-signet with  Cloudflare Pages  Cloudflare Pages

Latest commit: 79ded1e
Status: ✅  Deploy successful!
Preview URL: https://0ffcf2ee.wallet-23u.pages.dev
Branch Preview URL: https://hide-balance-fiat.wallet-23u.pages.dev

View logs

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying wallet-mutinynet with  Cloudflare Pages  Cloudflare Pages

Latest commit: 79ded1e
Status: ✅  Deploy successful!
Preview URL: https://5285cfd0.arkade-wallet.pages.dev
Branch Preview URL: https://hide-balance-fiat.arkade-wallet.pages.dev

View logs

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying wallet-bitcoin-v8 with  Cloudflare Pages  Cloudflare Pages

Latest commit: 79ded1e
Status: ✅  Deploy successful!
Preview URL: https://f8c6b921.wallet-v8.pages.dev
Branch Preview URL: https://hide-balance-fiat.wallet-v8.pages.dev

View logs

@coderabbitai coderabbitai 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.

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/components/Balance.tsx (1)

46-51: Consider using && operator for conditional rendering.

The conditional rendering logic is correct. For a more idiomatic React style, you could use the && operator instead of a ternary with null:

-      {showBoth ? (
-        <FlexRow>
+      {showBoth && (
+        <FlexRow>
           <Text color='dark80'>{otherBalance}</Text>
           <Text small>{otherUnit}</Text>
         </FlexRow>
-      ) : null}
+      )}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1596ff4 and 79ded1e.

📒 Files selected for processing (1)
  • src/components/Balance.tsx (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/Balance.tsx (3)
src/components/Text.tsx (1)
  • Text (25-79)
src/icons/Eye.tsx (1)
  • EyeIcon (1-11)
src/components/FlexRow.tsx (1)
  • FlexRow (16-45)
🔇 Additional comments (2)
src/components/Balance.tsx (2)

38-40: LGTM!

The display correctly uses the computed mainBalance and mainUnit values, making the component dynamic and responsive to the currency display mode.


24-28: Approve currency display logic The code correctly switches between fiat and sats based on currencyDisplay; the CurrencyDisplay and Fiats enums guarantee valid, defined values.

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.

1 participant