-
Notifications
You must be signed in to change notification settings - Fork 439
Fix/perp share interface #8998
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
base: x
Are you sure you want to change the base?
Fix/perp share interface #8998
Conversation
…d add exit price translation support
…improved UI consistency
…with detailed service fee display and protocol fee comparison
…ions for multiple languages
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
WalkthroughThis pull request updates loading states and UI styling across Perp order book and position views, refactors position share rendering with responsive layouts and localization, introduces a protocol fee comparison component in Swap views, and adds corresponding translation enum members. Changes
Sequence Diagram(s)sequenceDiagram
participant PreSwapInfoGroup
participant PreSwapInfoItem
participant ProtocolFeeComparisonList
PreSwapInfoGroup->>PreSwapInfoGroup: Extract serviceFee from preSwapData
PreSwapInfoGroup->>PreSwapInfoItem: Pass popoverContent (ReactNode: Stack + ProtocolFeeComparisonList)
PreSwapInfoItem->>PreSwapInfoItem: Check if popoverContent is string or ReactNode
alt String content
PreSwapInfoItem->>PreSwapInfoItem: Render as SizableText
else ReactNode content
PreSwapInfoItem->>ProtocolFeeComparisonList: Render ReactNode directly
ProtocolFeeComparisonList->>ProtocolFeeComparisonList: Merge otherWalletFeeData with oneKey entry
ProtocolFeeComparisonList->>ProtocolFeeComparisonList: Calculate progress bar widths (fee/maxFee)
ProtocolFeeComparisonList->>ProtocolFeeComparisonList: Render list with icons and fee percentages
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas requiring attention:
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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 |
…nd adjust token spacing in canvas configuration
fcdb935 to
85ee7dd
Compare
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: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (19)
packages/shared/src/locale/json/bn.jsonis excluded by!packages/shared/src/locale/json/*.jsonpackages/shared/src/locale/json/de.jsonis excluded by!packages/shared/src/locale/json/*.jsonpackages/shared/src/locale/json/en_US.jsonis excluded by!packages/shared/src/locale/json/*.jsonpackages/shared/src/locale/json/es.jsonis excluded by!packages/shared/src/locale/json/*.jsonpackages/shared/src/locale/json/fr_FR.jsonis excluded by!packages/shared/src/locale/json/*.jsonpackages/shared/src/locale/json/hi_IN.jsonis excluded by!packages/shared/src/locale/json/*.jsonpackages/shared/src/locale/json/id.jsonis excluded by!packages/shared/src/locale/json/*.jsonpackages/shared/src/locale/json/it_IT.jsonis excluded by!packages/shared/src/locale/json/*.jsonpackages/shared/src/locale/json/ja_JP.jsonis excluded by!packages/shared/src/locale/json/*.jsonpackages/shared/src/locale/json/ko_KR.jsonis excluded by!packages/shared/src/locale/json/*.jsonpackages/shared/src/locale/json/pt.jsonis excluded by!packages/shared/src/locale/json/*.jsonpackages/shared/src/locale/json/pt_BR.jsonis excluded by!packages/shared/src/locale/json/*.jsonpackages/shared/src/locale/json/ru.jsonis excluded by!packages/shared/src/locale/json/*.jsonpackages/shared/src/locale/json/th_TH.jsonis excluded by!packages/shared/src/locale/json/*.jsonpackages/shared/src/locale/json/uk_UA.jsonis excluded by!packages/shared/src/locale/json/*.jsonpackages/shared/src/locale/json/vi.jsonis excluded by!packages/shared/src/locale/json/*.jsonpackages/shared/src/locale/json/zh_CN.jsonis excluded by!packages/shared/src/locale/json/*.jsonpackages/shared/src/locale/json/zh_HK.jsonis excluded by!packages/shared/src/locale/json/*.jsonpackages/shared/src/locale/json/zh_TW.jsonis excluded by!packages/shared/src/locale/json/*.json
📒 Files selected for processing (12)
packages/kit/src/views/Perp/components/OrderBook/DefaultLoadingNode.tsx(1 hunks)packages/kit/src/views/Perp/components/OrderInfoPanel/Components/PositionsRow.tsx(2 hunks)packages/kit/src/views/Perp/components/OrderInfoPanel/List/CommonTableListView.tsx(2 hunks)packages/kit/src/views/Perp/components/PerpOrderBook.tsx(1 hunks)packages/kit/src/views/Perp/components/PositionShare/ShareContentRenderer.tsx(6 hunks)packages/kit/src/views/Perp/components/PositionShare/ShareImageGenerator.tsx(3 hunks)packages/kit/src/views/Perp/components/PositionShare/constants.ts(2 hunks)packages/kit/src/views/Swap/components/PreSwapInfoGroup.tsx(4 hunks)packages/kit/src/views/Swap/components/PreSwapInfoItem.tsx(2 hunks)packages/kit/src/views/Swap/components/ProtocolFeeComparisonList.tsx(1 hunks)packages/kit/src/views/Swap/components/SwapServiceFeeOverview.tsx(2 hunks)packages/shared/src/locale/enum/translations.ts(2 hunks)
🔇 Additional comments (13)
packages/kit/src/views/Perp/components/OrderInfoPanel/Components/PositionsRow.tsx (1)
380-380: LGTM! Icon size adjustment for visual consistency.The share button icon size reduced from
$4to$3.5in both desktop and mobile position rows. This is a minor UI refinement that improves visual balance.Also applies to: 737-737
packages/kit/src/views/Perp/components/PerpOrderBook.tsx (1)
93-93: LGTM! Loading skeleton adjusted for better visual fit.The skeleton width increased (120→180) and height decreased slightly (16→14), with rounded corners added. This aligns with the broader loading UI improvements across the PR.
packages/kit/src/views/Perp/components/OrderBook/DefaultLoadingNode.tsx (1)
178-183: LGTM! Simplified loading state is cleaner.Replaced dynamic array mapping with 4 explicit skeletons (100%, 80%, 60%, 40% widths). The fixed sequence is more maintainable and aligns with the broader shift toward simplified loading indicators across Perp views.
Minor note: The
MOBILE_ROWSconstant (lines 58-76) is still defined but no longer used in this variant. Consider removing if it's not used elsewhere.packages/kit/src/views/Perp/components/OrderInfoPanel/List/CommonTableListView.tsx (1)
21-21: LGTM! Cleaner loading state with centered spinner.Replaced multiple skeleton placeholders with a single centered
Spinner size="large". This simplifies the desktop list loading UI and aligns with the broader pattern of streamlined loading indicators across Perp views.Also applies to: 562-567
packages/kit/src/views/Perp/components/PositionShare/constants.ts (1)
109-141: LGTM! Visual refinements improve share image readability.Multiple constants updated for better typography and spacing:
- Font sizes increased (side: 24→28, price labels/values: 25→28)
- Spacing expanded (priceGap: 1.5→8, priceSpacingY: 40→50, tokenSpacing: 40→48)
- Colors adjusted for better contrast (long green less bright, backgrounds darker, referral background more opaque)
The
priceGapincrease from 1.5 to 8 is the most significant change—verify that the larger gap between price labels and values looks good in the share images.packages/kit/src/views/Swap/components/SwapServiceFeeOverview.tsx (1)
6-6: LGTM! Good refactor to dedicated component.Extracted protocol fee list rendering to
ProtocolFeeComparisonList. This improves code organization and reusability. TheserviceFeecalculation remains local and is passed as a prop.Also applies to: 48-48
packages/kit/src/views/Perp/components/PositionShare/ShareContentRenderer.tsx (4)
90-90: LGTM! Less aggressive PnL font scaling.Changed the scaling factor from 0.08 to 0.06 for long PnL text. This makes the font size reduction more gradual and maintains better readability.
231-231: LGTM! Gap scaling now responsive.Both price display containers now multiply
layout.priceGapbyscale, making the gap responsive to different image sizes. This works in tandem with the increasedpriceGapconstant (1.5→8) defined in constants.ts.Also applies to: 263-263
273-275: LGTM! Exit price label now localized.Replaced hardcoded "Exit Price" string with
ETranslations.perp_position_exit_price. Good i18n practice.
320-331: LGTM! Referral label text re-enabled with localization.The referral section now renders the localized "Referral Link" label above the URL. Gap increased to use the responsive
layout.priceGap * scalepattern. This improves clarity and consistency with other price displays.packages/kit/src/views/Perp/components/PositionShare/ShareImageGenerator.tsx (2)
240-247: Good: localized exit price label.Switching to ETranslations.perp_position_exit_price aligns with i18n usage.
311-317: Good: show localized referral link label.Matches existing ETranslations.referral_referral_link usage.
packages/shared/src/locale/enum/translations.ts (1)
2246-2246: Verify locale translations were synced for new keys.The enum keys are defined (lines 2246, 2591) and in-use, but locale JSON files don't exist in the repo—they're generated via
yarn fetch:locale. Before merging, confirm these new keys exist in your external translation source for all 21 supported locales: bn, de, en, en-US, es, fr-FR, hi-IN, id, it-IT, ja-JP, ko-KR, pt, pt-BR, ru, th-TH, uk-UA, vi, zh-CN, zh-HK, zh-TW.
Summary by CodeRabbit
New Features
Improvements