Skip to content

fix(vendor): pass currency to QuoteForm so price label reflects configured currency - #291

Draft
dccakes wants to merge 1 commit into
mainfrom
claude/sharp-feynman-vqe78m
Draft

dccakes wants to merge 1 commit into
mainfrom
claude/sharp-feynman-vqe78m

Conversation

@dccakes

@dccakes dccakes commented Sep 6, 2026

Copy link
Copy Markdown
Owner

Daily Product Improvement

Summary

The QuoteForm price field always displayed Price ($) regardless of the workspace's configured currency. For a British couple using GBP, the form showed a dollar sign — an obvious mismatch. The fix adds a currency prop to QuoteForm and updates both call sites in VendorDetailPanel to forward the currency already available there.

Recommendations Reviewed

  • Area: vendor/quote-form.tsx · Issue: Price label hardcoded to Price ($) ignoring configured currency · Recommendation: Add currency prop and display it like the rest of the app · Type: UX/UI · Priority: High · Effort: Small

  • Area: checklist/_components/checklist-page-client.tsx · Issue: Category filter chips render raw enum values (VENDORS, STATIONERY) via {category} instead of using TASK_CATEGORY_LABELS[category] · Recommendation: Replace {category} with {TASK_CATEGORY_LABELS[category]} in filter chips · Type: UX/UI · Priority: Medium · Effort: Small

  • Area: guest-list/guests-view.tsx · Issue: Guest drawer "Save changes" button stays static during mutation — no "Saving…" text swap or spinner, unlike every other save in the app · Recommendation: Swap to "Saving…" while updateHouseholdMutation.isPending · Type: UX/UI · Priority: Medium · Effort: Small

  • Area: budget/category-card.tsx · Issue: The expanded-section Edit and Delete trigger buttons are not disabled while deleteCategory.isPending or editCategory.isPending, allowing duplicate mutations · Recommendation: Add disabled prop to both buttons when a mutation is in flight · Type: UX/UI · Priority: Medium · Effort: Small

  • Area: guest-list/index.tsx · Issue: Inline error state uses raw <h1> without font-serif/font-mono styling, no icon, no CTA — inconsistent with every other empty/error state in the app · Recommendation: Rebuild using the standard centered-icon + serif title + mono subtitle + Button CTA pattern · Type: UX/UI · Priority: Medium · Effort: Small

  • Area: vendor/vendor-detail-panel.tsx · Issue: When vendor/vendorData/enrichedVendor are falsy, the component returns two headless no-op components instead of null · Recommendation: Replace with a plain return null · Type: Frontend cleanup · Priority: Low · Effort: Small

Selected Improvement

Add currency prop to QuoteForm and forward it from VendorDetailPanel for both create and edit modes.

Why This Was Selected

Every other monetary label in the app — ExpenseForm, CategoryForm, BudgetSummary — threads the currency code through and displays it next to amounts. QuoteForm was the one outlier, silently hardcoding $. The fix is small, safe, and directly visible to any non-USD user the moment they open the quote form.

Changes Made

  • Added currency?: string prop to QuoteFormProps with a default of DEFAULT_CURRENCY
  • Imported DEFAULT_CURRENCY from ~/lib/budget/currency
  • Changed the price field label from Price ($) to Price ({currency})
  • Passed currency={currency} from VendorDetailPanel to both QuoteForm usages (create and edit modes)

Files Changed

  • src/components/vendor/quote-form.tsx
  • src/components/vendor/vendor-detail-panel.tsx

Verification

  • TypeScript: npx tsc --noEmit — no new errors
  • Biome lint: npx @biomejs/biome check on both files — no errors, one pre-existing class-order warning in unrelated code
  • Reviewed both QuoteForm call sites in VendorDetailPanel — currency now forwarded correctly in both create and edit contexts
  • QuoteForm used nowhere else in the codebase — confirmed with grep

Future Recommendations

  • Recommendation: Use TASK_CATEGORY_LABELS[category] instead of raw {category} in checklist filter chips · Priority: Medium · Effort: Small · Reason not included: Separate focused area; CSS uppercase makes the visual impact subtle (mainly VENDORS → VENDOR) · Should become GitHub issue: No

  • Recommendation: Add "Saving…" label to guest drawer save button while mutation is pending · Priority: Medium · Effort: Small · Reason not included: Different module; keeping this PR focused · Should become GitHub issue: No

  • Recommendation: Disable Budget CategoryCard Edit/Delete trigger buttons during pending mutations · Priority: Medium · Effort: Small · Reason not included: Different module · Should become GitHub issue: No

  • Recommendation: Align GuestList inline error state with the app design system (font-serif title, font-mono subtitle, icon, Button CTA) · Priority: Medium · Effort: Small · Reason not included: Separate module; error path only · Should become GitHub issue: No

  • Recommendation: Replace the VendorDetailPanel falsy-guard early return with return null instead of two headless no-op components · Priority: Low · Effort: Small · Reason not included: Code smell only, no user-visible impact · Should become GitHub issue: No

  • Recommendation: guest-search-filter.tsx custom RSVP/Tag/Country dropdowns lack Radix-backed keyboard navigation and ARIA semantics — replace with Select from ~/components/ui/select · Priority: High · Effort: Large · Reason not included: Large refactor touching three interlinked filter controls · Should become GitHub issue: Yes — accessibility regression vs the rest of the app

  • Recommendation: Full i18n pass for authenticated dashboard (all hardcoded English strings) — next-intl is installed but only used in public-facing wedding website routes · Priority: Medium · Effort: Large · Reason not included: Cross-cutting, requires product/design alignment on locale scope · Should become GitHub issue: Yes

GitHub Issues Created or Proposed

Proposed: [Guest List] Replace custom filter dropdowns with accessible Radix-backed Select components

  • Problem: RSVP Status, Guest Tag, and Country filter dropdowns in guest-search-filter.tsx are manually implemented with useState + useOuterClick + absolutely-positioned <div>, lacking keyboard navigation, role="listbox", and ARIA semantics. All other dropdowns in the app use ~/components/ui/select.
  • Recommended solution: Replace all three custom dropdown controls with the Select component from ~/components/ui/select (Radix-backed), or a Combobox for the tag filter which supports typeahead.
  • Expected benefit: Keyboard and screen-reader users can filter the guest list; visual consistency with the rest of the app.
  • Suggested acceptance criteria: All three filter controls are keyboard-navigable; selecting an option via keyboard works; Escape closes the dropdown; role="combobox" or role="listbox" is present.
  • Priority: High · Effort: Large · Source: Daily product improvement review

🤖 Generated with Claude Code

https://claude.ai/code/session_01ST4FAdpfv1douVDVhhCD8Y


Generated by Claude Code

…ts the configured currency

The Price field in QuoteForm always showed `Price ($)` regardless of the
workspace's currency setting. The rest of the app (ExpenseForm, CategoryForm)
already uses `currency` as a label token, so QuoteForm now follows the same
pattern — accepting a `currency` prop (defaults to DEFAULT_CURRENCY) and
rendering `Price ({currency})` instead of the hardcoded dollar sign.

VendorDetailPanel already received `currency` from its parent; both
QuoteForm call sites in the panel now forward it.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ST4FAdpfv1douVDVhhCD8Y
@vercel

vercel Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
scv Error Error Sep 6, 2026 10:14am UTC

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Jest Test Coverage

Coverage Summary

Lines Statements Branches Functions
Coverage: 83%
83.11% (36264/43631) 82.14% (3813/4642) 69.49% (1073/1544)

This branch had an error being deployed

1 failed deployment
Preview — be16c482 Deployed Sep 6, 2026 by vercel[bot]
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