-
Notifications
You must be signed in to change notification settings - Fork 0
Fix/shopping tab improvements #404
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
1. Currency dropdown: Created ShoppingCurrencyModal showing fiat currencies + XEC in a single list, sorted alphabetically (simpler than the 3-tab modal) 2. Removed 'Goods & Services' heading: Changed to 'Offers' to match P2P Trading tab. Also hide payment method badges on Shopping tab since all items are goods/services 3. Fixed scrolling: Removed local scrollbar (overflow: auto, maxHeight) to use global page scrolling like P2P Trading tab
- Added VND to LIST_TICKER_GOODS_SERVICES in constants.ts - VND can now be selected in Create Offer price dropdown for G&S offers - VND will appear in Shopping tab currency filter - Conversion uses direct VND/XEC rate from fiat API (1 VND ≈ 4.35 XEC) - Updated getCoinRate function with clearer comments for currency conversion
- XEC prices: Limited to 2 decimal places (e.g., 43,542.01 XEC instead of 43,542.014) - Fiat prices (VND): No decimals with thousands separators (e.g., 10,000 VND instead of 10000) - Updated useOfferPrice hook to round XEC amounts to 2 decimals - Updated formatAmountForGoodsServices to round to 2 decimals - Added formatFiatPrice helper function in OfferDetailInfo components - Display: Price: 43,542.01 XEC / unit (10,000 VND)
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughReplaces the shopping page header and scroll container, adds a ShoppingCurrencyModal and integrates it into the shopping filter, introduces consistent fiat formatting helpers across Offer components, adds hidePaymentMethods prop to OfferItem, rounds XEC-related amounts, and adds VND to ticker constants. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Filter as ShoppingFilterComponent
participant Modal as ShoppingCurrencyModal
participant Parent as Parent/Store
User->>Filter: open currency selector
Filter->>Modal: set isOpen(true)
Modal->>User: display searchable list (includes XEC)
User->>Modal: select currency / clear / close
Modal->>Filter: setSelectedItem(FilterCurrencyType) / onDismissModal()
Filter->>Parent: apply filter (paymentMethod/value)
Parent->>Filter: updated offers list
Filter->>User: render offers (OfferItem with hidePaymentMethods)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds Vietnamese Dong (VND) currency support to the shopping tab and implements several UI/UX improvements for displaying prices in the goods and services marketplace.
Changes:
- Added VND to the list of supported currencies for goods and services pricing
- Implemented fiat price formatting with thousands separators and no decimals for better readability
- Created a simplified currency selection modal specifically for the shopping tab
- Added XEC price rounding to 2 decimal places for cleaner display
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/telegram-ecash-escrow/src/store/constants.ts | Added VND to LIST_TICKER_GOODS_SERVICES |
| apps/telegram-ecash-escrow/src/store/util.ts | Updated comments for VND support and added XEC rounding to 2 decimals |
| apps/telegram-ecash-escrow/src/hooks/useOfferPrice.tsx | Added XEC price rounding to 2 decimal places |
| apps/telegram-ecash-escrow/src/components/OfferItem/OfferItem.tsx | Added formatFiatPrice function and hidePaymentMethods prop |
| apps/telegram-ecash-escrow/src/components/OfferDetailInfo/OfferDetailInfo.tsx | Added formatFiatPrice function for consistent price display |
| apps/telegram-ecash-escrow/src/components/DetailInfo/OfferDetailInfo.tsx | Added formatFiatPrice function for consistent price display |
| apps/telegram-ecash-escrow/src/components/FilterList/ShoppingCurrencyModal.tsx | Created new simplified currency modal for shopping tab |
| apps/telegram-ecash-escrow/src/components/FilterOffer/ShoppingFilterComponent.tsx | Updated to use ShoppingCurrencyModal instead of FilterCurrencyModal |
| apps/telegram-ecash-escrow/src/app/shopping/page.tsx | Changed title, removed explicit scroll container styling, and added hidePaymentMethods prop |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/telegram-ecash-escrow/src/components/FilterList/ShoppingCurrencyModal.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/telegram-ecash-escrow/src/app/shopping/page.tsx (1)
204-217:⚠️ Potential issue | 🟠 MajorRemove
scrollableTargetto use window scrolling, or add the container id.
The element with idscrollableDivdoes not exist in this component. The InfiniteScroll library will silently fall back to window scrolling when the target element is missing, which may not match the intended behavior.Suggested fix (use window scrolling)
- scrollableTarget="scrollableDiv"
🤖 Fix all issues with AI agents
In
`@apps/telegram-ecash-escrow/src/components/FilterList/ShoppingCurrencyModal.tsx`:
- Around line 143-146: The IconButton inside DialogTitle (className "back-btn",
onClick handleClose) is icon-only and needs an accessible text label; update the
IconButton to include an aria-label (e.g., "Back" or "Close") or associate it
with a visually-hidden text element so screen readers can announce its purpose,
ensuring the ChevronLeft icon remains purely presentational (role or
aria-hidden) if you add a separate label.
In `@apps/telegram-ecash-escrow/src/components/OfferItem/OfferItem.tsx`:
- Around line 168-172: The guard in formatFiatPrice currently treats 0 as falsy
and returns an empty string; change the check to only treat
null/undefined/empty-string as "no price" (e.g. price == null || price === '')
so zero is handled as a valid value, then continue parsing (convert string via
parseFloat) and handle NaN as before; update the early-return logic in
formatFiatPrice to preserve zero values instead of swallowing them.
🧹 Nitpick comments (2)
apps/telegram-ecash-escrow/src/components/DetailInfo/OfferDetailInfo.tsx (1)
99-108: Consider centralizingformatFiatPriceto avoid duplication.
The same helper appears in multiple components; a shared util would reduce drift.apps/telegram-ecash-escrow/src/components/FilterList/ShoppingCurrencyModal.tsx (1)
98-108: Guard against duplicate currency codes (e.g., XEC).
IfLIST_CURRENCIES_USEDever adds XEC, the list will show duplicates. Consider de‑duping by code before sorting.♻️ Optional de-duplication
- const allCurrencies = [...LIST_CURRENCIES_USED, xecOption]; - - // Sort alphabetically by code - return allCurrencies.sort((a, b) => a.code.localeCompare(b.code)); + const allCurrencies = [...LIST_CURRENCIES_USED, xecOption]; + const uniqueByCode = Array.from(new Map(allCurrencies.map(c => [c.code, c])).values()); + + // Sort alphabetically by code + return uniqueByCode.sort((a, b) => a.code.localeCompare(b.code));
apps/telegram-ecash-escrow/src/components/FilterList/ShoppingCurrencyModal.tsx
Outdated
Show resolved
Hide resolved
- ShoppingCurrencyModal: Add aria-label to IconButton for accessibility - shopping/page.tsx: Remove invalid scrollableTarget reference (use window scrolling)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@apps/telegram-ecash-escrow/src/components/FilterList/ShoppingCurrencyModal.tsx`:
- Around line 141-151: Add an accessible name by generating a stable id with
React's useId() inside the ShoppingCurrencyModal component and applying it to
the DialogTitle element (e.g., id={titleId}) and to the StyledDialog via
aria-labelledby={titleId}; update the function component to import and call
useId(), set the DialogTitle id to that generated value, and pass
aria-labelledby to StyledDialog (the component handling open/isOpen and onClose
via handleClose/Transition).
🧹 Nitpick comments (3)
apps/telegram-ecash-escrow/src/components/DetailInfo/OfferDetailInfo.tsx (1)
99-108: Consider extractingformatFiatPriceinto a shared utility.The same helper exists in
apps/telegram-ecash-escrow/src/components/OfferItem/OfferItem.tsx; centralizing it will prevent drift and keep formatting consistent.apps/telegram-ecash-escrow/src/components/FilterList/ShoppingCurrencyModal.tsx (2)
98-108: De-duplicate by code before sorting to avoid future duplicates (e.g., if XEC is ever added upstream).
This prevents duplicate entries and React key collisions ifLIST_CURRENCIES_USEDever includes the same code.♻️ Proposed refactor
- const allCurrencies = [...LIST_CURRENCIES_USED, xecOption]; - - // Sort alphabetically by code - return allCurrencies.sort((a, b) => a.code.localeCompare(b.code)); + const allCurrencies = [...LIST_CURRENCIES_USED, xecOption]; + const uniqueByCode = Array.from(new Map(allCurrencies.map(c => [c.code, c])).values()); + + // Sort alphabetically by code + return uniqueByCode.sort((a, b) => a.code.localeCompare(b.code));
120-134: Replace magic number5with a named constant for maintainability.
Line 122/131 will be easier to understand and safer to change later.♻️ Suggested tweak
- const handleSelect = (currency: { code: string; name: string }) => { + const PAYMENT_METHOD_GOODS_SERVICES = 5; + + const handleSelect = (currency: { code: string; name: string }) => { const filterCurrency: FilterCurrencyType = { - paymentMethod: 5, // PAYMENT_METHOD.GOODS_SERVICES + paymentMethod: PAYMENT_METHOD_GOODS_SERVICES, // PAYMENT_METHOD.GOODS_SERVICES value: currency.code }; @@ - const handleClear = () => { - setSelectedItem?.({ paymentMethod: 5, value: '' }); + const handleClear = () => { + setSelectedItem?.({ paymentMethod: PAYMENT_METHOD_GOODS_SERVICES, value: '' });
apps/telegram-ecash-escrow/src/components/FilterList/ShoppingCurrencyModal.tsx
Show resolved
Hide resolved
- Use React's useId() hook to generate unique dialog title ID - Add aria-labelledby to StyledDialog referencing the title ID - Improves screen reader support by properly associating dialog with its title
Polish the shopping tab, added VND support.
Summary by CodeRabbit
New Features
UI/UX Improvements