Conversation
There was a problem hiding this comment.
Pull request overview
This PR refines the wallet price-alert UI flow by renaming the “All Alerts” screen to “Price Alerts” and simplifying navigation to start directly on the aggregated alerts list (removing the per-asset AlertPage screen).
Changes:
- Replace the
All_Alertstring with a newPrice_Alertsstring (EN + zh-CN) and update the screen title usage. - Remove
AlertPageand its navigation route; startAlertFragmenton theAlldestination instead. - Initialize the alerts list filter (
coins) from the passed-incoin.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| app/src/main/res/values/strings.xml | Rename/replace the alerts screen title string to “Price Alerts”. |
| app/src/main/res/values-zh-rCN/strings.xml | zh-CN translation update for the new “Price Alerts” string key. |
| app/src/main/java/one/mixin/android/ui/wallet/alert/AllAlertPage.kt | Update PageScaffold title to use R.string.Price_Alerts. |
| app/src/main/java/one/mixin/android/ui/wallet/alert/AlertPage.kt | Remove the per-asset alert page implementation. |
| app/src/main/java/one/mixin/android/ui/wallet/alert/AlertFragment.kt | Remove Alert destination and switch start destination to All; set initial coins filter. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| enum class AlertDestination { | ||
| Alert, All, Edit, | ||
| All, Edit, | ||
| } |
There was a problem hiding this comment.
Removing the Alert destination from the NavHost can crash state restoration (e.g., process recreation or app update) if a previously-saved back stack contains the old "Alert" route; Navigation will try to restore a destination that no longer exists. Consider keeping an Alert route that redirects to All (or keeping the enum value + composable) for backward compatibility, at least for one release.
| override fun onViewCreated(view: View, savedInstanceState: Bundle?) { | ||
| super.onViewCreated(view, savedInstanceState) | ||
| coins = setOf(coin) |
There was a problem hiding this comment.
coins is initialized to emptySet() for the first composition and then set to setOf(coin) in onViewCreated(), which can briefly start collecting all alert groups (and potentially render an unfiltered UI) before switching to the single-coin filtered flow. Initialize coins before setContent (e.g., in onCreate, or set the initial state from args) to avoid unnecessary collection/flicker.
| override fun onViewCreated(view: View, savedInstanceState: Bundle?) { | |
| super.onViewCreated(view, savedInstanceState) | |
| coins = setOf(coin) | |
| override fun onCreate(savedInstanceState: Bundle?) { | |
| super.onCreate(savedInstanceState) | |
| coins = setOf(coin) | |
| } | |
| override fun onViewCreated(view: View, savedInstanceState: Bundle?) { | |
| super.onViewCreated(view, savedInstanceState) |
d23e842 to
3e99318
Compare
No description provided.