Skip to content

Remove balance label, adjust spacing, fix fiat decimals - #484

Merged
bordalix merged 4 commits into
masterfrom
wt-remove-balance-20260327
Apr 1, 2026
Merged

bordalix merged 4 commits into
masterfrom
wt-remove-balance-20260327

Conversation

@sahilc0

@sahilc0 sahilc0 commented Mar 27, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Remove the "My balance" label from the wallet homepage balance display — the label was redundant
  • Adjust balance margin from 3rem 0 2rem 0 → 2.5rem 0 1rem 0 for a tighter layout (buttons closer to balance)
  • Fix fiat display to always show two decimal places (e.g. "17.30 USD" instead of "17.3 USD") by adding minimumFractionDigits support to prettyNumber

Changed files

  • src/components/Balance.tsx — removed label, updated margin, pass minimumFractionDigits for fiat
  • src/lib/format.ts — added optional minimumFractionDigits param to prettyNumber

Test plan

  • Wallet homepage shows balance amount without "My balance" label
  • Spacing between logo, balance, and buttons looks correct
  • Fiat balance displays with two decimal places (e.g. "17.30" not "17.3")
  • JPY displays with zero decimals (unchanged behavior)
  • Sats display unaffected
  • Balance hide/show toggle still works

Summary by CodeRabbit

  • Bug Fixes

    • Improved fiat balance formatting with enhanced decimal precision control
    • Fixed numeric zero validation in number formatting utility
    • Removed redundant balance header label from display
    • Adjusted balance section spacing for better visual hierarchy
  • Tests

    • Updated test assertions to reflect UI changes

@sahilc0
sahilc0 requested review from bordalix and pietro909 March 27, 2026 10:20
@coderabbitai

coderabbitai Bot commented Mar 27, 2026 •

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 27c18fb0-d578-4733-a47f-39f82d14f06c

📥 Commits

Reviewing files that changed from the base of the PR and between 7720cfa and f9c5e6c.

📒 Files selected for processing (4)
  • src/components/Balance.tsx
  • src/lib/format.ts
  • src/test/e2e/init.test.ts
  • src/test/screens/wallet/index.test.tsx
✅ Files skipped from review due to trivial changes (2)
  • src/test/screens/wallet/index.test.tsx
  • src/test/e2e/init.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/components/Balance.tsx

Walkthrough

Updates the Balance component's display by removing the "My balance" header text and adjusting spacing. Enhances the prettyNumber formatting function with an optional minimumFractionDigits parameter and fixes its zero-value handling logic. Test assertions updated to match the removed header text.

Changes

Cohort / File(s) Summary
Balance UI Component Updates
src/components/Balance.tsx
Removes "My balance" header text from rendered output; changes outer FlexCol margin from conditional (showBoth ? '3rem 0 2rem 0' : '3rem 0 0.5rem 0') to fixed ('2.5rem 0 1rem 0'); updates fiatBalance formatting to pass minimumFractionDigits parameter to prettyNumber call.
Number Formatting Logic
src/lib/format.ts
Adds optional minimumFractionDigits parameter to prettyNumber function signature; forwards parameter to Intl.NumberFormat options; changes early-return validation from returning '0' for all falsy values to only returning '0' for undefined, null, or NaN, allowing numeric 0 to proceed through normal formatting.
Test Assertions
src/test/e2e/init.test.ts, src/test/screens/wallet/index.test.tsx
Updates test selectors from text=My balance to text=SATS to reflect removed header text; all other test assertions remain unchanged.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • bordalix
  • pietro909
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ 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%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ 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 clearly summarizes the three main changes: removing the balance label, adjusting spacing, and fixing fiat decimals to display two decimal places.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch wt-remove-balance-20260327

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

cloudflare-workers-and-pages Bot commented Mar 27, 2026 •

Copy link
Copy Markdown

Deploying wallet-bitcoin with  Cloudflare Pages  Cloudflare Pages

Latest commit: f9c5e6c
Status: ✅  Deploy successful!
Preview URL: https://8218fe38.wallet-bitcoin.pages.dev
Branch Preview URL: https://wt-remove-balance-20260327.wallet-bitcoin.pages.dev

View logs

@ghost

ghost commented Mar 27, 2026

Copy link
Copy Markdown

✓ Review Passed

UI/display improvements look good:

  • Label removal: "My balance" label removal is clean and reduces redundancy
  • Spacing: Updated margin (3rem → 2.5rem top, 2rem → 1rem bottom) creates tighter layout as intended
  • Fiat formatting: minimumFractionDigits support correctly ensures "17.30 USD" instead of "17.3 USD"

Icon updates: Back and Close icons appear to have design improvements (simplified paths). ✓

Suggestion: Verify the hide/show balance toggle works correctly after margin changes—the balance display element should still be accessible.

Test coverage looks thorough. Minor note: JPY zero-decimals test is marked as unchecked—if not urgent, consider adding it to the test plan.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Mar 27, 2026 •

Copy link
Copy Markdown

Deploying wallet-mutinynet with  Cloudflare Pages  Cloudflare Pages

Latest commit: f9c5e6c
Status: ✅  Deploy successful!
Preview URL: https://a3e41a8f.arkade-wallet.pages.dev
Branch Preview URL: https://wt-remove-balance-20260327.arkade-wallet.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: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/lib/format.ts`:
- Around line 87-99: The function prettyNumber treats 0 as falsy and returns '0'
early, preventing decimal formatting for zero; change the guard in prettyNumber
to only bypass formatting when num is null or undefined (e.g., num === null ||
num === undefined) and optionally handle NaN explicitly (e.g., return '0' for
NaN) so that 0 is passed through to Intl.NumberFormat and will render fixed
decimals per maximum/minimumFractionDigits and useGrouping options.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c4b348ab-19bd-4a84-9e4c-3b80e3857083

📥 Commits

Reviewing files that changed from the base of the PR and between 514408a and 9680223.

📒 Files selected for processing (5)
  • src/components/Balance.tsx
  • src/icons/Back.tsx
  • src/icons/Close.tsx
  • src/ionic.css
  • src/lib/format.ts

Comment thread src/lib/format.ts Resolved
@ghost

ghost commented Mar 27, 2026

Copy link
Copy Markdown

Arkana PR Review

✅ Balance UI Polish
Tight, focused polish on the balance display — all good changes.

Improvements:

  • Removing "My balance" label reduces clutter (it's obvious it's your balance in context)
  • Margin adjustment (3rem → 2.5rem top, 2rem → 1rem bottom) creates better visual hierarchy and button proximity
  • Fiat decimals fix is important: enforcing 2 decimal places (e.g., "17.30 USD" vs "17.3") maintains consistency and looks more professional

Code Changes:

  • fiatDecimals() correctly passed as both maximumFractionDigits and minimumFractionDigits to enforce exactly that many decimals
  • Back/Close icon simplifications (SVG paths) look visually identical, minor code cleanup
  • Header border color consistency (using CSS var instead of hardcoded Rename instances of "ark" to "arkade" for clarity #444)

Testing:

  • Verify fiat decimals render correctly at different exchange rates and currencies
  • Confirm spacing looks right on various screen sizes

Overall: Small, safe changes that improve polish. Ready to merge.

@ghost

ghost commented Mar 27, 2026

Copy link
Copy Markdown

Review: Remove balance label, adjust spacing, fix fiat decimals

✅ Clean refactor

  • Removing redundant "My balance" label improves visual clarity
  • Tighter margin spacing (3rem → 2.5rem, 2rem → 1rem) brings buttons closer, which is good for mobile
  • Fiat decimal fix ("17.30 USD" vs "17.3 USD") is UX-correct and consistent with accounting conventions

Minor point:

  • Test plan has JPY unchecked. Verify zero-decimal formatting works for JPY (should not show ".00")

✅ Mergeable — straightforward styling + format fix.

@bordalix

Copy link
Copy Markdown
Collaborator

Fix merge conflicts and LFG!

sahilc0 added 4 commits March 31, 2026 15:02
Remove "My balance" label from wallet homepage balance display.
Adjust margin from 3rem/2rem to 2.5rem/1rem for tighter layout.
Add minimumFractionDigits to prettyNumber so fiat always shows
two decimal places (e.g. "17.30" instead of "17.3").
…rmatting

Change falsy check to explicit null/undefined/NaN guard so zero
values pass through Intl.NumberFormat and respect minimumFractionDigits
(e.g. 0 renders as '0.00' for fiat).
The 'My balance' label was removed from the Balance component.
Update e2e and unit tests to match.
@sahilc0
sahilc0 force-pushed the wt-remove-balance-20260327 branch from 7720cfa to f9c5e6c Compare March 31, 2026 20:14
@ghost

ghost commented Mar 31, 2026

Copy link
Copy Markdown

Review: wallet#484

Title: Remove balance label, adjust spacing, fix fiat decimals

Summary

Clean UI improvement—removes redundant "My balance" label, tightens spacing, and fixes fiat formatting to always display 2 decimals.

Observations

✅ Good

  • Label removal reduces clutter; balance amount is self-explanatory
  • Spacing adjustment (3rem → 2.5rem top margin, 2rem → 1rem bottom) tightens layout naturally
  • minimumFractionDigits fix ensures consistent fiat display (e.g., "17.30" vs "17.3")
  • Explicit null/NaN check in prettyNumber (num === undefined || num === null || Number.isNaN(num)) prevents falsy coercion bugs with zero values
  • Tests updated to match new markup

⚠️ Minor observations

  • Removed conditional margin logic (showBoth ? ... : ... → fixed 2.5rem 0 1rem 0). Verify layout doesn't regress when only one balance type is visible (e.g., Sats-only mode). The fixed margin should still work, but worth a manual check.
  • minimumFractionDigits parameter added as 4th arg to prettyNumber after useGrouping. This shifts the signature and could break callers passing positional args. Check if all call sites are compatible.

Security & Protocol

  • No security impact; purely UI/formatting.
  • No protocol changes; safe to merge.

Status: Looks good. Recommend testing the layout in both single/dual-currency modes before merge.

@ghost

ghost commented Mar 31, 2026

Copy link
Copy Markdown

Review: #484 - Remove balance label, adjust spacing, fix fiat decimals

✅ Changes are good:

  • UI/UX: Removing redundant "My balance" label is sensible—the balance area is visually clear without it. Tighter spacing (2.5rem/1rem) improves layout harmony.
  • Format fix: Explicit null/undefined/NaN check in prettyNumber is correct. Allows zero to pass through Intl.NumberFormat and respect minimumFractionDigits (e.g., 0 → '0.00' for fiat).
  • Test updates: E2E and unit test changes match the label removal properly.

No security or protocol concerns. This is a pure wallet UI improvement.

@bordalix
bordalix merged commit abb8e4f into master Apr 1, 2026
5 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request Apr 1, 2026
@bordalix
bordalix deleted the wt-remove-balance-20260327 branch May 18, 2026 16:17
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