Feature/trade guide#6227
Conversation
9ecee6a to
9fd9840
Compare
Target files
1. Candle zoom behavior
2. Delayed candle detail on press (
|
|
@SeniorZhai I've opened a new pull request, #6228, to work on those changes. Once the pull request is ready, I'll request review from you. |
b2c2d44 to
7c413f4
Compare
2b12cc2 to
e08b485
Compare
bc7b411 to
88c81a4
Compare
…t pricing Add pinch zoom to candle chart
88c81a4 to
26c5b84
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds new wallet onboarding/marketing UI (import/watch guidance + free-transfer promo) and introduces a Spot Trading Guide bottom sheet, alongside some related UI behavior tweaks (wallet tabs ordering, candle chart gestures).
Changes:
- Add new localized strings/URLs and assets for wallet import/watch guidance and free-transfer promo.
- Extend wallet UI to show empty-state guide cards for Import/Watch categories and add a dismissible “free transfers” card in the add-wallet bottom sheet.
- Add Spot Trading Guide (Compose) and update Trade page/fragment plumbing to open the correct guide per tab; enhance candle chart with pinch/pan scaling.
Reviewed changes
Copilot reviewed 15 out of 23 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| app/src/main/res/values/strings.xml | Adds new English strings/URLs for wallet import/watch guides and trade guide copy; updates free-transfer campaign URL. |
| app/src/main/res/values-zh-rCN/strings.xml | Adds new Simplified Chinese counterparts; updates free-transfer campaign URL. |
| app/src/main/res/layout/fragment_add_wallet_bottom_sheet.xml | Makes add-wallet sheet scrollable and adds promo + “safe to import” content block. |
| app/src/main/res/drawable/ic_import_wallet.xml | Adds import-wallet vector icon. |
| app/src/main/res/drawable/ic_dot_assist.xml | Adds themed dot drawable used for bullet points. |
| app/src/main/res/drawable/ic_add_watch_wallet.xml | Adds watch-wallet vector icon. |
| app/src/main/res/drawable/ic_add_wallet_freee.xml | Adds promo icon used in add-wallet sheet. |
| app/src/main/res/drawable-xxhdpi/mixin_import_safety_preview_0.png | Adds import-safety preview image asset. |
| app/src/main/res/drawable-xxhdpi/mixin_import_safety_preview_1.png | Adds import-safety preview image asset. |
| app/src/main/res/drawable-xxhdpi/mixin_import_safety_preview_2.png | Adds import-safety preview image asset. |
| app/src/main/res/drawable-xxhdpi/mixin_import_safety_preview_3.png | Adds import-safety preview image asset. |
| app/src/main/res/drawable-xxhdpi/mixin_import_safety_preview_4.png | Adds import-safety preview image asset. |
| app/src/main/res/drawable-xxhdpi/mixin_import_safety_preview_5.png | Adds import-safety preview image asset. |
| app/src/main/res/drawable-xxhdpi/mixin_import_safety_preview_6.png | Adds import-safety preview image asset. |
| app/src/main/res/drawable-xxhdpi/mixin_import_safety_preview_7.png | Adds import-safety preview image asset (currently unused in the shown layout). |
| app/src/main/java/one/mixin/android/ui/wallet/components/WalletCategoryFilter.kt | Reorders the Safe tab placement in the category filter. |
| app/src/main/java/one/mixin/android/ui/wallet/components/AssetDashboardScreen.kt | Adds empty-state guide cards and refactors wallet filtering; introduces dynamic resource lookup helpers. |
| app/src/main/java/one/mixin/android/ui/wallet/AddWalletBottomSheetDialogFragment.kt | Adds promo-card dismissal persistence and click handling to open campaign docs. |
| app/src/main/java/one/mixin/android/ui/home/web3/trade/TradePage.kt | Changes trading-guide callback to include tab index and routes guide launches accordingly. |
| app/src/main/java/one/mixin/android/ui/home/web3/trade/TradeFragment.kt | Launches Spot vs Perpetual guide bottom sheets based on current tab/wallet context. |
| app/src/main/java/one/mixin/android/ui/home/web3/trade/SpotTradeGuidePage.kt | New Compose UI for Spot trade guide with tabs and interactive examples. |
| app/src/main/java/one/mixin/android/ui/home/web3/trade/SpotTradeGuideBottomSheetDialogFragment.kt | New full-height compose bottom sheet container for the Spot trade guide. |
| app/src/main/java/one/mixin/android/ui/home/web3/trade/CandleChart.kt | Adds transform gestures + scaling support to the candle chart. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private fun Context.stringByName(name: String): String { | ||
| val resourceId = resources.getIdentifier(name, "string", packageName) | ||
| return if (resourceId != 0) getString(resourceId) else "" | ||
| } | ||
|
|
||
| private fun Context.drawableByName(name: String): Int? { | ||
| return resources.getIdentifier(name, "drawable", packageName) | ||
| .takeIf { it != 0 } | ||
| } |
There was a problem hiding this comment.
stringByName()/drawableByName() rely on Resources.getIdentifier, which is invisible to Android's resource shrinker. Since shrinkResources true is enabled (app/build.gradle:154), these strings/drawables can be removed in release builds, causing empty text / missing icons at runtime. Prefer referencing resources via R.string.* / R.drawable.* (or add an explicit keep rule for these resources if dynamic lookup is truly required).
| @@ -253,7 +311,7 @@ private fun ScrollableCandleChart( | |||
| } | |||
| ) | |||
| } | |||
| .horizontalScroll(scrollState, enabled = !isTouching) | |||
| .horizontalScroll(scrollState, enabled = !isTouching && !isPinching) | |||
| .clipToBounds() | |||
There was a problem hiding this comment.
isPinching is set to true inside detectTransformGestures, but it is never reset to false. After the first pan/zoom gesture, the long-press crosshair (pointerInput below) and horizontalScroll remain disabled permanently (enabled = !isTouching && !isPinching). Reset the flag when the transform gesture ends/cancels (or switch to transformable and use its in-progress state) so other interactions keep working.
| <HorizontalScrollView | ||
| android:layout_width="match_parent" | ||
| android:layout_height="wrap_content" | ||
| android:layout_marginTop="24dp" | ||
| android:fillViewport="true" | ||
| android:scrollbars="none"> | ||
|
|
||
| <FrameLayout | ||
| android:layout_width="match_parent" | ||
| android:layout_height="wrap_content"> | ||
|
|
||
| <LinearLayout | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:layout_gravity="center_horizontal" | ||
| android:orientation="horizontal"> |
There was a problem hiding this comment.
This HorizontalScrollView won't scroll as intended because its direct child FrameLayout is layout_width="match_parent". That constrains the inner LinearLayout and prevents it from becoming wider than the viewport. Use a wrap_content direct child (or make the FrameLayout wrap_content) so the preview images can overflow horizontally and actually scroll.
| android:layout_alignParentEnd="true" | ||
| android:layout_centerVertical="true" | ||
| android:layout_marginEnd="36dp" | ||
| android:src="@drawable/ic_add_wallet_freee" |
There was a problem hiding this comment.
The drawable name ic_add_wallet_freee looks like a typo (extra 'e'), which makes the resource harder to discover/maintain and inconsistent with other ic_* names. Consider renaming to ic_add_wallet_free (and updating references) before this name spreads further.
| android:src="@drawable/ic_add_wallet_freee" | |
| android:src="@drawable/ic_add_wallet_free" |
| "1 USDT ≈ ${inverted.numberFormat8()} BTC" | ||
| } else { | ||
| "1 BTC ≈ ${marketPrice.priceFormat()} USDT" |
There was a problem hiding this comment.
This file hard-codes several user-visible strings (e.g. "->" and "1 BTC ≈ … USDT" / "1 USDT ≈ … BTC"). These won't be translated and will ignore locale-specific formatting rules. Please move these to string resources with placeholders and use stringResource(...) (and consider formatting numbers via locale-aware APIs where appropriate).
| "1 USDT ≈ ${inverted.numberFormat8()} BTC" | |
| } else { | |
| "1 BTC ≈ ${marketPrice.priceFormat()} USDT" | |
| stringResource( | |
| id = R.string.spot_trade_price_1_usdt_approx_btc, | |
| inverted.numberFormat8(), | |
| ) | |
| } else { | |
| stringResource( | |
| id = R.string.spot_trade_price_1_btc_approx_usdt, | |
| marketPrice.priceFormat(), | |
| ) |
| val usdtPrice = usdtToken?.priceUsd?.toBigDecimalOrNull()?.takeIf { it > BigDecimal.ZERO } ?: BigDecimal.ONE | ||
| val btcPrice = btcToken?.priceUsd?.toBigDecimalOrNull()?.takeIf { it > BigDecimal.ZERO } ?: BigDecimal("95594.89") |
There was a problem hiding this comment.
calculateMarketPrice() falls back to a hard-coded BTC price ("95594.89") when price data is missing. This value will quickly become stale and can mislead users in the guide if pricing fetch fails. Prefer showing a placeholder/disabled state until prices load, or derive a neutral fallback (e.g., BigDecimal.ZERO/ONE) and render accordingly.
| val usdtPrice = usdtToken?.priceUsd?.toBigDecimalOrNull()?.takeIf { it > BigDecimal.ZERO } ?: BigDecimal.ONE | |
| val btcPrice = btcToken?.priceUsd?.toBigDecimalOrNull()?.takeIf { it > BigDecimal.ZERO } ?: BigDecimal("95594.89") | |
| val usdtPrice = usdtToken.safePrice() | |
| val btcPrice = btcToken.safePrice() |
| bindOptionalView("free_transfer_card") { | ||
| openFreeTransferDoc() | ||
| } | ||
| bindOptionalView("free_transfer_learn_more") { | ||
| openFreeTransferDoc() | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private fun bindOptionalView( | ||
| idName: String, | ||
| onClick: () -> Unit, | ||
| ) { | ||
| val id = binding.root.resources.getIdentifier(idName, "id", requireContext().packageName) | ||
| if (id == 0) return | ||
| binding.root.findViewById<View>(id)?.setOnClickListener { onClick() } | ||
| } |
There was a problem hiding this comment.
bindOptionalView() uses resources.getIdentifier + findViewById to attach click listeners to views that already exist in this binding (freeTransferCard, freeTransferLearnMore). This adds indirection and makes refactors (renaming IDs) easier to break silently. Prefer using the generated binding properties directly, or switch to binding.root.findViewById(R.id.free_transfer_learn_more) if you truly need optional lookup.
- Add explicit elevation=0.dp to all Button that didn't have it - Create MixinButton reusable component in MaterialButton.kt Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
# Conflicts: # app/src/main/res/values-zh-rCN/strings.xml # app/src/main/res/values/strings.xml
7679ea7 to
add5726
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 31 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| val importWalletTitle = context.stringByName("import_wallet_title") | ||
| val importWalletDescription = context.stringByName("import_wallet_empty_description") | ||
| val importWalletGuideUrl = context.stringByName("import_wallet_guide_url") | ||
| val watchWalletDescription = context.stringByName("watch_wallet_empty_description") | ||
| val watchWalletGuideUrl = context.stringByName("watch_wallet_guide_url") | ||
| val importWalletIconRes = context.drawableByName("ic_import_wallet") | ||
| val watchWalletIconRes = context.drawableByName("ic_add_watch_wallet") |
There was a problem hiding this comment.
stringByName("import_wallet_title" / "import_wallet_empty_description" / "import_wallet_guide_url" / "watch_wallet_empty_description" / "watch_wallet_guide_url") currently resolves to an empty string because these string resources don’t exist in the project, so the empty-state guide cards will render blank text and attempt to open an empty URL. Please add the missing string resources (all locales) and reference them via stringResource(...) (or at least fail fast when getIdentifier returns 0) so this can’t silently ship with empty content.
| <string name="Trading_Guide">Spot Trading Guide</string> | ||
| <string name="Overview">Overview</string> |
There was a problem hiding this comment.
Trading_Guide is used as the label in HelpBottomSheetContent (shown from both spot and perpetual contexts). Changing it to “Spot Trading Guide” will make the help option misleading when the current page is perpetual. Consider keeping Trading_Guide generic (e.g. “Trading Guide”) and introducing a separate spot-specific title string for the spot guide page, or make the help sheet title depend on the current tab.
| <string name="wallet_summary_tip_all">Safe 金库、观察钱包和隐藏的资产不计入统计</string> | ||
| <string name="wallet_summary_tip_safe">共管的 Safe 金库不计入统计</string> | ||
| <string name="Trading_Guide">交易说明</string> | ||
| <string name="Trading_Guide">现货交易指南</string> |
There was a problem hiding this comment.
Trading_Guide is also used as the help-sheet option title from both spot and perpetual contexts. Translating it specifically as “现货交易指南” will be misleading when the current page is perpetual. Please keep Trading_Guide as a generic “交易指南/交易说明” equivalent and add a separate spot-specific string for the spot guide title if needed.
| <string name="Trading_Guide">现货交易指南</string> | |
| <string name="Trading_Guide">交易指南</string> | |
| <string name="Spot_Trading_Guide">现货交易指南</string> |
| private fun StepperButton( | ||
| text: String, | ||
| enabled: Boolean, | ||
| onClick: () -> Unit, | ||
| ) { | ||
| Surface( | ||
| color = if (enabled) Color.Transparent else MixinAppTheme.colors.backgroundWindow, | ||
| shape = CircleShape, | ||
| border = BorderStroke(1.dp, MixinAppTheme.colors.borderColor), | ||
| modifier = Modifier | ||
| .size(24.dp) | ||
| .clickable(enabled = enabled, onClick = onClick), | ||
| ) { | ||
| Box(contentAlignment = Alignment.Center) { | ||
| Text( | ||
| text = text, | ||
| fontSize = 14.sp, | ||
| color = if (enabled) MixinAppTheme.colors.textPrimary else MixinAppTheme.colors.textAssist, | ||
| ) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @Composable |
There was a problem hiding this comment.
StepperButton is defined but never used in this file, so it will generate an unused-function warning and adds maintenance overhead. Please remove it or wire it into the UI (e.g., use it in AmountStepper / OrderPriceStepper) so the source of truth for stepper styling isn’t duplicated.
| private fun StepperButton( | |
| text: String, | |
| enabled: Boolean, | |
| onClick: () -> Unit, | |
| ) { | |
| Surface( | |
| color = if (enabled) Color.Transparent else MixinAppTheme.colors.backgroundWindow, | |
| shape = CircleShape, | |
| border = BorderStroke(1.dp, MixinAppTheme.colors.borderColor), | |
| modifier = Modifier | |
| .size(24.dp) | |
| .clickable(enabled = enabled, onClick = onClick), | |
| ) { | |
| Box(contentAlignment = Alignment.Center) { | |
| Text( | |
| text = text, | |
| fontSize = 14.sp, | |
| color = if (enabled) MixinAppTheme.colors.textPrimary else MixinAppTheme.colors.textAssist, | |
| ) | |
| } | |
| } | |
| } | |
| @Composable |
| "1 USDT ≈ ${inverted.numberFormat8()} BTC" | ||
| } else { | ||
| "1 BTC ≈ ${marketPrice.priceFormat()} USDT" |
There was a problem hiding this comment.
PriceSubtitle builds user-visible text with hardcoded English strings (e.g. "1 USDT ≈ ... BTC" / "1 BTC ≈ ... USDT"). This bypasses localization and makes translation impossible for non-English locales. Please move these to strings.xml with proper formatting arguments and use stringResource(...) here.
| "1 USDT ≈ ${inverted.numberFormat8()} BTC" | |
| } else { | |
| "1 BTC ≈ ${marketPrice.priceFormat()} USDT" | |
| stringResource( | |
| id = R.string.spot_trade_price_subtitle_usdt_btc, | |
| inverted.numberFormat8(), | |
| ) | |
| } else { | |
| stringResource( | |
| id = R.string.spot_trade_price_subtitle_btc_usdt, | |
| marketPrice.priceFormat(), | |
| ) |
e9db566 to
9543956
Compare
This reverts commit 4b8d482.
No description provided.