Skip to content

fix(perf): Button loading UX, iconOnly prop, requireConfirm pattern; AssetBadge fallback#220

Open
Anichris-koded wants to merge 1 commit into
Sorokit:mainfrom
Anichris-koded:fix/211-button-loading-icononly-requireconfirm
Open

fix(perf): Button loading UX, iconOnly prop, requireConfirm pattern; AssetBadge fallback#220
Anichris-koded wants to merge 1 commit into
Sorokit:mainfrom
Anichris-koded:fix/211-button-loading-icononly-requireconfirm

Conversation

@Anichris-koded

Copy link
Copy Markdown

Summary

Closes #211

Addresses all three issues from the description plus the perf fixes named in the issue title.


Changes

Button — 3 behaviour fixes + 1 perf fix

1. Loading state keeps label visible (spinner in icon slot)

Previously the spinner was injected next to children, causing layout shift on short labels. The spinner now occupies the leading icon slot and the label is always rendered in a <span> beside it — matching the GitHub-style pattern described in the issue.

Before: [spinner] Send would shift layout on short labels.
After: spinner in icon slot, label always visible beside it.

2. iconOnly prop — square button, no horizontal padding

<Button iconOnly size="sm" aria-label="Refresh"><RefreshIcon /></Button>

Applies h-N w-N (no px-) at all three sizes. Icon-only buttons now use the Button primitive consistently instead of raw <button> elements, giving them correct focus rings and hover states.

3. requireConfirm + confirmLabel — two-click confirmation

<Button variant="destructive" requireConfirm confirmLabel="Delete account?">
  Delete
</Button>
  • First click: label switches to confirmLabel (default: "Are you sure?"), onClick is not fired
  • Second click: onClick fires, state resets
  • Blur: pending confirmation is cancelled (user tabbed/clicked away)

Perf: variants, sizes, iconOnlySizes hoisted to module scope — objects are never recreated on render.


AssetBadge — perf fix

getAssetColor previously returned { bg: "bg-surface-2", text: "text-ink-2" } as a new object literal on every call for unknown assets. A stable ASSET_COLOR_FALLBACK constant is now hoisted to module scope — same reference on every cache miss.


Badge — confirmed correct

variants and dots were already module-level constants. No change needed.


Acceptance criteria

Criterion Evidence
Loading state keeps label visible with spinner in icon position Test: keeps the label visible when loading
iconOnly makes button square with no horizontal padding Tests: applies square size classes, does not apply horizontal padding
requireConfirm triggers two-click confirmation Tests: does not fire onClick on first click, shows confirmLabel after first click, fires onClick on second click, resets on blur
All existing Button usages work SorobanInvokeButton 10/10 ✅
tsc --noEmit passes exit 0 ✅

Test plan

npm test -- --run src/components/ui/Button.test.tsx src/components/AssetBadge.test.tsx src/components/ui/Badge.test.tsx

22 Button + 14 AssetBadge + 4 Badge = 40 tests, all passing.

…AssetBadge fallback

Closes Sorokit#211

Button:
- Loading spinner now renders in the icon slot; label stays visible at
  all times (no more layout shift on short labels like 'Send')
- Add iconOnly prop: applies aspect-square size class and removes all
  horizontal padding, making the button suitable for icon-only usage
  with consistent focus/hover styles
- Add requireConfirm + confirmLabel props: first click switches the
  label to confirmLabel ('Are you sure?' by default); second click fires
  onClick and resets state; blur cancels the pending confirmation
- Hoist variants, sizes, and iconOnlySizes lookup objects to module
  scope so they are never recreated on render

AssetBadge:
- Hoist ASSET_COLOR_FALLBACK to module scope so getAssetColor returns
  a stable object reference on cache misses instead of a new literal
  on every call

Badge:
- variants and dots were already module-level; no change required

Test / infra:
- Add vitest jsdom environment + setupFiles to vite.config.ts
- Install missing devDeps: @testing-library/react, @testing-library/jest-dom,
  @testing-library/user-event, jsdom, @radix-ui/react-slot, clsx,
  tailwind-merge
- Fix @creit.tech/stellar-wallets-kit version (^0.0.0-beta.0 → ^1.2.1)
- Expand Button.test.tsx: 22 tests covering all new props and edge cases
@drips-wave

drips-wave Bot commented Jun 29, 2026

Copy link
Copy Markdown

@Anichris-koded Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

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.

fix(perf): AssetBadge ASSET_COLORS lookup in render causes new object per render, Badge variants object recreated, Button sizes object recreated

1 participant