Refactor compose bottom sheets into fragments#6240
Conversation
SeniorZhai
force-pushed
the
feature/trde_guide
branch
3 times, most recently
from
88c81a4 to
26c5b84
Compare
# Conflicts: # app/src/main/java/one/mixin/android/ui/address/page/TransferDestinationInputPage.kt # app/src/main/java/one/mixin/android/ui/home/web3/trade/TradePage.kt # app/src/main/java/one/mixin/android/ui/wallet/alert/AlertEditPage.kt # app/src/main/java/one/mixin/android/ui/wallet/alert/AlertFragment.kt
SeniorZhai
force-pushed
the
refactor/modal-bottom-sheets
branch
from
5a7d537 to
a9b9d0f
Compare
There was a problem hiding this comment.
Pull request overview
Refactors parts of the Web3 trading/help UI and token-selection UI to use fragment-based bottom sheets (instead of Compose ModalBottomSheetLayout), aligning behavior with the app’s existing bottom sheet fragment patterns.
Changes:
- Converted the Trade help bottom sheet from an in-Compose
ModalBottomSheetLayoutto a newTradeHelpBottomSheetDialogFragment. - Renamed/repurposed the token chooser bottom sheet fragment to
DepositTokensBottomSheetDialogFragmentand updated call sites. - Updated the choose-tokens bottom sheet layout to support full-height bottom sheet presentation with an inset spacer (
ph) and weighted content.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| app/src/main/res/layout/fragment_choose_tokens_bottom_sheet.xml | Restructures layout to full-height sheet with top placeholder spacer and weighted list content. |
| app/src/main/java/one/mixin/android/ui/wallet/MarketDetailsFragment.kt | Switches token chooser bottom sheet usage to DepositTokensBottomSheetDialogFragment. |
| app/src/main/java/one/mixin/android/ui/home/web3/trade/TradePage.kt | Removes Compose modal bottom sheet and delegates showing help sheet to host via callback. |
| app/src/main/java/one/mixin/android/ui/home/web3/trade/TradeHelpBottomSheetDialogFragment.kt | Adds new fragment-based Compose bottom sheet for Trade help actions. |
| app/src/main/java/one/mixin/android/ui/home/web3/trade/TradeFragment.kt | Wires TradePage help action to show the new TradeHelpBottomSheetDialogFragment. |
| app/src/main/java/one/mixin/android/ui/home/web3/market/DepositTokensBottomSheetDialogFragment.kt | Renames/refactors chooser fragment; adds inset spacer sizing logic for the ph view. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| override fun ComposeContent() { | ||
| MixinAppTheme { | ||
| HelpBottomSheetContent( | ||
| onContactSupport = { | ||
| dismiss() | ||
| onContactSupport?.invoke() | ||
| }, | ||
| onTradingGuide = { | ||
| dismiss() | ||
| onTradingGuide?.invoke() | ||
| }, | ||
| onDismiss = { dismiss() } | ||
| ) |
There was a problem hiding this comment.
HelpBottomSheetContent supports a guideTitle parameter, but this fragment always uses the default value. Before the refactor the title was chosen based on whether the user is on Spot vs Perpetual. Add a fragment argument/property for the computed title (and pass it into HelpBottomSheetContent(guideTitle = ...)) to preserve the previous UX.
| onShowHelpBottomSheet = { onContactSupport, onTradingGuide -> | ||
| this@apply.hideKeyboard() | ||
| TradeHelpBottomSheetDialogFragment.newInstance().apply { | ||
| this.onContactSupport = onContactSupport | ||
| this.onTradingGuide = onTradingGuide | ||
| }.show(parentFragmentManager, TradeHelpBottomSheetDialogFragment.TAG) | ||
| }, |
There was a problem hiding this comment.
This shows TradeHelpBottomSheetDialogFragment but doesn't provide the Spot/Perpetual-specific guide title that the previous Compose bottom sheet computed. If you add an argument/property on TradeHelpBottomSheetDialogFragment for guideTitle, pass it here when constructing the fragment so the sheet can render the correct option label.
| IconButton(onClick = { | ||
| coroutineScope.launch { | ||
| bottomSheetState.show() | ||
| } | ||
| onShowHelpBottomSheet( | ||
| { context.openUrl(Constants.HelpLink.CUSTOMER_SERVICE) }, | ||
| { onShowTradingGuide(pagerState.currentPage) } | ||
| ) |
There was a problem hiding this comment.
The help bottom sheet used to show a context-specific guide title (Spot vs Perpetual). With the fragment refactor, the click handler no longer provides a guideTitle, so HelpBottomSheetContent will fall back to the generic Trading_Guide string and loses the previous behavior. Consider passing the computed title (or an enum/tab type) into onShowHelpBottomSheet so the fragment can render the same title as before.
No description provided.