PRO-3757 - Updated pulse model design#433
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (5)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughPresentational and responsive refactor across Pulse UI: swapped Search icon asset, standardized horizontal padding and wrapper divs, restyled toggles/buttons/panels/controls, reduced interactive icon sizes, added HighDecimalsFormatted truncation callback, and updated tests and xs Tailwind breakpoint. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant BuyView as BuyButton
participant TokenState as TokenState
participant Renderer as DOM
BuyView->>TokenState: query selected token
alt token present
BuyView->>Renderer: render "Buy" with token info (text-base)
else token absent
BuyView->>Renderer: render "Select token" (text-base)
end
sequenceDiagram
autonumber
participant SellView as Sell
participant HighFmt as HighDecimalsFormatted
participant Local as LocalState
participant Renderer as DOM
SellView->>Local: toggle showNumInP / set tokenAmount
SellView->>HighFmt: render value (pass setTruncatedFlag callback)
HighFmt-->>SellView: setTruncatedFlag(true/false)
SellView->>Renderer: render formatted display or numeric input based on showNumInP
sequenceDiagram
autonumber
participant SearchView as Search
participant Wrapper as StyledContainer
participant RefreshComp as Refresh
participant Renderer as DOM
SearchView->>Wrapper: place Refresh/Esc inside outer styled container
Wrapper->>Renderer: render inner centering div(s)
Renderer->>RefreshComp: render spinner/img at 18×18
RefreshComp-->>Renderer: user click / loading (behavior unchanged)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
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 |
Deploying x with
|
| Latest commit: |
e39acbf
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ac70fd70.x-e62.pages.dev |
| Branch Preview URL: | https://pro-3757-pulse-design-change.x-e62.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
src/apps/pulse/components/App/HomeScreen.tsx (1)
939-989: Fix invalid CSS property syntax in Buy/Sell toggle buttons.Both Buy and Sell button spans have the same critical CSS syntax issues identified in other components:
- CSS properties should use camelCase (fontFamily, fontWeight, fontSize) instead of quoted hyphenated strings.
- Invalid values: 'font-style': 'Medium' (should be removed), 'leading-trim': 'NONE' (not standard), 'letter-spacing': '-2%' (should be '-0.26px').
- This pattern is duplicated across BuyButton.tsx, SellButton.tsx, and the percentage buttons in Sell.tsx.
Apply the same fix as suggested in other button components and consider extracting this repeated styling into a shared constant to eliminate duplication.
src/apps/pulse/components/Buy/BuyButton.tsx (1)
141-163: Fix invalid CSS property syntax and values.This inline style object has the same critical issues identified in
SellButton.tsx(lines 125-145):
- CSS properties should use camelCase (fontFamily, fontWeight, fontSize) instead of quoted hyphenated strings.
- Invalid values: 'font-style': 'Medium' (should be removed), 'leading-trim': 'NONE' (not standard), 'letter-spacing': '-2%' (should be '-0.26px').
- This exact pattern is duplicated across SellButton.tsx and HomeScreen.tsx.
Apply the same fix as suggested in SellButton.tsx and consider extracting the styling into a shared constant to eliminate duplication across BuyButton, SellButton, and HomeScreen components.
src/apps/pulse/components/Sell/Sell.tsx (1)
428-442: Fix invalid CSS property syntax and values.The inline style object for the percentage button labels has the same critical CSS syntax issues identified in other button components:
- CSS properties should use camelCase (fontFamily, fontWeight, fontSize) instead of quoted hyphenated strings ('font-family', 'font-weight', etc.).
- Invalid values: 'leading-trim': 'NONE' (not standard CSS), 'letter-spacing': '-2%' (should be '-0.26px' or similar).
- 'font': 'Poppins' should be 'fontFamily': 'Poppins'.
Apply the same fix as suggested in SellButton.tsx and BuyButton.tsx. Consider extracting this repeated styling into a shared constant to eliminate duplication across multiple components.
🧹 Nitpick comments (2)
src/apps/pulse/components/Buy/BuyButton.tsx (1)
71-76: Consider simplifying the nested ternary.The nested ternary with an ESLint disable comment suggests this logic could be clearer. Consider refactoring for readability:
- // eslint-disable-next-line no-nested-ternary - return selectedToken?.symbol - ? `Buy ${selectedToken.symbol}` - : selectedToken === null - ? 'Select token' - : 'Buy'; + if (selectedToken === null) { + return 'Select token'; + } + return selectedToken?.symbol ? `Buy ${selectedToken.symbol}` : 'Buy';src/apps/pulse/components/Search/Search.tsx (1)
310-313: Remove commented code.The commented-out code should be removed to keep the codebase clean. If this code might be needed later, rely on version control history instead.
- {/* <div - className="mt-2.5 w-10 h-10 bg-black rounded-[10px] p-0.5 pb-1 pl-0.5 pr-0.5" - data-testid="pulse-search-refresh-button" - > */} <div
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/apps/pulse/assets/refresh-icon.svgis excluded by!**/*.svgsrc/apps/pulse/assets/search-icon.pngis excluded by!**/*.png
📒 Files selected for processing (9)
src/apps/pulse/components/App/HomeScreen.tsx(7 hunks)src/apps/pulse/components/Buy/Buy.tsx(4 hunks)src/apps/pulse/components/Buy/BuyButton.tsx(3 hunks)src/apps/pulse/components/Misc/Refresh.tsx(1 hunks)src/apps/pulse/components/Misc/Settings.tsx(1 hunks)src/apps/pulse/components/Search/ChainSelect.tsx(1 hunks)src/apps/pulse/components/Search/Search.tsx(3 hunks)src/apps/pulse/components/Sell/Sell.tsx(3 hunks)src/apps/pulse/components/Sell/SellButton.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
PR: pillarwallet/x#391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
Applied to files:
src/apps/pulse/components/Sell/Sell.tsx
🧬 Code graph analysis (3)
src/apps/pulse/components/Search/Search.tsx (1)
src/apps/pulse/components/Misc/Refresh.tsx (1)
Refresh(10-36)
src/apps/pulse/components/App/HomeScreen.tsx (1)
src/apps/pulse/components/Misc/Refresh.tsx (1)
Refresh(10-36)
src/apps/pulse/components/Sell/Sell.tsx (1)
src/utils/blockchain.ts (1)
getLogoForChainId(165-199)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: unit-tests
- GitHub Check: lint
🔇 Additional comments (9)
src/apps/pulse/components/Misc/Refresh.tsx (1)
17-32: LGTM!The sizing changes from Tailwind classes to inline styles and the reduced icon dimensions (18x18) align with the design refresh. The background color update to
#1E1D24is consistent with the theme changes across the PR.src/apps/pulse/components/Search/ChainSelect.tsx (1)
6-6: LGTM!The background color update to
#1E1D24is consistent with the broader theme refresh across the Pulse UI.src/apps/pulse/components/Misc/Settings.tsx (1)
6-23: LGTM!The restructured button with explicit inline styles and centered icon wrapper aligns with the design refresh. The inline styles use correct camelCase syntax.
src/apps/pulse/components/Sell/Sell.tsx (2)
238-250: LGTM! Truncation logic correctly implements PR requirement.The truncation logic correctly implements the PR objective: display the token name only when the combined length of symbol and name is 13 characters or less. When the combined length exceeds 13, only the symbol is displayed. This aligns with the stated requirement.
317-317: LGTM! Symbol truncation with tooltip is user-friendly.Displaying only the first 3 characters of the symbol with a tooltip for symbols longer than 4 characters is a good UX pattern that maintains visual consistency while providing full information on hover.
src/apps/pulse/components/Search/Search.tsx (1)
314-343: LGTM!The refresh area restructuring with explicit wrapper divs and inline styles aligns with the design refresh and uses correct camelCase CSS property syntax.
src/apps/pulse/components/App/HomeScreen.tsx (1)
23-23: Verify the icon format change from SVG to PNG.The SearchIcon was changed from SVG to PNG format. SVG is generally preferred for icons due to:
- Better scalability across different screen resolutions
- Smaller file size for simple icons
- Easier styling with CSS (fill, stroke, etc.)
Ensure this change aligns with the design requirements and that the PNG provides sufficient quality at all target resolutions.
src/apps/pulse/components/Buy/Buy.tsx (2)
503-509: LGTM! Design updates align with Figma specifications.The styling changes (color adjustment to #1E1D24, border radius reduced to 6, removed center justification) are presentational refinements that match the updated design system.
556-591: LGTM! Truncation logic correctly implements the PR requirement.The token name is now conditionally rendered based on the combined length of symbol and name (≤13 characters), matching the PR objective. The layout refinements (compact vertical stack, fixed dimensions, updated text styling) support the new design. The 0.3 opacity on the token name creates appropriate visual hierarchy.
| <span | ||
| style={{ | ||
| 'font-family': 'Poppins', | ||
| 'font-weight': 500, | ||
| 'font-style': 'Medium', | ||
| 'font-size': '13px', | ||
| 'leading-trim': 'NONE', | ||
| 'line-height': '100%', | ||
| 'letter-spacing': '-2%', | ||
| 'text-align': 'center', | ||
| }} | ||
| > | ||
| {getButtonText( | ||
| isLoadingOffer, | ||
| isInitialized, | ||
| token, | ||
| tokenAmount, | ||
| sellOffer, | ||
| isDisabled() | ||
| )} | ||
| </span> |
There was a problem hiding this comment.
Fix invalid CSS property syntax and values.
The inline style object contains critical issues:
-
Property names: CSS properties in React inline styles should use camelCase (e.g.,
fontFamily,fontWeight,fontSize) instead of quoted hyphenated strings ('font-family', 'font-weight', 'font-size'). -
Invalid values:
'font-style': 'Medium'is invalid. ThefontStyleproperty accepts 'normal', 'italic', or 'oblique', not font weights.'leading-trim': 'NONE'is not standard CSS and is not widely supported.'letter-spacing': '-2%'is invalid. Letter-spacing should be specified in pixels (e.g., '-0.26px') or ems, not percentages.
-
Code duplication: This exact styling pattern is repeated in
BuyButton.tsxandHomeScreen.tsx. Consider extracting it into a reusable styled component or constant.
Apply this diff to fix the CSS syntax:
<span
style={{
- 'font-family': 'Poppins',
- 'font-weight': 500,
- 'font-style': 'Medium',
- 'font-size': '13px',
- 'leading-trim': 'NONE',
- 'line-height': '100%',
- 'letter-spacing': '-2%',
- 'text-align': 'center',
+ fontFamily: 'Poppins',
+ fontWeight: 500,
+ fontSize: '13px',
+ lineHeight: '100%',
+ letterSpacing: '-0.26px',
+ textAlign: 'center',
}}
>Additionally, consider extracting this repeated styling into a constant at the top of the file or a shared constant file:
const BUTTON_TEXT_STYLE = {
fontFamily: 'Poppins',
fontWeight: 500,
fontSize: '13px',
lineHeight: '100%',
letterSpacing: '-0.26px',
textAlign: 'center',
} as const;Then use it as: <span style={BUTTON_TEXT_STYLE}>
🤖 Prompt for AI Agents
In src/apps/pulse/components/Sell/SellButton.tsx around lines 125 to 145, the
inline style object uses invalid React CSS syntax and values (hyphenated quoted
keys, incorrect fontStyle value, unsupported leading-trim, and percentage
letter-spacing); replace the quoted hyphenated keys with camelCase properties
(e.g., fontFamily, fontWeight, fontSize, lineHeight, letterSpacing, textAlign),
change fontStyle to a valid value or remove it if representing weight, remove
the nonstandard leading-trim property, and convert letter-spacing to a valid
unit (e.g., '-0.26px'); then extract the corrected object into a shared constant
(e.g., BUTTON_TEXT_STYLE) at the top of the file or in a shared styles file and
use <span style={BUTTON_TEXT_STYLE}> to avoid duplication across BuyButton.tsx
and HomeScreen.tsx.
github-actions
bot
temporarily deployed
to
Preview (PRO-3757-Pulse_Design_Changes)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
src/apps/pulse/components/App/HomeScreen.tsx (2)
863-877: Fix invalid SVG attributes on PNG image.The inline style contains SVG-specific attributes (
fill,stroke,strokeWeight,strokePosition,strokeStatePoint,strokeEndPoint) that have no effect on a PNG<img>element.Apply this diff to remove the ineffective properties:
<span style={{ marginLeft: 14, marginRight: 10 }}> <img src={SearchIcon} alt="search-icon" width={12} height={12} - style={{ - fill: '#FFFFFF', - stroke: '#FFFFFF', - strokeWeight: 1.5, - strokePosition: 'outside', - strokeStatePoint: 'Round', - strokeEndPoint: 'Round', - }} /> </span>
879-891: Fix invalid CSS property syntax.The inline style object contains invalid CSS:
font: 'Poppins'should befontFamily: 'Poppins'- Using
fontshorthand incorrectly will prevent the font from being appliedApply this diff:
<div className="flex-1" style={{ color: 'grey', textAlign: 'left', opacity: 0.5, - font: 'Poppins', + fontFamily: 'Poppins', height: 20, fontSize: 13, }} >src/apps/pulse/components/Buy/Buy.tsx (1)
744-754: Fix invalid inline style syntax in percentage buttons.The inline style contains multiple CSS syntax errors:
- Hyphenated properties (
'leading-trim','line-height','letter-spacing','text-align') must use camelCase in React'leading-trim'is not a standard CSS property'letter-spacing': '-2%'is invalid (letter-spacing doesn't accept percentage units)Apply this diff to fix the syntax:
<span - className="opacity-50 font-normal text-sm" - style={{ - 'leading-trim': 'NONE', - 'line-height': '13px', - 'letter-spacing': '-2%', - 'text-align': 'center', - }} + className="opacity-50 font-normal text-sm leading-[13px] tracking-tight text-center" > {isMax ? 'MAX' : `$${item}`} </span>This replaces the invalid inline styles with equivalent Tailwind classes for cleaner, more maintainable code.
🧹 Nitpick comments (2)
src/apps/pulse/components/Buy/BuyButton.tsx (1)
71-76: Simplify the nested ternary expression.The nested ternary on lines 71-76 is hard to read and maintain, even with the eslint-disable comment. The logic could be clearer as a simple if-else chain or early returns.
Apply this diff to improve readability:
- // eslint-disable-next-line no-nested-ternary - return selectedToken?.symbol - ? `Buy ${selectedToken.symbol}` - : selectedToken === null - ? 'Select token' - : 'Buy'; + if (selectedToken?.symbol) { + return `Buy ${selectedToken.symbol}`; + } + return selectedToken === null ? 'Select token' : 'Buy';src/apps/pulse/components/Sell/Sell.tsx (1)
312-312: Document the 3-character symbol truncation.The symbol is truncated to 3 characters (
token.symbol.slice(0, 3)) for display next to the amount input. While this keeps the UI compact, it may cause confusion for tokens with similar prefixes (e.g., 'USDC' and 'USDT' both become 'USD').Consider adding a comment explaining this truncation logic, and ensure the tooltip on lines 314-318 adequately addresses this by showing the full symbol on hover.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
src/apps/pulse/components/App/tests/__snapshots__/AppWrapper.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/App/tests/__snapshots__/HomeScreen.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/Buy/tests/__snapshots__/Buy.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/Buy/tests/__snapshots__/BuyButton.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/Buy/tests/__snapshots__/PreviewBuy.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/Search/tests/__snapshots__/Search.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/Sell/tests/__snapshots__/PreviewSell.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/Sell/tests/__snapshots__/Sell.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/Sell/tests/__snapshots__/SellButton.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (5)
src/apps/pulse/components/App/HomeScreen.tsx(11 hunks)src/apps/pulse/components/Buy/Buy.tsx(8 hunks)src/apps/pulse/components/Buy/BuyButton.tsx(3 hunks)src/apps/pulse/components/Sell/Sell.tsx(6 hunks)src/apps/pulse/components/Sell/tests/Sell.test.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
PR: pillarwallet/x#391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
Applied to files:
src/apps/pulse/components/Sell/Sell.tsx
🧬 Code graph analysis (3)
src/apps/pulse/components/Sell/Sell.tsx (1)
src/utils/blockchain.ts (1)
getLogoForChainId(165-199)
src/apps/pulse/components/Buy/Buy.tsx (1)
src/utils/blockchain.ts (1)
getLogoForChainId(165-199)
src/apps/pulse/components/App/HomeScreen.tsx (1)
src/apps/pulse/components/Misc/Refresh.tsx (1)
Refresh(10-36)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: lint
- GitHub Check: unit-tests
- GitHub Check: build
🔇 Additional comments (3)
src/apps/pulse/components/Sell/tests/Sell.test.tsx (1)
123-132: LGTM! Test expectations align with the updated UI logic.The test correctly verifies:
- The truncated symbol display ('TES' from 'TEST')
- The full symbol in the token selector ('TEST')
- The updated USD formatting ('$100.00')
These changes align with the PR's truncation logic and formatting updates.
src/apps/pulse/components/Buy/Buy.tsx (1)
553-562: LGTM! Consistent truncation logic with Sell component.The conditional rendering of the token name based on combined length (
token.symbol.length + token.name.length <= 13) matches the implementation in the Sell component and aligns with the PR requirements.src/apps/pulse/components/Sell/Sell.tsx (1)
239-249: Confirm symbol truncation requirementsIn Sell.tsx (lines 239–249), the
token.symbol.length + token.name.length <= 13check only hides the name—very long symbols could still overflow. Verify with design if symbols longer than 13 characters also need truncation or ellipses.
github-actions
bot
temporarily deployed
to
Preview (PRO-3757-Pulse_Design_Changes)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/apps/pulse/components/App/HomeScreen.tsx (2)
863-878: Fix invalid CSS properties on PNG image.The inline style object contains invalid CSS properties that have no effect on a PNG
<img>element:
fill,stroke,strokeWeight,strokePosition,strokeStatePoint,strokeEndPointare SVG-specific attributesThese properties were flagged in a previous review but remain unresolved.
Apply this diff to remove the ineffective properties:
<span style={{ marginLeft: 14, marginRight: 10 }}> <img src={SearchIcon} alt="search-icon" width={12} height={12} - style={{ - fill: '#FFFFFF', - stroke: '#FFFFFF', - strokeWeight: 1.5, - strokePosition: 'outside', - strokeStatePoint: 'Round', - strokeEndPoint: 'Round', - }} /> </span>
879-891: Fix invalid CSS property syntax.The inline style uses
font: 'Poppins'which is invalid in React inline styles. This was flagged in a previous review but remains unresolved.Apply this diff:
<div className="flex-1" style={{ color: 'grey', textAlign: 'left', opacity: 0.5, - font: 'Poppins', + fontFamily: 'Poppins', height: 20, fontSize: 13, }} >src/apps/pulse/components/Buy/Buy.tsx (1)
741-751: Fix invalid inline style syntax in quick-bid buttons.The inline style object uses invalid CSS property names that will cause React warnings:
- Hyphenated properties (
'leading-trim','line-height','letter-spacing','text-align') must use camelCase in React inline styles'leading-trim'is not a standard CSS property'letter-spacing': '-2%'is invalid (percentages are not valid for letter-spacing)This was flagged in a previous review as addressed, but the issues remain in the current code.
Apply this diff to fix the CSS syntax:
<span - className="opacity-50 font-normal text-sm" - style={{ - 'leading-trim': 'NONE', - 'line-height': '13px', - 'letter-spacing': '-2%', - 'text-align': 'center', - }} + className="opacity-50 font-normal text-sm leading-[13px] tracking-tight text-center" > {isMax ? 'MAX' : `$${item}`} </span>Note: Using Tailwind classes eliminates the need for inline styles and ensures valid CSS.
🧹 Nitpick comments (1)
src/apps/pulse/components/App/HomeScreen.tsx (1)
982-998: Consider using Tailwind classes for alignment.The wrapper div uses inline styles for
justifyContent,alignItems, anddisplay. For consistency with the rest of the codebase, consider using Tailwind utility classes:<div style={{ padding: '8px 1px', width: 36, height: 34, backgroundColor: '#1E1D24', borderRadius: 8, - display: 'flex', - justifyContent: 'center', }} + className="flex justify-center" >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/apps/pulse/components/App/HomeScreen.tsx(9 hunks)src/apps/pulse/components/Buy/Buy.tsx(10 hunks)src/apps/pulse/components/Sell/Sell.tsx(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/apps/pulse/components/Sell/Sell.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
src/apps/pulse/components/App/HomeScreen.tsx (1)
src/apps/pulse/components/Misc/Refresh.tsx (1)
Refresh(10-36)
src/apps/pulse/components/Buy/Buy.tsx (2)
src/apps/pulse/utils/blockchain.ts (1)
isNativeToken(17-18)src/utils/blockchain.ts (1)
getLogoForChainId(165-199)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: lint
- GitHub Check: unit-tests
- GitHub Check: build
🔇 Additional comments (2)
src/apps/pulse/components/App/HomeScreen.tsx (1)
935-965: LGTM! Toggle button styling fixed.The Buy and Sell toggle buttons now properly use Tailwind utility classes (
className="text-center font-medium text-sm") instead of the invalid inline styles that were flagged in previous reviews. This resolves the critical CSS syntax issues.src/apps/pulse/components/Buy/Buy.tsx (1)
553-562: Verify token name and symbol are always defined.The truncation logic directly accesses
token.symbol.lengthandtoken.name.length. Ensure thatSelectedTokentype guarantees these properties are always defined (not optional or nullable) to prevent potential runtime errors.If there's a possibility of undefined values, add null checks:
- {token.symbol.length + token.name.length <= 13 && ( + {token.symbol && token.name && token.symbol.length + token.name.length <= 13 && (
github-actions
bot
temporarily deployed
to
Preview (PRO-3757-Pulse_Design_Changes)
There was a problem hiding this comment.
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)
src/apps/pulse/components/Sell/Sell.tsx (1)
308-321: Tooltip is not keyboard accessible.The tooltip displaying the full token symbol only appears on mouse hover (
onMouseEnter/onMouseLeave), making it inaccessible to keyboard users. Consider using a more accessible tooltip implementation or adding keyboard focus handlers.Example enhancement with keyboard support:
<p className="text-grey flex-shrink-0 opacity-50 cursor-help mobile:text-4xl xs:text-4xl desktop:text-4xl tablet:text-4xl desktop:w-20 tablet:w-20 mobile:w-20 xs:w-20 font-medium" data-testid="pulse-sell-token-symbol" + tabIndex={0} onMouseEnter={() => setShowTooltip(true)} onMouseLeave={() => setShowTooltip(false)} + onFocus={() => setShowTooltip(true)} + onBlur={() => setShowTooltip(false)} > {token.symbol.slice(0, 3)} </p>
🧹 Nitpick comments (2)
src/apps/pulse/components/Sell/Sell.tsx (2)
239-249: Truncation logic correctly implements PR requirement.The conditional rendering of the token name based on combined length (≤ 13 characters) correctly implements the specified truncation behavior.
Consider extracting the magic number to a named constant for better maintainability:
+const MAX_TOKEN_DISPLAY_LENGTH = 13; + const Sell = (props: SellProps) => { ... - {token.symbol.length + token.name.length <= 13 && ( + {token.symbol.length + token.name.length <= MAX_TOKEN_DISPLAY_LENGTH && (
182-459: Consider consolidating colors and styling approach.The component mixes Tailwind classes with inline styles and uses hardcoded hex color values throughout (
#121116,#1E1D24,#FFFFFF, etc.). For better maintainability:
- Extract color values to a theme file or constants
- Prefer Tailwind classes over inline styles where possible
- Consider using CSS-in-JS solution if more complex styling is needed
This would make future design updates easier to manage consistently across components.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/apps/pulse/components/Sell/Sell.tsx(9 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
PR: pillarwallet/x#391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
Applied to files:
src/apps/pulse/components/Sell/Sell.tsx
🧬 Code graph analysis (1)
src/apps/pulse/components/Sell/Sell.tsx (1)
src/utils/blockchain.ts (1)
getLogoForChainId(165-199)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: lint
- GitHub Check: unit-tests
- GitHub Check: build
🔇 Additional comments (2)
src/apps/pulse/components/Sell/Sell.tsx (2)
181-183: LGTM! Layout updates align with design requirements.The container and card styling changes appropriately implement the new design specifications.
380-435: LGTM! Percentage buttons implementation is correct.The button layout, styling, and disabled state handling are properly implemented.
github-actions
bot
temporarily deployed
to
Preview (PRO-3757-Pulse_Design_Changes)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/apps/pulse/components/Sell/Sell.tsx (2)
204-206: Clarify theborderRadiusunit.The
borderRadius: 50lacks a unit specifier. For a circular border, use'50%'or specify pixels explicitly.Apply this diff:
style={{ - borderRadius: 50, + borderRadius: '50%', }}
293-304: Remove commented code.The commented wrapper div (lines 293-294, 304) should be removed to keep the codebase clean. If this code might be needed later, rely on version control history instead.
Apply this diff:
<div className="flex items-center max-w-60 desktop:w-60 tablet:w-60 mobile:w-52 xs:w-44 text-right justify-end bg-transparent outline-none pr-0" style={{ height: 36 }} > - {/* <div className="flex-1 min-w-0 overflow-hidden"> */} <input className={`no-spinner flex mobile:text-4xl xs:text-4xl desktop:text-4xl tablet:text-4xl font-medium text-right ${token - ? "desktop:w-40 tablet:w-40 mobile:w-32 xs:w-24 mr-1" : "desktop:w-60 tablet:w-60 mobile:w-52 xs:w-44"}`} + ? "desktop:w-40 tablet:w-40 mobile:w-32 xs:w-24 mr-1.5" : "desktop:w-60 tablet:w-60 mobile:w-52 xs:w-44"}`} placeholder={inputPlaceholder} onChange={handleTokenAmountChange} value={tokenAmount} type="text" onFocus={() => setInputPlaceholder('')} data-testid="pulse-sell-amount-input" /> - {/* </div> */} {token && (
🧹 Nitpick comments (1)
src/apps/pulse/components/Sell/Sell.tsx (1)
365-365: Consider consistent symbol display.Line 313 truncates the token symbol to 3 characters in the main display, but here the full symbol is shown. This inconsistency might confuse users, especially when symbols are long.
If truncation is needed for space, consider truncating here as well:
{formatExponentialSmallNumber( limitDigitsNumber(tokenBalance) )}{' '} - {token.symbol}($ + {token.symbol.slice(0, 3)}($ {formatExponentialSmallNumber( limitDigitsNumber(tokenBalance * parseFloat(token.usdValue)) )} )Or provide a tooltip for consistency with line 308-320.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/apps/pulse/components/Sell/Sell.tsx(8 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
PR: pillarwallet/x#391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
Applied to files:
src/apps/pulse/components/Sell/Sell.tsx
🧬 Code graph analysis (1)
src/apps/pulse/components/Sell/Sell.tsx (1)
src/utils/blockchain.ts (1)
getLogoForChainId(165-199)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: lint
- GitHub Check: unit-tests
- GitHub Check: build
| className={`text-grey opacity-50 mobile:text-4xl xs:text-4xl desktop:text-4xl tablet:text-4xl font-medium ${token.symbol.length > 3 ? 'cursor-help' : ''}`} | ||
| data-testid="pulse-sell-token-symbol" | ||
| onMouseEnter={() => setShowTooltip(true)} | ||
| onMouseLeave={() => setShowTooltip(false)} | ||
| > | ||
| {token.symbol.length > 4 | ||
| ? `${token.symbol.slice(0, 3)}...` | ||
| : token.symbol} | ||
| {token.symbol.slice(0, 3)} | ||
| </p> | ||
| {showTooltip && token.symbol.length > 4 && ( | ||
| {showTooltip && token.symbol.length > 3 && ( | ||
| <div className="absolute bottom-full left-1/2 transform -translate-x-1/2 mb-2 px-2 py-1 bg-black text-white text-xs rounded shadow-lg z-10 whitespace-nowrap"> | ||
| {token.symbol} | ||
| </div> | ||
| )} | ||
| </div> | ||
| </> |
There was a problem hiding this comment.
Make tooltip keyboard-accessible.
The tooltip only triggers on mouse events (onMouseEnter/onMouseLeave), preventing keyboard users from accessing the full symbol. This violates WCAG 2.1 SC 1.3.1 (Info and Relationships) and SC 2.1.1 (Keyboard).
Add keyboard event handlers:
<p
className={`text-grey opacity-50 mobile:text-4xl xs:text-4xl desktop:text-4xl tablet:text-4xl font-medium ${token.symbol.length > 3 ? 'cursor-help' : ''}`}
data-testid="pulse-sell-token-symbol"
+ tabIndex={token.symbol.length > 3 ? 0 : undefined}
onMouseEnter={() => setShowTooltip(true)}
onMouseLeave={() => setShowTooltip(false)}
+ onFocus={() => setShowTooltip(true)}
+ onBlur={() => setShowTooltip(false)}
>
{token.symbol.slice(0, 3)}
</p>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| className={`text-grey opacity-50 mobile:text-4xl xs:text-4xl desktop:text-4xl tablet:text-4xl font-medium ${token.symbol.length > 3 ? 'cursor-help' : ''}`} | |
| data-testid="pulse-sell-token-symbol" | |
| onMouseEnter={() => setShowTooltip(true)} | |
| onMouseLeave={() => setShowTooltip(false)} | |
| > | |
| {token.symbol.length > 4 | |
| ? `${token.symbol.slice(0, 3)}...` | |
| : token.symbol} | |
| {token.symbol.slice(0, 3)} | |
| </p> | |
| {showTooltip && token.symbol.length > 4 && ( | |
| {showTooltip && token.symbol.length > 3 && ( | |
| <div className="absolute bottom-full left-1/2 transform -translate-x-1/2 mb-2 px-2 py-1 bg-black text-white text-xs rounded shadow-lg z-10 whitespace-nowrap"> | |
| {token.symbol} | |
| </div> | |
| )} | |
| </div> | |
| </> | |
| <p | |
| className={`text-grey opacity-50 mobile:text-4xl xs:text-4xl desktop:text-4xl tablet:text-4xl font-medium ${token.symbol.length > 3 ? 'cursor-help' : ''}`} | |
| data-testid="pulse-sell-token-symbol" | |
| tabIndex={token.symbol.length > 3 ? 0 : undefined} | |
| onMouseEnter={() => setShowTooltip(true)} | |
| onMouseLeave={() => setShowTooltip(false)} | |
| onFocus={() => setShowTooltip(true)} | |
| onBlur={() => setShowTooltip(false)} | |
| > | |
| {token.symbol.slice(0, 3)} | |
| </p> | |
| {showTooltip && token.symbol.length > 3 && ( | |
| <div className="absolute bottom-full left-1/2 transform -translate-x-1/2 mb-2 px-2 py-1 bg-black text-white text-xs rounded shadow-lg z-10 whitespace-nowrap"> | |
| {token.symbol} | |
| </div> | |
| )} | |
| </> |
🤖 Prompt for AI Agents
In src/apps/pulse/components/Sell/Sell.tsx around lines 308 to 320, the
token-symbol tooltip is only triggered by mouse events so keyboard users cannot
access it; make the symbol element keyboard-accessible by adding tabIndex={0},
onFocus and onBlur handlers that mirror onMouseEnter/onMouseLeave
(setShowTooltip true/false), and add an onKeyDown handler that toggles/shows the
tooltip when Enter or Space is pressed; also add appropriate ARIA attributes
(e.g., aria-haspopup="true" and aria-expanded based on showTooltip, and
aria-describedby or aria-label linking to the tooltip) so screen readers can
discover the relationship.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/apps/pulse/components/Sell/Sell.tsx (1)
365-369: Guard USD balance calculation against NaNIf token.usdValue is undefined/non-numeric, this renders NaN. Default to 0.
- {formatExponentialSmallNumber( - limitDigitsNumber(tokenBalance * parseFloat(token.usdValue)) - )} + {formatExponentialSmallNumber( + limitDigitsNumber(tokenBalance * (Number(token.usdValue) || 0)) + )}
♻️ Duplicate comments (3)
src/apps/pulse/components/Sell/Sell.tsx (3)
203-206: Use a circular radius value (or Tailwind) for the token logo imageNumeric 50 implies 50px; intent is a circle. Use '50%' or Tailwind rounded-full.
- className="w-6 h-6 ml-1 mr-1" - data-testid="pulse-sell-token-selector-logo" - style={{ - borderRadius: 50, - }} + className="w-6 h-6 ml-1 mr-1 rounded-full" + data-testid="pulse-sell-token-selector-logo"
293-304: Remove commented-out wrapper divThese comments add noise and were previously flagged.
- {/* <div className="flex-1 min-w-0 overflow-hidden"> */} <input className={`no-spinner flex mobile:text-4xl xs:text-4xl desktop:text-4xl tablet:text-4xl font-medium text-right ${token ? "desktop:w-40 tablet:w-40 mobile:w-32 xs:w-24 mr-1" : "desktop:w-60 tablet:w-60 mobile:w-52 xs:w-44"}`} placeholder={inputPlaceholder} onChange={handleTokenAmountChange} value={tokenAmount} type="text" onFocus={() => setInputPlaceholder('')} data-testid="pulse-sell-amount-input" /> - {/* </div> */}
239-249: Null-safe length check for token.name to prevent runtime errortoken.name can be undefined; accessing .length will throw. Add a guard.
- {token.symbol.length + token.name.length <= 13 && ( + {token.name && token.symbol.length + token.name.length <= 13 && ( <p className="opacity-30 desktop:text-sm mobile:text-xs xs:text-xs font-normal ml-1" style={{ color: '#FFFFFF', }} data-testid="pulse-sell-token-selector-name" > {token.name} </p> )}
🧹 Nitpick comments (3)
src/apps/pulse/components/Sell/Sell.tsx (3)
209-214: Anchor avatar initials overlay correctly; remove conflicting size classesMake the fallback avatar container relative and drop w-full/h-full to avoid conflicts.
- <div className="w-full h-full overflow-hidden rounded-full w-[34px] h-6 ml-1 mr-1"> + <div className="relative overflow-hidden rounded-full w-[34px] h-6 ml-1 mr-1"> <RandomAvatar name={token.name || ''} /> <span className="absolute inset-0 flex items-center justify-center text-white text-xs"> {token.name?.slice(0, 2)} </span> </div>
1-1: Use native Number.parseInt with radix and drop lodash importAvoid pulling lodash for parseInt; reduces bundle size and ambiguity.
-import { parseInt } from 'lodash'; +// (removed lodash parseInt; using native Number.parseInt)- const percentage = isMax ? 100 : parseInt(item); + const percentage = isMax ? 100 : Number.parseInt(item, 10);Also applies to: 384-387
154-174: Reset amount on token change to avoid stale UXReset input and derived state when token switches, as established in this flow.
useEffect(() => { if (tokenAmount && tokenAmount.trim() !== '') { const inputAmount = parseFloat(tokenAmount); if (!Number.isNaN(inputAmount)) { if (!Number.isNaN(token?.usdValue)) { setMinAmount(inputAmount * Number(token?.usdValue) < 1); } else { setMinAmount(false); } setNotEnoughLiquidity(inputAmount > tokenBalance); } else { setNotEnoughLiquidity(false); setMinAmount(false); } } else { setMinAmount(false); setNotEnoughLiquidity(false); } }, [token, tokenBalance, tokenAmount]); + + // Reset input and derived state when switching tokens + useEffect(() => { + setTokenAmount(''); + setParentTokenAmount(''); + setInputPlaceholder('0.00'); + setLocalSellOffer(null); + setNotEnoughLiquidity(false); + setMinAmount(false); + }, [token]);Based on learnings
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/apps/pulse/components/Sell/Sell.tsx(8 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
PR: pillarwallet/x#391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
Applied to files:
src/apps/pulse/components/Sell/Sell.tsx
🧬 Code graph analysis (1)
src/apps/pulse/components/Sell/Sell.tsx (1)
src/utils/blockchain.ts (1)
getLogoForChainId(165-199)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: lint
- GitHub Check: unit-tests
- GitHub Check: build
github-actions
bot
temporarily deployed
to
Preview (PRO-3757-Pulse_Design_Changes)
github-actions
bot
temporarily deployed
to
Preview (PRO-3757-Pulse_Design_Changes)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
src/apps/pulse/components/Sell/Sell.tsx (3)
205-207: Clarify theborderRadiusunit.The
borderRadius: 50lacks a unit specifier. For a circular border, use'50%'or specify pixels explicitly with'50px'.Apply this diff:
style={{ - borderRadius: 50, + borderRadius: '50%', }}
240-250: Fix potential null pointer error when accessingtoken.name.length.If
token.nameisnullorundefined, accessing.lengthwill throw a runtime error. Line 213 uses optional chaining (token.name?.slice), confirming thattoken.namecan be undefined.Apply this diff to add null safety:
- {token.symbol.length + token.name.length <= 13 && ( + {token.name && token.symbol.length + token.name.length <= 13 && ( <p className="opacity-30 desktop:text-sm mobile:text-xs xs:text-xs font-normal ml-1" style={{ color: '#FFFFFF', }} data-testid="pulse-sell-token-selector-name" > {token.name} </p> )}
311-318: Make tooltip keyboard-accessible.The tooltip only triggers on mouse events (
onMouseEnter/onMouseLeave), preventing keyboard users from accessing the full symbol. This violates WCAG 2.1 SC 2.1.1 (Keyboard).Add keyboard event handlers:
<p className={`text-grey opacity-50 mobile:text-4xl xs:text-4xl desktop:text-4xl tablet:text-4xl font-medium ${token.symbol.length > 3 ? 'cursor-help' : ''}`} data-testid="pulse-sell-token-symbol" + tabIndex={token.symbol.length > 3 ? 0 : undefined} onMouseEnter={() => setShowTooltip(true)} onMouseLeave={() => setShowTooltip(false)} + onFocus={() => setShowTooltip(true)} + onBlur={() => setShowTooltip(false)} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + setShowTooltip((v) => !v); + } + }} + aria-label={token.symbol} > {token.symbol.slice(0, 3)} </p>src/apps/pulse/components/Buy/Buy.tsx (1)
741-751: Fix invalid inline style syntax.The inline styles contain syntax errors that will prevent proper rendering:
'leading-trim': 'NONE'is not a standard CSS property- Hyphenated properties (
'line-height','letter-spacing','text-align') must use camelCase in React inline styles'letter-spacing': '-2%'is invalid (should usepxoremunits)Replace inline styles with Tailwind classes for cleaner, more maintainable code:
- <span - className="opacity-50 font-normal text-sm" - style={{ - 'leading-trim': 'NONE', - 'line-height': '13px', - 'letter-spacing': '-2%', - 'text-align': 'center', - }} - > + <span className="opacity-50 font-normal text-sm leading-[13px] tracking-tight text-center"> {isMax ? 'MAX' : `$${item}`} </span>Alternatively, fix the inline style syntax:
<span className="opacity-50 font-normal text-sm" style={{ - 'leading-trim': 'NONE', - 'line-height': '13px', - 'letter-spacing': '-2%', - 'text-align': 'center', + lineHeight: '13px', + letterSpacing: '-0.025em', + textAlign: 'center', }} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/apps/pulse/components/Buy/Buy.tsx(10 hunks)src/apps/pulse/components/Sell/Sell.tsx(12 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
PR: pillarwallet/x#391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
Applied to files:
src/apps/pulse/components/Sell/Sell.tsx
🧬 Code graph analysis (2)
src/apps/pulse/components/Buy/Buy.tsx (2)
src/apps/pulse/utils/blockchain.ts (1)
isNativeToken(17-18)src/utils/blockchain.ts (1)
getLogoForChainId(165-199)
src/apps/pulse/components/Sell/Sell.tsx (2)
src/utils/blockchain.ts (1)
getLogoForChainId(165-199)src/utils/number.tsx (1)
limitDigitsNumber(42-69)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (8)
src/apps/pulse/components/Sell/Sell.tsx (4)
55-55: Verify initialminAmountstate.Setting
minAmounttotrueinitially means the minimum amount warning will display before the user enters any value. Confirm this is the intended behavior, as it differs from the typical pattern where validation errors appear only after user interaction.
365-377: LGTM: Improved balance display.The use of
HighDecimalsFormattedwithlimitDigitsNumberprovides precise balance formatting with appropriate styling for trailing zeros. The format clearly shows both token amount and USD value.
387-442: LGTM: Clean percentage button implementation.The percentage buttons are well-structured with:
- Consistent sizing and spacing
- Clear disabled state styling
- Proper responsive behavior
- Clean separation of button logic and presentation
445-466: LGTM: Consistent button container styling.The sell button container uses appropriate inline styles for precise sizing control, consistent with the overall responsive design approach.
src/apps/pulse/components/Buy/Buy.tsx (4)
614-637: LGTM: Clean USD input implementation.The USD input area is well-structured with responsive sizing and clear visual separation between the amount and currency label.
639-710: LGTM: Comprehensive error handling.The error handling logic clearly communicates various validation states to users with appropriate warning messages.
759-788: LGTM: Consistent button container styling.The buy button container matches the Sell component's approach with appropriate inline styles for precise sizing control.
202-206: Fix incorrect native token selection logic.The chainId equality check was removed, which means this will now select ANY native token as long as
chainIdOfMaxStableBalanceis truthy, potentially selecting a native token from the wrong chain.The previous logic correctly verified that the token's chainId matched
chainIdOfMaxStableBalance. Restore the equality check:const nativeToken = tokens.find( (t) => Number(getChainId(t.blockchain as MobulaChainNames)) === - chainIdOfMaxStableBalance && isNativeToken(t.contract) + chainIdOfMaxStableBalance && + isNativeToken(t.contract) );Likely an incorrect or invalid review comment.
| {token.symbol.length + token.name.length <= 13 && ( | ||
| <p | ||
| className="ml-1 opacity-30 font-normal desktop:text-sm mobile:text-xs xs:text-xs" | ||
| style={{ | ||
| color: '#FFFFFF', | ||
| }} | ||
| > | ||
| {token.name} | ||
| </p> | ||
| )} |
There was a problem hiding this comment.
Fix potential null pointer error when accessing token.name.length.
If token.name is null or undefined, accessing .length will throw a runtime error. Line 529 uses optional chaining (token.name?.slice), confirming that token.name can be undefined.
Apply this diff to add null safety:
- {token.symbol.length + token.name.length <= 13 && (
+ {token.name && token.symbol.length + token.name.length <= 13 && (
<p
className="ml-1 opacity-30 font-normal desktop:text-sm mobile:text-xs xs:text-xs"
style={{
color: '#FFFFFF',
}}
>
{token.name}
</p>
)}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {token.symbol.length + token.name.length <= 13 && ( | |
| <p | |
| className="ml-1 opacity-30 font-normal desktop:text-sm mobile:text-xs xs:text-xs" | |
| style={{ | |
| color: '#FFFFFF', | |
| }} | |
| > | |
| {token.name} | |
| </p> | |
| )} | |
| {token.name && token.symbol.length + token.name.length <= 13 && ( | |
| <p | |
| className="ml-1 opacity-30 font-normal desktop:text-sm mobile:text-xs xs:text-xs" | |
| style={{ | |
| color: '#FFFFFF', | |
| }} | |
| > | |
| {token.name} | |
| </p> | |
| )} |
🤖 Prompt for AI Agents
In src/apps/pulse/components/Buy/Buy.tsx around lines 553 to 562, the condition
uses token.name.length which can be undefined and cause a runtime error; change
the condition to safely handle missing names (e.g. use optional chaining or a
fallback length: token.symbol.length + (token.name?.length ?? 0) <= 13) and keep
the rendering guard consistent (only render the <p> when token.name is present
and non-empty). Ensure both the boolean check and the JSX rendering use the same
null-safe check so no .length is accessed on undefined.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
github-actions
bot
temporarily deployed
to
Preview (PRO-3757-Pulse_Design_Changes)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
src/apps/pulse/components/Sell/Sell.tsx (4)
297-308: Remove commented code.The commented wrapper div lines should be removed to keep the codebase clean. Use version control to recover old code if needed.
Apply this diff:
<div className="flex items-center max-w-60 desktop:w-60 tablet:w-60 mobile:w-52 xs:w-44 text-right justify-end bg-transparent outline-none pr-0" style={{ height: 36 }} > - {/* <div className="flex-1 min-w-0 overflow-hidden"> */} <input className={`no-spinner flex mobile:text-4xl xs:text-4xl desktop:text-4xl tablet:text-4xl font-medium text-right ${token ? "desktop:w-40 tablet:w-40 mobile:w-32 xs:w-24 mr-1" : "desktop:w-60 tablet:w-60 mobile:w-52 xs:w-44"}`} placeholder={inputPlaceholder} onChange={handleTokenAmountChange} value={tokenAmount} type="text" onFocus={() => setInputPlaceholder('')} data-testid="pulse-sell-amount-input" /> - {/* </div> */} {token && (
311-324: Make tooltip keyboard-accessible.The tooltip only responds to mouse events, preventing keyboard users from accessing the full symbol. Add keyboard handlers and ARIA attributes for accessibility.
Apply this diff:
<div className="relative"> <p className={`text-grey opacity-50 mobile:text-4xl xs:text-4xl desktop:text-4xl tablet:text-4xl font-medium ${token.symbol.length > 3 ? 'cursor-help' : ''}`} data-testid="pulse-sell-token-symbol" + tabIndex={token.symbol.length > 3 ? 0 : undefined} onMouseEnter={() => setShowTooltip(true)} onMouseLeave={() => setShowTooltip(false)} + onFocus={() => setShowTooltip(true)} + onBlur={() => setShowTooltip(false)} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + setShowTooltip((v) => !v); + } + }} + aria-label={token.symbol.length > 3 ? token.symbol : undefined} + role={token.symbol.length > 3 ? 'button' : undefined} > {token.symbol.slice(0, 3)} </p> {showTooltip && token.symbol.length > 3 && ( - <div className="absolute bottom-full left-1/2 transform -translate-x-1/2 mb-2 px-2 py-1 bg-black text-white text-xs rounded shadow-lg z-10 whitespace-nowrap"> + <div + className="absolute bottom-full left-1/2 transform -translate-x-1/2 mb-2 px-2 py-1 bg-black text-white text-xs rounded shadow-lg z-10 whitespace-nowrap" + role="tooltip" + > {token.symbol} </div> )}
205-207: Add unit to borderRadius value.The
borderRadius: 50lacks a unit specifier. Use'50%'for a circular border or'50px'for a fixed pixel radius.Apply this diff:
style={{ - borderRadius: 50, + borderRadius: '50%', }}
240-250: Add null safety check for token.name.Accessing
token.name.lengthwithout checking iftoken.nameexists can throw a runtime error. Line 213 uses optional chaining (token.name?.slice), confirming thattoken.namecan be undefined.Apply this diff:
- {token.symbol.length + token.name.length <= 13 && ( + {token.name && token.symbol.length + token.name.length <= 13 && ( <p
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/apps/pulse/components/Sell/Sell.tsx(11 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
PR: pillarwallet/x#391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
Applied to files:
src/apps/pulse/components/Sell/Sell.tsx
🧬 Code graph analysis (1)
src/apps/pulse/components/Sell/Sell.tsx (2)
src/utils/blockchain.ts (1)
getLogoForChainId(165-199)src/utils/number.tsx (1)
limitDigitsNumber(42-69)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
| }} | ||
| > | ||
| ${formatExponentialSmallNumber(token.usdValue)} | ||
| ${token.usdValue} |
There was a problem hiding this comment.
Why removing formatExponentialSmallNumber?
There was a problem hiding this comment.
I don't know why will revert this
| /> | ||
| <div className="flex ml-1.5"> | ||
| <img src={ArrowDown} className="w-2 h-1" alt="arrow-down" /> | ||
| </div> |
There was a problem hiding this comment.
where is data-testid="pulse-sell-token-selector-arrow"????
| > | ||
| <div | ||
| className="flex items-center justify-center max-w-[150px] w-32" | ||
| style={{ |
There was a problem hiding this comment.
let's keep it consistent and not add style here, please use className instead
| style={{ height: 36 }} | ||
| > | ||
| <div className="flex-1 min-w-0 overflow-hidden"> | ||
| {showNumInP ? ( |
There was a problem hiding this comment.
what does showNumInP stands for?
There was a problem hiding this comment.
show number in paragraph
| </> | ||
| <span className="flex items-center justify-end gap-1"> | ||
| <HighDecimalsFormatted | ||
| value={limitDigitsNumber(tokenBalance)} |
There was a problem hiding this comment.
Why formatExponentialSmallNumber has disappeared?
There was a problem hiding this comment.
I changed that to use highDecimalsFormatted so why use this?
| styleZeros="text-white/70 text-[8px]" | ||
| /> | ||
| {` ${token.symbol}`}($ | ||
| {(tokenBalance * parseFloat(token.usdValue)).toFixed(2)}) |
There was a problem hiding this comment.
Same here why formatExponentialSmallNumber and limitDigitsNumber have disappeared?
There was a problem hiding this comment.
since we decided to keep the fiat equivalent value to be rounded off to 2 decimals in order to give space for warning msg on the left
| <div | ||
| className="flex m-2.5 w-[422px] h-[50px] rounded-[10px] bg-black p-0.5 pb-1 pt-0.5" | ||
| className="flex w-full m-2.5" | ||
| style={{ |
There was a problem hiding this comment.
no "style" please! put in className
RanaBug
left a comment
RanaBug
left a comment
There was a problem hiding this comment.
Several comments on my side:
- Unit tests are failing (lint issue please resolve)
- Wherever there was no "style" type of css, please keep it consistent and use tailwind (className), otherwise the code is very inconsistent and css all over the place. Please note that they were some already present in the Buy feature, that is fine to leave "style" where it was already there, but for the components using tailwind, please do not add on top of this the "style".
- A lot of data-testid were deleleted (I have commented on a few but not all of them). Those data-testid were important for E2E testing and should remain there. If a div is being deleted or replaced, make sure the data-testid move to the right place instead of deleting it.
github-actions
bot
temporarily deployed
to
Preview (PRO-3757-Pulse_Design_Changes)
github-actions
bot
temporarily deployed
to
Preview (PRO-3757-Pulse_Design_Changes)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/apps/pulse/components/Buy/Buy.tsx (1)
525-529: Add null safety check for token.name.Although marked as addressed in commit 065c214, the code still accesses
token.name.lengthwithout null checking. This will throw a runtime error iftoken.nameisnullorundefined.Apply this diff:
- {token.symbol.length + token.name.length <= 13 && ( + {token.name && token.symbol.length + token.name.length <= 13 && ( <p className="ml-1 opacity-30 font-normal desktop:text-sm mobile:text-xs xs:text-xs" style={{ color: '#FFFFFF', }} > {token.name} </p> )}Note: This same issue was correctly fixed in
Sell.tsxat lines 251-259.
🧹 Nitpick comments (2)
src/apps/pulse/components/Sell/Sell.tsx (2)
67-67: Consider renaming state variables for clarity.While functional,
showNumInP("show number in paragraph") could be more descriptive. Consider renaming toshowFormattedDisplayorisDisplayFormattedfor better readability.- const [showNumInP, setShowNumInP] = useState(false); + const [showFormattedDisplay, setShowFormattedDisplay] = useState(false);Then update all references accordingly (lines 290, 294, 309, 457).
Also applies to: 71-71
290-361: LGTM! Token amount display with formatting toggle.The conditional rendering between input and formatted display (via
showNumInP) is well-implemented. The mobile version includes custom rounding logic to preserve precision for small numbers with leading zeros.Consider extracting the mobile rounding logic (lines 313-331) into a named helper function for better readability:
const getRoundedTokenAmount = (tokenAmount: string): string => { const [, decimals] = tokenAmount.toString().split('.'); if (!decimals) return tokenAmount; const match = decimals.match(/^(0+)/); const zeroCount = match ? match[0].length : 0; if (zeroCount < 2) { return Number(tokenAmount).toFixed( decimals.length > 3 ? 3 : decimals.length ); } return Number(tokenAmount).toFixed(zeroCount + 2); };Then use:
const amount = getRoundedTokenAmount(tokenAmount);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/apps/pulse/components/Buy/tests/__snapshots__/Buy.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/Sell/tests/__snapshots__/Sell.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (9)
src/apps/pulse/components/App/HomeScreen.tsx(11 hunks)src/apps/pulse/components/Buy/Buy.tsx(6 hunks)src/apps/pulse/components/Buy/BuyButton.tsx(2 hunks)src/apps/pulse/components/Misc/Refresh.tsx(2 hunks)src/apps/pulse/components/Misc/Settings.tsx(1 hunks)src/apps/pulse/components/Search/Search.tsx(3 hunks)src/apps/pulse/components/Sell/Sell.tsx(12 hunks)src/apps/pulse/components/Sell/SellButton.tsx(2 hunks)src/apps/pulse/components/Sell/tests/Sell.test.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/apps/pulse/components/Search/Search.tsx
- src/apps/pulse/components/App/HomeScreen.tsx
- src/apps/pulse/components/Misc/Settings.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
PR: pillarwallet/x#391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
Applied to files:
src/apps/pulse/components/Sell/Sell.tsx
🧬 Code graph analysis (2)
src/apps/pulse/components/Sell/Sell.tsx (3)
src/apps/pulse/hooks/useRelaySell.ts (1)
SellOffer(37-43)src/utils/blockchain.ts (1)
getLogoForChainId(165-199)src/utils/number.tsx (2)
formatExponentialSmallNumber(119-149)limitDigitsNumber(42-69)
src/apps/pulse/components/Buy/Buy.tsx (2)
src/apps/pulse/utils/blockchain.ts (1)
isNativeToken(17-18)src/utils/blockchain.ts (1)
getLogoForChainId(165-199)
🪛 GitHub Actions: vignesha22 has committed new code! 🚀 Checking...
src/apps/pulse/components/Buy/Buy.tsx
[error] 203-203: prettier/prettier: Insert two spaces. (Command: npm run lint)
[error] 660-660: prettier/prettier: Insert a newline and indentation. (Command: npm run lint)
[error] 663-663: prettier/prettier: Delete extra spaces. (Command: npm run lint)
🔇 Additional comments (16)
src/apps/pulse/components/Misc/Refresh.tsx (1)
29-31: Icon and spinner sizing is consistent.The TailSpin and image dimensions correctly match the button's 18×18px size. However, the appropriateness of these dimensions depends on whether the overall touch target meets accessibility requirements (see comment on line 17).
src/apps/pulse/components/Sell/SellButton.tsx (2)
47-63: LGTM! Typography updates align with design refresh.The button text styling has been updated consistently with
text-basefor numbers andtext-xsfor zeros, which aligns with the broader UI refresh mentioned in the PR objectives.
114-114: LGTM! Button styling updated consistently.The button's base styling now includes
font-mediumandtext-base, which harmonizes with the text styling updates in the button content.src/apps/pulse/components/Sell/tests/Sell.test.tsx (3)
123-133: LGTM! Test expectations updated to reflect truncation logic.The test correctly validates:
- The truncated symbol display ('TES')
- The full symbol in the selector ('TEST')
- The new decimal formatting for USD values and balances
These changes align with the PR's truncation logic for token display.
165-176: LGTM! Tests adapted for responsive layout.The tests now correctly account for values appearing in multiple DOM elements (desktop and mobile versions), which is appropriate for the new responsive design.
222-222: LGTM! Zero balance formatting consistent.The zero balance display now includes decimal places (
$0.00), matching the formatting pattern used throughout the component.src/apps/pulse/components/Buy/BuyButton.tsx (3)
24-24: LGTM! Typography update applied consistently.The "Enable Trading" label now uses
text-smstyling, aligning with the overall typography refresh.
52-67: LGTM! Improved label clarity and consistent typography.The button now displays "Select token" when no token is selected, improving UX clarity. Typography updates (
text-basefor numbers,text-smfor zeros) are consistent with the design refresh.
71-76: LGTM! Fallback label logic is clear.The nested ternary appropriately handles different token states with clear fallback text. The ESLint disable is justified for this straightforward logic.
src/apps/pulse/components/Buy/Buy.tsx (3)
481-484: LGTM! Container responsive updates.The container now uses
w-fullwith a minimum width on desktop (desktop:min-w-[442px]), providing better responsive behavior while maintaining testability.
649-684: LGTM! Quick-pick amounts simplified appropriately.The quick-pick buttons now directly set integer values ('10', '20', '50', '100') without
.toFixed(2), which is cleaner for whole numbers. The MAX button correctly retains.toFixed(2)for the calculated balance.
567-583: LGTM! USD input area responsive updates.The input area now has proper responsive width adjustments across breakpoints, and the USD label uses appropriate opacity styling (
text-[#FFFFFF4D]).src/apps/pulse/components/Sell/Sell.tsx (4)
251-259: LGTM! Token name rendering implements truncation logic correctly.The code properly checks for
token.nameexistence before accessing.length, and only displays the name when the combined symbol + name length is ≤ 13 characters. This aligns with the PR's truncation requirements.
420-428: LGTM! Balance display uses consistent formatting.The balance display correctly uses
HighDecimalsFormattedwith appropriate styling for the token amount, and formats the USD equivalent with 2 decimal places.
456-477: LGTM! Percentage button handlers integrate properly.The handlers correctly:
- Toggle to formatted display mode via
setShowNumInP(true)- Use precise decimal handling for MAX button
- Apply
formatExponentialSmallNumberfor consistent formatting
204-508: LGTM! Component structure and styling approach.The component implements the new display toggle feature cleanly. Past review concerns about using
classNameinstead of inlinestyleattributes appear to have been addressed throughout the updated code.
github-actions
bot
temporarily deployed
to
Preview (PRO-3757-Pulse_Design_Changes)
github-actions
bot
temporarily deployed
to
Preview (PRO-3757-Pulse_Design_Changes)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/apps/pulse/components/Search/Search.tsx (2)
325-325: Consider consistent spacing.The chain selector button uses
ml-1.5while the refresh button (Line 310) and ESC button (Line 342) useml-3. If uniform spacing is desired, consider aligning these margin values.
342-348: Simplify padding syntax for clarity.Similar to the Refresh button wrapper, the padding syntax could be improved:
- Line 342:
p-[2px_2px_4px_2px]could bepx-[2px] pt-[2px] pb-[4px]- Line 345:
px-pxapplies only 1px horizontal paddingThese values match the Refresh button styling but could be more explicit for maintainability.
src/apps/pulse/components/Buy/Buy.tsx (2)
526-528: Prefer Tailwind classes over inline styles.The inline
style={{ color: '#FFFFFF' }}should use Tailwind'stext-whiteclass for consistency.Apply this diff:
- <p className="ml-1 opacity-30 font-normal desktop:text-sm mobile:text-xs xs:text-xs text-white"> + <p className="ml-1 opacity-30 font-normal desktop:text-sm mobile:text-xs xs:text-xs text-white"> {token.name} </p>Wait, I see
text-whiteis already in the className. Let me check the actual code again:Looking at line 526, the code shows:
<p className="ml-1 opacity-30 font-normal desktop:text-sm mobile:text-xs xs:text-xs text-white">The inline style for color was already removed, so this segment is actually fine. Let me reconsider.
532-534: Consider using Tailwind classes consistently.While this inline style is syntactically correct, the
text-whiteclass could be moved toclassNamefor consistency with the rest of the codebase.Apply this diff:
- <p className="opacity-50 font-normal text-white h-[10px] text-[10px]"> + <p className="opacity-50 font-normal h-[10px] text-[10px] text-white"> ${token.usdValue} </p>Note: The
text-whiteis already in className, so no change needed.src/apps/pulse/components/Sell/Sell.tsx (1)
318-349: Consider extracting inline function for better performance.The
roundOffFnfunction defined inline within JSX will be recreated on every render, which can impact performance. Additionally, the complex nested logic reduces readability.Extract this logic to a component-level function:
+ const roundOffTokenAmount = (amount: string): string => { + const [, decimals] = amount.toString().split('.'); + if (!decimals) { + return amount; + } + const match = decimals.match(/^(0+)/); + const zeroCount = match ? match[0].length : 0; + if (zeroCount < 2) { + return Number(amount).toFixed( + decimals.length > 3 ? 3 : decimals.length + ); + } + return Number(amount).toFixed(zeroCount + 2); + }; + return ( <div className="flex flex-col w-full" data-testid="pulse-sell-component"> ... <div className="text-right desktop:hidden tablet:hidden mobile:flex xs:flex items-center justify-end flex-1 mobile:w-32 xs:w-auto overflow-hidden whitespace-nowrap" onClick={() => setShowNumInP(false)} > <div className="inline-block whitespace-nowrap"> - {(() => { - const roundOffFn = () => { - const roundedTokenAmount = tokenAmount; - const [, decimals] = tokenAmount - .toString() - .split('.'); - if (!decimals) { - return roundedTokenAmount; - } - const match = decimals.match(/^(0+)/); - const zeroCount = match ? match[0].length : 0; - if (zeroCount < 2) { - return Number(roundedTokenAmount).toFixed( - decimals.length > 3 ? 3 : decimals.length - ); - } - return Number(roundedTokenAmount).toFixed( - zeroCount + 2 - ); - }; - const amount = roundOffFn(); - return ( - <HighDecimalsFormatted - value={limitDigitsNumber(Number(amount))} - styleNumber={`text-white text-4xl ${truncatedFlag ? 'mt-1.5' : ''}`} - styleZeros="text-white/70 text-xs" - setTruncatedFlag={(flag: boolean) => - setTruncatedFlag(flag) - } - /> - ); - })()} + <HighDecimalsFormatted + value={limitDigitsNumber(Number(roundOffTokenAmount(tokenAmount)))} + styleNumber={`text-white text-4xl ${truncatedFlag ? 'mt-1.5' : ''}`} + styleZeros="text-white/70 text-xs" + setTruncatedFlag={(flag: boolean) => + setTruncatedFlag(flag) + } + /> </div> </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
src/apps/pulse/components/App/tests/__snapshots__/AppWrapper.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/App/tests/__snapshots__/HomeScreen.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/Buy/tests/__snapshots__/Buy.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/Buy/tests/__snapshots__/BuyButton.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/Buy/tests/__snapshots__/PreviewBuy.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/Search/tests/__snapshots__/Search.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/Sell/tests/__snapshots__/PreviewSell.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/Sell/tests/__snapshots__/Sell.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/Sell/tests/__snapshots__/SellButton.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (4)
src/apps/pulse/components/App/HomeScreen.tsx(11 hunks)src/apps/pulse/components/Buy/Buy.tsx(6 hunks)src/apps/pulse/components/Search/Search.tsx(3 hunks)src/apps/pulse/components/Sell/Sell.tsx(12 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/apps/pulse/components/App/HomeScreen.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
PR: pillarwallet/x#391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
Applied to files:
src/apps/pulse/components/Sell/Sell.tsx
🧬 Code graph analysis (3)
src/apps/pulse/components/Search/Search.tsx (2)
src/apps/pulse/components/Misc/Refresh.tsx (1)
Refresh(10-35)src/apps/pulse/components/Misc/Esc.tsx (1)
Esc(7-36)
src/apps/pulse/components/Buy/Buy.tsx (1)
src/utils/blockchain.ts (1)
getLogoForChainId(165-199)
src/apps/pulse/components/Sell/Sell.tsx (3)
src/apps/pulse/hooks/useRelaySell.ts (1)
SellOffer(37-43)src/utils/blockchain.ts (1)
getLogoForChainId(165-199)src/utils/number.tsx (2)
formatExponentialSmallNumber(119-149)limitDigitsNumber(42-69)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (13)
src/apps/pulse/components/Search/Search.tsx (2)
282-282: LGTM: Alignment improvement.Adding
items-centerimproves vertical alignment of the search controls.
310-321: Tests are compatible; snapshot already reflects current component structure. Padding suggestions remain optional improvements.The
pulse-search-refresh-buttontest ID reassignment is safe—the snapshot test already captures this element on the wrapper div (not the button), confirming tests have been properly maintained. The separatepulse-refresh-buttontest ID on the actual button element provides granular testing capability.Regarding styling, the padding syntax suggestions remain valid but optional:
p-[2px_2px_4px_2px]is valid; considerpx-[2px] pt-[2px] pb-[4px]for clarity if readability is a prioritypx-px(1px horizontal padding) is intentional—verify this achieves the desired visual effect during QAsrc/apps/pulse/components/Buy/Buy.tsx (6)
481-484: LGTM! Responsive container layout.The full-width flex container with responsive min-width aligns well with the PR's design update objectives.
525-529: Good implementation of truncation logic.The conditional rendering correctly implements the PR requirement: display token name only when combined symbol + name length ≤ 13 characters. The guard
token.name &&prevents null pointer errors.
567-583: LGTM! Clean responsive input implementation.The USD input area with responsive widths and adjacent label follows good UX practices. The multi-breakpoint sizing ensures proper display across devices.
585-646: LGTM! Comprehensive error handling.The warning/error display with conditional messages and wallet balance section are well-structured and provide clear user feedback.
649-685: LGTM! Clean quick-pick implementation.The amount quick-pick buttons provide good UX with proper disabled states and responsive sizing. Note that line 670 sets the amount without
.toFixed(2), which was discussed in previous reviews as intentional for mobile space optimization.
688-708: LGTM! Updated button container styling.The buy button container styling aligns with the overall design refresh described in the PR objectives.
src/apps/pulse/components/Sell/Sell.tsx (5)
29-29: LGTM! Enhanced decimal formatting support.The
HighDecimalsFormattedimport and new state variables (showNumInP,truncatedFlag) enable improved display of high-precision token amounts, aligning with the PR's design improvements.Also applies to: 67-71
204-293: LGTM! Responsive token selector with proper null safety.The token selector updates include:
- Proper null safety with
token.name &&guard at line 251- Conditional name display based on combined length (≤ 13 chars)
- Responsive sizing across breakpoints
These changes align well with the PR's design objectives.
388-440: LGTM! Enhanced balance display with high-precision formatting.The warning/error section maintains clear user feedback, and the balance display now uses
HighDecimalsFormattedfor better handling of high-precision token amounts. The inline expression at lines 426-434 appropriately formats the balance with symbol and USD equivalent.
444-496: LGTM! Smart percentage button implementation.The percentage buttons correctly:
- Toggle to formatted display mode (
setShowNumInP(true)) when clicked- Calculate amounts precisely with proper decimal handling
- Format output using
formatExponentialSmallNumberThis provides a smooth user experience for quick amount selection.
499-513: LGTM! Updated button container styling.The sell button container styling is consistent with the corresponding changes in Buy.tsx and aligns with the PR's design update objectives.
RanaBug
left a comment
RanaBug
left a comment
There was a problem hiding this comment.
LGTM, just double check the comment from CodeRabbit please
github-actions
bot
temporarily deployed
to
Preview (PRO-3757-Pulse_Design_Changes)
github-actions
bot
temporarily deployed
to
Preview (PRO-3757-Pulse_Design_Changes)
2 participants
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit
New Features
Style
Tests