Skip to content

Unwrap MetaFeatureSettingsScreen#131

Merged
Goooler merged 11 commits into
trunkfrom
unwrap-meta-feature-settings-design
Apr 24, 2026
Merged

Unwrap MetaFeatureSettingsScreen#131
Goooler merged 11 commits into
trunkfrom
unwrap-meta-feature-settings-design

Conversation

@Goooler
Copy link
Copy Markdown
Owner

@Goooler Goooler commented Apr 23, 2026

Refs #119.

@Goooler Goooler force-pushed the unwrap-meta-feature-settings-design branch 2 times, most recently from 825caf1 to 6d6984b Compare April 24, 2026 07:19
@Goooler Goooler force-pushed the unwrap-meta-feature-settings-design branch from 6d6984b to 8d1df82 Compare April 24, 2026 07:23
@Goooler Goooler requested a review from Copilot April 24, 2026 07:24

This comment was marked as resolved.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@Goooler
Copy link
Copy Markdown
Owner Author

Goooler commented Apr 24, 2026

@copilot apply changes based on the comments in this thread

- Wrap importGeoFile() body in try/catch, setting importResult to
  Failed on any exception so the UI is never left in a stale state
- Use uiState.update { ... } in updateState() for atomic read-modify-write

Agent-Logs-Url: https://github.com/Goooler/MihomoForAndroid/sessions/ae75f819-f7f0-4f0a-a5fb-374c5b0bfe06

Co-authored-by: Goooler <10363352+Goooler@users.noreply.github.com>
Copy link
Copy Markdown

Copilot AI commented Apr 24, 2026

@copilot apply changes based on the comments in this thread

Applied both remaining unresolved comments in 534b402:

  1. importGeoFile() exception handling — wrapped the entire coroutine body in try/catch, so any exception from content resolver queries, stream opens, or file I/O sets importResult to Failed instead of leaving it stale.
  2. updateState() atomicity — replaced uiState.value = transform(uiState.value) with uiState.update(transform) (imported kotlinx.coroutines.flow.update) to make the read-modify-write atomic, matching the pattern used in NetworkSettingsViewModel.

This comment was marked as resolved.

@Goooler Goooler force-pushed the unwrap-meta-feature-settings-design branch from bb611a7 to 46e38f2 Compare April 24, 2026 07:54

This comment was marked as resolved.

Goooler and others added 3 commits April 24, 2026 16:02
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

This comment was marked as resolved.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR continues the “Design → @Composable + ViewModel” refactor for Meta feature settings (refs #119), moving state/persistence and geo-file import logic into a dedicated ViewModel while converting the UI into a standalone Compose screen.

Changes:

  • Introduces MetaFeatureSettingsViewModel to own ConfigurationOverride UI state, persistence, and geo database import behavior.
  • Reworks MetaFeatureSettingsScreen into a pure Compose screen that binds to the ViewModel and delegates state updates through an actions interface.
  • Adds new preference-item overloads to support stateless/value-driven preference wiring (instead of MutableState write-through).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
app/src/main/kotlin/com/github/kr328/clash/ui/component/SettingsPreference.kt Adds value/onValueChange overloads for list/edit-text preference items to better support ViewModel-backed state.
app/src/main/kotlin/com/github/kr328/clash/settings/vm/MetaFeatureSettingsViewModel.kt New ViewModel for loading/patching overrides and importing geo database files.
app/src/main/kotlin/com/github/kr328/clash/settings/ui/MetaFeatureSettingsScreen.kt Converts the previous design-based screen into a ViewModel-backed Compose screen and introduces an actions interface.
app/src/main/kotlin/com/github/kr328/clash/settings/MetaFeatureSettingsActivity.kt Migrates the activity from DesignActivity to BaseActivity + setContent and wires in the ViewModel.
Comments suppressed due to low confidence (2)

app/src/main/kotlin/com/github/kr328/clash/settings/ui/MetaFeatureSettingsScreen.kt:79

  • importResult is kept as the latest StateFlow value, and LaunchedEffect(importResult) will re-run when the Composable re-enters composition (e.g., configuration change) with importResult == Success/UnsupportedFormat/Failed, causing the toast/dialog to show again without a new import. Consider modeling import results as one-shot events (SharedFlow/Channel) or clearing/resetting the result after it has been handled (e.g., set back to NotStart once the toast/dialog is triggered).
    app/src/main/kotlin/com/github/kr328/clash/settings/ui/MetaFeatureSettingsScreen.kt:71
  • Using stringResource(R.string.geofile_imported) and then calling .format(...) bypasses Android's resource formatting helpers and relies on String.format behavior. Prefer context.getString(R.string.geofile_imported, result.displayName) (or stringResource(R.string.geofile_imported, ...) in composition) so formatting is handled consistently with Android string resources and translations.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Goooler Goooler merged commit 261a3de into trunk Apr 24, 2026
7 checks passed
@Goooler Goooler deleted the unwrap-meta-feature-settings-design branch April 24, 2026 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants