General: UI Rewrite and Polishing#2381
Draft
d4rken wants to merge 355 commits into
Draft
Conversation
d4rken
added
enhancement
Replaces the three Fragments (AppCleanerListFragment, AppJunkDetailsFragment, AppJunkFragment) with Compose Host/Screen pairs and folds AppJunkViewModel into AppJunkDetailsViewModel — selection state moves to the screen and a single MutableStateFlow tracks per-junk collapsed categories. AppJunkRoute is retired (verified zero call sites). InstallIdNavType / NullableInstallIdNavType stay in MainNavGraph for AppControl + Analyzer.
Both VMs rebase on ViewModel4 + SingleEventFlow + safeStateIn. AppCleanerListViewModel uses synchronous taskSubmitter.submit() for snappier snackbar timing; the folded Details VM uses the taskSubmitter.state listener with handledResults dedupe. Pro gate runs at both requestDelete() and confirmDelete() entry points; an empty-junk filter drops AppJunks that go matchless after a per-path appCleaner.exclude(installId, paths) so the pager doesn't show ghost zero-junk pages.
DeleteSpec sealed type + AppCleanerTaskFactory.buildAppCleanerTask helper map a UI intent to the right AppCleanerProcessingTask. Category / SingleFile / SelectedFiles tasks force includeInaccessible=false (the param defaults to true on the task type, which would otherwise silently clear the inaccessible cache during a file-only delete). Category specs omit targetContents — backend resolves matches via the category's filter to avoid the targetContents lookup colliding via single { tc.matches(it.path) } at AppCleaner.kt:226.
Material3 AlertDialog renders the 'Show details' neutral action as an inline TextButton (Material3 has no neutral slot). The List screen's row body click triggers single-delete; the trailing details IconButton navigates to AppJunkDetailsScreen and is disabled in selection mode; the row's app icon long-press opens the system app settings. Both List and Details emit Event.ExclusionsCreated(count) and surface a snackbar with a 'View' action that navigates to ExclusionsListRoute.
Tests: AppJunkElementBuilderTest covers the 4-element-type builder + per-category collapse + sort. AppCleanerTaskFactoryTest locks in the includeInaccessible=false invariant for non-Inaccessible specs and covers stale-snapshot / cross-category SelectedFiles reconstruction. coil-compose was promoted from implementation to api in app-common-coil so consumers can use AsyncImage directly without redeclaring the dep.
Date 2026-04-26. 50→53 screens. 50→52 VMs (AppJunkViewModel folded into AppJunkDetailsViewModel). ~12→~9 unconverted Fragments. AppCleaner moved to closed-batch list.
Migrates AppControlListFragment + AppActionDialog to Compose hosts, both
rebased on ViewModel4 with SingleEventFlow events. AppActionDialog becomes
a ModalBottomSheet route registered via AppControlNavigation; AppActionRoute
now resolves through entry<>{} instead of dialog<>(). The list keeps its
search field, sort dropdown, asc/desc toggle, full FilterSettings.Tag chip
set, and selection-mode TopAppBar (Delete/Exclude/SelectAll as icons; rest
in overflow). The action sheet renders a sealed AppActionItem list built by
the new pure buildAppActionItems helper, unit-tested across 13 cases
covering current-vs-other-user, existing-exclusion, enable/disable, archive/
restore, size/usage/export availability.
Codex-reviewed integrations: live-id revalidation per click/confirm via
firstOrNull (replaces backend's single{} which throws on stale ids); Pro
re-gating after SAF returns; AppExportTask uses savePath; single-app
Uninstall/Archive/Restore confirms re-instated (Archive/Restore via root/
ADB skip OS confirms otherwise); list scans on null data instead of
navUp() — auto-pop would dead-end Dashboard/shortcut/FAB entries; sheet
stays open after every action (parity with legacy, resolves snackbar race).
Resolves the dead-click FIXME at ExclusionListViewModel.openAppControl().
Closes the AppControl tool's Fragment surface.
AppActionRoute(installId: InstallId) crashed with MissingFieldException on
every action-sheet open. SavedStateHandle.toRoute<>() does not reliably
deserialise Nav3 routes whose serializable args are non-nullable, even with
a typeMap registered (project memory: feedback_nav3_route_binding).
Switch to the Picker pattern: entry<AppActionRoute> { route ->
AppActionSheetHost(installId = route.installId) }. The host now passes the
installId to the VM via setInstallId(), and the VM observes a
MutableStateFlow<InstallId?> in its state combine.
55 screens converted (+2). 54 VMs on ViewModel4 (+2). Unconverted Fragments down to ~7 (Scheduler 2, Analyzer 5). AppControl tool moves to closed-batch list with detailed rationale; ExclusionsList AppControl-FAB FIXME marked resolved.
Replaces SchedulerManagerFragment + ScheduleItemDialog with Compose hosts. Both ViewModels rebased on ViewModel4.
Manager screen: Scaffold + LazyColumn renders schedule cards (ScheduleRow), an AlarmHintRow when any schedule is enabled, and a BatteryHintRow on API 31+ when optimization is not ignored. Top bar wires the back arrow to navUp, the help icon to WebpageTool, and a debug-only schedule trigger gated by Bugs.isDebug. The inline post-schedule commands editor became a Compose AlertDialog with a multiline OutlinedTextField.
Schedule item sheet: ModalBottomSheet route bound via the Picker entry-arg pattern (entry<ScheduleItemRoute> { route -> ScheduleItemSheetHost(scheduleId = route.scheduleId) }) — SavedStateHandle.toRoute<>() crashes for non-nullable serializable args under Nav3 (matches the AppActionRoute fix). Form is backed by a separate MutableStateFlow<FormDraft> initialized once from the live schedule lookup so external schedulerManager.state emissions don't overwrite the user's input. saveSchedule refuses to resurrect a deleted schedule (isCreate guard) and refuses to save into an externally-enabled schedule (live.isEnabled guard) — SchedulerManager only re-checks WorkManager on scheduledAt change, so a partial save would diverge displayed time from the actual delay. canSave normalizes blank labels to null so empty-string saves are blocked.
Time picker uses Compose Material3 TimePicker + rememberTimePickerState(is24Hour = true) inside an AlertDialog, replacing the FragmentManager-bound MaterialTimePicker. Sheet inner Column uses imePadding + navigationBarsPadding so the Save button stays clear of the soft keyboard.
Battery hint failures route through errorEvents.emit(throwable) so ErrorEventHandler shows the standard error dialog (parity with the legacy asErrorDialogBuilder; ActivityNotFoundException.message is nullable so a snackbar-only path was unsafe). updateCommandsAfterSchedule replaces the legacy getSchedule(id)!! NPE-bait with a firstOrNull lookup that no-ops when the schedule is missing or now enabled. Row ETA derivation wraps Schedule.calcExecutionEta in runCatching to defuse the infinite loop on repeatInterval.toDays() == 0.
Tests: SchedulerRoutesSerializationTest extended to cover SchedulerSettingsRoute and SchedulerManagerRoute data-objects. New ScheduleItemViewModelTest covers the two save-time guards (resurrect-deleted, externally-enabled) plus the create-new path and the canSave label invariant. All 36 :app-tool-scheduler tests pass; all 751 :app:testFossDebugUnitTest pass; :app:assembleFossDebug clean.
Smoke-tested on emulator sdmse-rewrite: list renders, edit sheet opens without MissingFieldException, label/time/days editing persists, Pro-gated toggle routes to Upgrade for non-Pro users, delete is one-tap with no confirm (legacy parity), FAB creates a new schedule with random id, IME Done on the label field opens the time picker.
55 → 57 screens converted, 54 → 56 VMs on ViewModel4, ~7 → ~5 unconverted Fragments. Scheduler moved to closed-batch with the ModalBottomSheet route pattern, Compose Material3 TimePicker, and the FormDraft + save-time-guard architecture documented.
…eens to Compose Closes the entire app-tool-analyzer Fragment surface, completing the Compose rewrite for the priority-1 cleaning tools. Five screens converted: DeviceStorage, StorageContent, Apps, AppDetails, Content. All 5 ViewModels rebased on ViewModel4. The four data-class routes drop their from(handle)+typeMap companions and switch to the Picker-style bindRoute pattern (per the route-binding feedback memory). Three screens with required upstream data (StorageContent/AppDetails/Content) use sealed Loading/Ready/NotFound state instead of nullable defaulted State(). Init-time submit work (StorageScanTask, AppDeepScanTask) moves to a separate routeFlow.filterNotNull().take(1) flow so SharingStarted.WhileSubscribed restarts don't re-fire it. Trend chart wraps the legacy SpaceHistoryChartView via AndroidView. Selection mode adds a Select All toolbar action mirroring legacy installListSelection. Compose snackbar text uses context.getString/getQuantityString instead of @composable resource APIs. Progress ratios are clamped 0f..1f to satisfy Material3's strict LinearProgressIndicator. Media category navigation switches single() to singleOrNull() with a no-op fallback. AnalyzerNavigation registers the 5 entries; MainNavGraph drops the 5 fragment registrations and the four NavType vals (no remaining consumers). ExclusionListViewModel.openStoragePicker() FIXME closes since DeviceStorageRoute is now a Compose entry. SpaceHistoryChartView.isCompact gets a public setter so AndroidView can opt-in. Closes #N/A
Priority-1 batch closed: 5 Analyzer screens (DeviceStorage / StorageContent / Apps / AppDetails / Content) on Compose; the entire app-tool-analyzer Fragment surface is retired. Screen count 57 -> 62, ViewModel count 56 -> 61, unconverted Fragment count ~5 -> 0. The last priority-1 FIXME (ExclusionListViewModel storage-picker chip) closes since DeviceStorageRoute is now a Compose entry.
First batch of post-rewrite cleanup. Deletes 24 files (1,534 lines) of legacy infrastructure with zero callers after the Compose conversion. Removed Fragment chain (Fragment2/3, DialogFragment2/3, BottomSheetDialogFragment2, PreferenceFragment2/3), orphan uix helpers (ToolbarHost, Service2, ViewModelLazyKeyed, DetailsPagerAdapter3, FragmentStatePagerAdapter4 + ext), nav helpers (LegacyNavigationBridge, FragmentExtensions, NavDestinationExtensions), MainNavGraph marker, LogViewerAdapter, and the lists/selection/ helpers. Trimmed RecyclerViewExtensions.kt to setupDefaults — its only live consumer is RecorderActivity. Kept ViewModel3, Activity2, NavCommand, NavControllerExtensions, and the rest of common.lists.* because RecorderActivity is still XML-based and consumes them. Subsequent batches will sweep layouts/adapters, then gradle deps.
Second batch of post-rewrite cleanup. Removes 1,249 lines across 36 files of XML resources and Kotlin source whose consumers were retired in the Compose conversion. Layouts (8): main_activity, debug_logview_fragment, debug_logviewer_row, data_areas_fragment, data_areas_list_item, view_swipetool, view_preview, view_new_badge. Menus (4): menu_main, menu_data_areas, menu_setup, menu_exclusion_editor_pkg. Legacy custom views with their paired XMLs (6 sources + 7 layouts): ProgressBarView, BreadCrumbBar (+ crumb item), QualityInputDialog, BadgedCheckboxPreference (replaced by Compose SettingsBadgedSwitchItem), legacy common.ui.AgeInputDialog and common.ui.SizeInputDialog (replaced by Compose compose.settings.dialogs versions). ids.xml files (9, all entirely dead nav-action IDs from the legacy Fragment NavGraph): appcleaner, systemcleaner, corpsefinder, deduplicator, scheduler, swiper, squeezer, app-common-stats, app-common-exclusion. Removed BreadCrumbBarStyle from app/styles.xml. Fixed ErrorDialogSetup.kt to reference eu.darken.sdmse.common.R.id.nav_host (the live shared id) instead of eu.darken.sdmse.R.id.nav_host (which was inlined in the deleted main_activity.xml). Now matches AutomationNoConsentException and InaccessibleDeletionException.
…e-livedata, and CaveatPreference Final batch of post-rewrite cleanup. Removes 188 lines across 20 files of gradle and source: build features, dead deps, and the last legacy Preference custom views. Stripped 'viewBinding = true' from 13 modules whose ViewBinding consumers were retired in the Compose conversion: corpsefinder, stats, coil, systemcleaner, picker, deduplicator, analyzer, squeezer, appcontrol, scheduler, appcleaner, exclusion, swiper. Kept in app (RecorderActivity + LogFileAdapter), app-common-ui (MascotView), app-common-automation (AutomationControlView). Stripped 'androidx.recyclerview:recyclerview-selection:1.2.0' from 11 build.gradle.kts files — entire library was unused after installListSelection deletion. Removed 'androidx.compose.runtime:runtime-livedata' from buildSrc addCompose() — its only consumer was LegacyNavigationBridge, deleted earlier in this cleanup sequence. Deleted CaveatPreferenceView, CaveatPreferenceGroup, and their two layout XMLs — both Preference subclasses were orphan after the preference XMLs converted to Compose. Verified: :app:assembleFossDebug + :app:testFossDebugUnitTest (751/751) + :app:lintVitalFossRelease all clean.
Updates the rewrite plan to reflect the 3 cleanup commits (source / resource / gradle deps purge — 80 files, ~3,000 deletions). Marks the Final-cleanup #3 checklist mostly closed, with two carve-outs: (1) Fragment navigation deps still live in 14 modules and (2) the lists/ + ViewBinding chain stays alive for the RecorderActivity holdout. Adds two new sections: #4 'RecorderActivity to Compose' (next priority — converting it unblocks dropping Activity2, ViewModel3, NavCommand, the lists/ chain, and viewBinding in app/), and #5 documenting the three legacy Navigation.findNavController(view, R.id.nav_host) callbacks (ErrorDialogSetup, AutomationNoConsentException, InaccessibleDeletionException) that are likely runtime-broken under the Compose-only MainActivity and need migrating before the navigation-fragment-ktx deps can be dropped.
Replace the XML-based RecorderActivity (Activity2 + ViewBinding + LogFileAdapter) with a ComponentActivity that uses setContent {} and a Compose UI hierarchy. RecorderViewModel rebases from ViewModel3 (LiveData) to ViewModel4 (SingleEventFlow + safeStateIn), exposes themeState built from GeneralSettings, and routes filesystem work through dispatcherProvider.IO.
Theming.kt is extended to also skip RecorderActivity from the XML-era theming pipeline (the Compose SdmSeTheme handles it). The Activity wraps share-intent launches with try/catch (ActivityNotFoundException, SecurityException) and routes failures back through vm.onShareLaunchFailed → errorEvents.
RecorderScreen.kt introduces 7 composables (hero / sensitive-info / session-path / failed / log-files-header / log-file-row / action-bar). The action bar uses three fixed-weight slots — share button alpha-hidden during zip with a CircularProgressIndicator overlay — so layout never reflows during state changes.
…rom app After RecorderActivity converted to Compose, the entire legacy Activity/VM/RecyclerView infrastructure is unused. Migrate MainActivity from Activity2 to ComponentActivity (only the inherited verbose-logging hooks were used; MainActivity has its own TAG and onResume override). Rebase the test ViewModel in ViewModel2ScopeTest on ViewModel4 and read errorEvents via .first() instead of .value. Reduce DashboardItem to a plain interface with stableId since DifferItem and its payloadProvider machinery are gone. Delete: Activity2.kt, ViewModel3.kt, NavCommand.kt, NavDirectionsExtensions.kt (NavEventSource), the entire app-common-ui/.../lists/ directory (17 files: BaseAdapter, BindableVH, DataAdapter, ListItem, MutableDataAdapter, RecyclerViewExtensions, ViewHolderBasedDivider, AsyncDiffer chain, ModularAdapter, modular/mods/*), LogFileAdapter, debug_recorder_activity.xml, debug_recorder_logfile_item.xml. Drop viewBinding = true from app/build.gradle.kts.
Update COMPOSE_REWRITE_PLAN.md after the RecorderActivity Compose conversion (commits 4e51209 + 2fcfb5e). Final-cleanup batch is now fully closed (5 commits, ~110 files, ~4,200 deletions). The XML/Adapters/ViewBinding sub-bullet is no longer partial — the entire app-common-ui/.../lists/ package, both debug_recorder_*.xml layouts, LogFileAdapter, and viewBinding=true on the app module are gone. The Activity2/ViewModel3/NavCommand/NavEventSource files are deleted. MainActivity is rebased on ComponentActivity. Section #4 is now the 3 legacy Navigation.findNavController callbacks (renumbered from #5) — that's the last cleanup blocker before navigation-fragment-ktx/navigation-ui-ktx can be dropped from the 14 modules that still pull them in.
Three error-dialog callbacks called Navigation.findNavController(view, R.id.nav_host).safeNavigate(SomeRoute) — broken at runtime under the Compose-only MainActivity since no view in the tree carries the nav_host tag. Extend LocalizedError with declarative fixActionRoute and infoActionRoute fields and dispatch them through the NavigationController already available to ComposeErrorDialog via LocalNavigationController.current. Migrated callsites: ErrorDialogSetup.kt (IncompleteSetupException → SetupRoute, WriteException → PathExclusionEditorRoute), AutomationNoConsentException, InaccessibleDeletionException. Existing intent-based fixAction/infoAction callbacks (gplay billing, automation web URLs, security center settings) stay unchanged. Bug fix in ComposeErrorDialog: infoAction tap now dismisses the dialog (the legacy MaterialAlertDialogBuilder did this implicitly via the neutral button). Removed dead helpers: NavControllerExtensions.kt (safeNavigate), SetupModuleSetup.kt (showFixSetupHint/installShowSetupHint never invoked), SnackbarExtensions.kt (enableBigText only consumed by the dead SetupModuleSetup), the var showSetupHint declaration in SetupModuleExtensions.kt, R.id.nav_host. SetupModule.Type.labelRes inlined into ErrorDialogSetup.kt (its only consumer).
…odules After the error-dialog fix actions were rewired through the existing NavigationController (commit 423e567), no Kotlin source references androidx.navigation.NavController anywhere in the codebase. The Fragment-aware Navigation 2.x dependencies are now dead weight. Removed from app, app-common-ui, app-common-stats, app-common-picker, app-common-coil, app-common-automation, app-common-exclusion, and 9 app-tool-* modules. androidx.navigation:navigation-testing stays as androidTest in app/. dependencyInsight on fossDebugRuntimeClasspath confirms neither -fragment-ktx nor -ui-ktx is pulled in transitively.
Update COMPOSE_REWRITE_PLAN.md after the error-dialog migration + nav-fragment-ktx purge. §4 is now fully closed (4 callsites rewired through declarative LocalizedError routes; dead helpers gone; ComposeErrorDialog infoAction-dismiss bug fixed). The Fragment navigation deps sub-bullet under §3 is also marked done. Renumbered the remaining cleanup target as §5: drop transitive fragment-ktx from addAndroidUI() — the three Fragment-era helper files (LiveDataExtensions, ViewBindingExtensions, ActivityExtensions) have no remaining callsites and can be deleted alongside the dep.
After the error-dialog migration, no main-source Kotlin code references androidx.fragment.app.Fragment. Drop fragment-ktx from addAndroidUI() and delete the three remaining Fragment-era helpers. Deleted: LiveDataExtensions.kt (observe2(Fragment) overloads), ViewBindingExtensions.kt (Fragment.viewBinding delegate), ActivityExtensions.kt (viewParent/view/isContentViewSet/showFragment/todoToast). Also dropped the dead LoadingOverlayViewStyle from styles.xml and the now-empty app/.../res/values/ids.xml (last entry fragment_frame was only consumed by the deleted ActivityExtensions; loading_overlay was only consumed by LoadingOverlayViewStyle). androidTest sources (HiltExtensions, EmptyFragmentActivity) still import Fragment / FragmentActivity / FragmentFactory. Those classes remain available transitively via androidx.appcompat:appcompat:1.7.1 (already pulled in by addAndroidCore()), so no androidTest dep adjustment was needed.
fragment-ktx is gone; the three Fragment-era helpers were truly dead. Bonus cleanup: dropped LoadingOverlayViewStyle and the now-empty ids.xml. Section §5 closed in a single commit. Documented §6: a pre-existing androidTest compilation failure in MainActivityTest.kt (carries over from the ViewModel4 migration — mocks LiveData where the VM exposes Flow, references a non-existent onGo() method). Unrelated to §5; queued for a separate small batch.
Three of the four androidTest files were dead code from the legacy XML-era. MainActivityTest.kt mocked MainViewModel.state as LiveData where the VM has been Flow since the ViewModel4 migration, and verified a non-existent vm.onGo() method — broken at compile time and meaningless post-rewrite. ExampleFragmentTest.kt was fully commented out (referenced the long-gone MainFragment + MainFragmentVM). Delete: MainActivityTest.kt, ExampleFragmentTest.kt, HiltExtensions.kt (launchFragmentInHiltContainer<T>() — only consumed by the commented-out test), EmptyFragmentActivity.kt (Hilt host for the fragment-launching helper). After this, no androidTest source imports androidx.fragment.app.Fragment. ExampleInstrumentedTest and BaseUITest stay (the latter is a 1-line scaffold for future Compose UI tests).
All 6 documented cleanup sections of the Compose rewrite are now closed. The branch has no outstanding cleanup blockers; future work falls outside the rewrite plan's original scope (e.g. converting MascotView and AutomationControlView from XML+ViewBinding to Compose, which would unblock dropping viewBinding from app-common-ui and app-common-automation).
Replaces the imperative AutomationControlView (custom ConstraintLayout +
ViewBinding) and the legacy MascotView (FrameLayout wrapper around
LottieAnimationView) with a ComposeView hosting a new AutomationOverlay
composable. AutomationService keeps the same WindowManager addView /
removeView lifecycle but the surface is now state-driven via a single
MutableStateFlow<AutomationOverlayState> instead of imperative setProgress /
setTitle / setCancelListener calls.
Implementation notes:
- ComposeView in a WindowManager overlay needs LifecycleOwner +
ViewModelStoreOwner + SavedStateRegistryOwner. New OverlayLifecycleOwner
helper drives explicit ON_CREATE/START/RESUME at addView and
PAUSE/STOP/DESTROY at removeView. Composition strategy is
DisposeOnDetachedFromWindow so we don't depend on the lifecycle owner
destroying composition.
- updateProgress(...) now also writes overlayState.progress, replacing the
always-on progress.onEach { setProgress(pd) } collector at line 144.
- changeOptions(...) writes title/subtitle to overlayState instead of
controlView.setTitle.
- Theme is snapshotted (themeMode + themeStyle) once at addView and passed
to SdmSeTheme — matches legacy ContextThemeWrapper-at-addView behavior.
- addView is wrapped so the lifecycle owner is destroyed if either
setContent or addView throws (BadTokenException or anything else).
- removeView path is try/finally so controlView and overlayLifecycle are
always nulled and onDestroy() fires even if removeView throws.
- AutomationOverlay uses the existing Compose SdmMascot (no Lottie
pause/resume needed — early-return on progress == null removes the mascot
from composition entirely). Zero-max guard mirrors ProgressOverlay.kt:
Counter/Percent fall back to indeterminate when current==0L || max==0L.
Size keeps the indeterminate spinner but renders displayValue (parity
with legacy progress_text behavior).
- Layout robustness: card capped at widthIn(max=600.dp), secondary text
uses maxLines only (no fixed heightIn cap), mascot capped at
heightIn(max=240.dp).
Cleanup:
- viewBinding = true dropped from app-common-automation and app-common-ui
(both were only used by the deleted custom views). No modules ship
viewBinding = true now.
- Compose plugin + addCompose() added to app-common-automation.
- MascotView declare-styleable removed from attrs.xml; ProgressOverlayView
styleable kept.
- vector_mascot_nobroom.xml is intentionally kept; launch_screen.xml still
references it.
- Easter-egg click counter (wiggle/rotate animations on every 5th/12th tap)
was dropped — debug-only, not surfaced to users.
Records that AutomationControlView and MascotView were the final viewBinding holdouts and were converted in commit 85fa450. Updates the historical 'viewBinding stripped from 14 modules' note to point at the follow-up that closes the remaining two, and adds a dedicated bullet covering OverlayLifecycleOwner, the state-flow plumbing, and the Compose overlay layout decisions.
Drops 257 lines of dead code: the legacy ProgressOverlayView custom ConstraintLayout, its two layout XMLs (view_progress_overlay.xml, view_progress_overlay_large.xml), and the now-empty attrs.xml in app-common-ui (the MascotView styleable was already removed in commit 85fa450; ProgressOverlayView was the only remaining entry). The Compose ProgressOverlay.kt has replaced it everywhere — no Kotlin or XML callers remain (verified by grep). Also drops the stale 'Compose counterpart to ProgressOverlayView' doc comment now that the legacy class is gone.
Mirrors the CorpseFinder test coverage improvement (commit d9901e6). Adds 78 new tests bringing the app-tool-swiper module from 63 to 141 unit tests. Covers SwiperSessionsViewModel, SwiperStatusViewModel, SwiperSwipeViewModel (including the stale-override guard branch), and a small SwiperSessionsScreen UI smoke test. Includes a private SwiperPreviewData helper for synthetic SwipeItem/PathLookup builders shared between tests. Uses the TestScope.backgroundScope pattern in the harness to keep safeStateIn's WhileSubscribed upstream collection alive without blocking runTest completion.
Captures four learnings from the Swiper test work: - safeStateIn + WhileSubscribed harness pattern using TestScope.backgroundScope (without this, every render-state assertion sees initialValue) - DataStoreValue extension stubbing (value() and value(T) are extensions, not members — stub flow / verify update()) - ExclusionManager.save Set capture (the single-Exclusion overload is an extension) - NavigationController.consumeResults hot-flow stubbing for picker results
…tivity inputs Copy-paste bug at AppControl.performScan: hasInfoActive was computed from `task.loadInfoSize && curState.canInfoSize` instead of `task.loadInfoActive && curState.canInfoActive`. Surface-level impact: after a toggle / uninstall / archive / restore operation, the post-action snapshot rebuild uses `snapshot.hasInfoActive` to decide whether to re-fetch isActive for the affected rows. With the bug: - Sizing on + activity off → updated rows had isActive re-queried that the rest of the list didn't have. - Sizing off + activity on → updated rows lost isActive while the rest of the list kept it → inconsistent column population. Added AppControlTest (9 tests) covering performScan flag computation: - hasInfoActive=true only when loadInfoActive AND canInfoActive (regression-shaped: also verifies hasInfoActive is independent of loadInfoSize) - hasInfoSize / hasInfoScreenTime / hasIncludedMultiUser follow their own respective AND formulas - AppScan.allApps receives the AND of task flag and capability gate, not the raw task flag - Result.itemCount reflects the AppInfo set size Uses the same SharedResource.createKeepAlive pattern that CorpseFinderTest established to keep keepResourceHoldersAlive(appScan) from failing on MockK relaxed mocks.
…nd hasInfoActive fix Expands AppControl unit + Compose UI test coverage from ~30 to 144 tests, following the patterns established by the recent CorpseFinder testing work. Surfaced and fixed one pre-existing bug: AppControl.performScan was computing Data.hasInfoActive from the size inputs (loadInfoSize && canInfoSize) instead of the activity inputs (loadInfoActive && canInfoActive). The wrong value then propagated into every post-action snapshot rebuild (toggle / uninstall / archive / restore), causing rows refreshed after an action to either lose or gain isActive info inconsistently with the rest of the list.
…ests Mirrors the CorpseFinder ViewModel test pattern (d9901e6) for the two settings ViewModels: state composition from DataStoreValue mocks, individual setter write-through with captured update lambdas, picker-result wiring, and navigation events for both search-locations and arbiter-config entry points. ArbiterConfigViewModelTest pins the canonical default criterium order, covers Mode.entries for every non-PreferredPath criterium type, asserts onModeSelected matches by class (not by index, which would mis-mutate adjacent criteria), and verifies the picker-result handler REPLACES PreferredPath.keepPreferPaths without touching sibling criteria. DeduplicatorSettingsViewModelTest pins the picker channel name ('picker.result.scan.location.paths') so consumer/producer drift can't silently swallow results, and asserts onSearchLocationsClick / onArbiterConfigClick emit the correct NavEvent.GoTo destinations.
Adds 34 unit tests covering state composition (sort-by-size, deleteTargetIds
across favorite/non-favorite-with-keeper/no-keeper branches), navUp on data
drain, deletion flows for clusters/single-dupe/multi-dupe (pro gating and
task submission), exclusion flows, layout-mode toggle, and task-result event
dispatch (init-time filtering, single-duplicate suppression).
Two pre-existing bugs fixed:
1. navUp during loading. The init flow used 'map { it.data }.drop(1).filter
{ !it.hasData }', so when performScan reset internalData to null at the
start of a refresh, the user got bounced back to the parent screen mid
scan. Switched to 'mapNotNull { it.data }.drop(1)...' to match the
CorpseFinder fix in d9901e6 and the DeduplicatorDetailsVM pattern
already in this module.
2. excludeClusters() snackbar count mismatch. The VM emitted
'sanitized.sumOf { it.count }', which is the requested-files count, not
the actually-saved exclusion count. When ExclusionManager.save() coalesces
duplicate paths the user saw an inflated count. Now reads
'undo.exclusionIds.size' (same fix shape as the CorpseFinder list VM).
41 unit tests covering the paged-details VM: - bindRoute idempotency (single-route and updatePage-then-rebind cases) - State target resolution: route identifier, updatePage swap, lastPosition fallback when the requested target is removed - Directory view toggle and per-cluster collapsed-dir set with stale-cluster pruning - Preview navigation: cluster (asserts paths), group with position clamping (over-range and negative), empty group no-op, single duplicate - openDuplicate via ViewIntentTool: emits OpenDuplicate(intent) when the intent resolves, suppresses the event when the tool returns null - deleteCluster/deleteGroup/deleteDuplicates: unconfirmed emits ConfirmDeletion with the correct DeleteTarget variant; confirmed-without-pro routes to UpgradeRoute(); confirmed-with-pro submits the right TargetMode - deleteDuplicates filters stale ids AND ids from other clusters (the currentClusterOrNull scope is asserted) - excludeCluster/excludeDuplicates with stale-id no-op paths and the saved- exclusion-count contract for the snackbar - onUndoExclude sets currentTarget and calls undoExclude (re-emit forced via progressFlow since the deduplicator.state input has distinctUntilChangedBy) - autoNavUpOnEmpty drain vs loading: empty → navUp, null → no navUp - Task result dispatch: filters pre-init tasks, suppresses single-duplicate deletes, dedups by task id, forwards multi-target results
19 JVM-runnable Compose UI tests using BaseComposeRobolectricTest: - DeduplicatorListScreenTest (6 tests): loading, empty, populated GRID/LINEAR rendering, layout-toggle action visibility, callback wiring on tap. The populated LINEAR test asserts an actual row body renders by checking for the duplicate file name — not just the absence of the empty placeholder. - DeduplicatorDetailsScreenTest (4 tests): loading, empty, single-cluster flat-view, single-cluster directory-view. Both populated cases assert ClusterContent composed via the 'Occupied storage' summary label rather than the deferred LazyColumn row items. Stays on single-item state to avoid HorizontalPager / ScrollableTabRow interactions that Robolectric handles poorly. Provides a mocked GuidedTourController via CompositionLocalProvider because the screen reads LocalGuidedTourController.current and the local has no default. - DeduplicatorSettingsScreenTest (9 tests): top bar, above-the-fold rows, Search-locations / Arbiter-config tap callbacks, allowDeleteAll switch, the Detection-method category header, and per-sleuth toggle wiring for all three sleuths (Checksum, PHash, Media) plus the Skip-uncommon switch. Catches regressions that swap sleuth callbacks across rows.
Fix #1 (state.value reads from business logic): SwiperStatusViewModel.navigateToItem and SwiperSwipeViewModel.setDecision / skip / advanceOrNavigate now read items + session directly from the source flows via a new snapshotForAction helper. Previously these methods read state.value, which is governed by safeStateIn's WhileSubscribed — they only worked because the Compose screen happened to be collecting. Fix #2/#3 (stale-override clearing in combine): Drop the in-combine mutation of currentIndexOverride and the eager 'clear override when session.currentIndex == 0' branch. The branch fired on every fresh-session bindRoute with a non-default startIndex, not just on post-delete back-navigation. coerceIn alone handles the post-delete out-of-bounds case. Fix #4 (dead updateState before delete): Drop sessionDao.updateState(COMPLETED) in Swiper.performDelete — the only consumers of state filter COMPLETED out, and the row is deleted on the next line. Fix #5 (finalize Room flow read): Add a comment explaining why swiper.getSession(sid).first() reliably observes the post-delete state, since the pattern looks racey but isn't (Room's flow.first() runs a fresh query). Tests updated: drop the test that documented the eager-clearing bug as desired behavior; merge two coercion tests into one; expand the override-applied test to cover both fresh and mid-session cases. 140 tests pass.
…s, document ViewModel render-state testing patterns
… bugs Brings app-tool-systemcleaner from 1 narrow VM test to 21 test files (305 module tests total) covering engine orchestration + all 5 ViewModels + all 5 Screens + custom-filter subsystem, mirroring the CorpseFinder coverage pattern from d9901e6. New engine coverage (35 tests): SystemCleanerTestFixtures shared harness, SystemCleanerExtensionsTest (hasData), FilterContentTest (size/totalSize/totalCount), SystemCleanerScanTest (scan/scheduler/oneclick dispatch + FilterSource onlyEnabled gating), SystemCleanerProcessingTest (targetFilters/targetContent scoping, NoSuchElementException for stale ids, swallowed PathException, ancestor-path matching), SystemCleanerExclusionTest (exclude/undoExclude, exclusionIds from save() result, stale-handle path), FilterSourceTest. New ViewModel coverage (78 tests): FilterContentDetailsViewModelTest, SystemCleanerSettingsViewModelTest (23 toggles via parameterized test), CustomFilterListViewModelTest, CustomFilterEditorViewModelTest (save/cancel/remove state machine, phantom-filter guard, criteria mutators). SystemCleanerListViewModelTest extended from 5 → 16 tests. New Compose UI coverage (22 tests): SystemCleanerListScreenTest, FilterContentDetailsScreenTest, SystemCleanerSettingsScreenTest, CustomFilterListScreenTest. CustomFilterEditorScreenTest extended 4 → 7. New custom-filter unit coverage (24 tests): CustomFilterTest (config→sieve mapping), EditorOptionsCreatorTest (DIRECTORY/FILE branches, mixed-area label), CustomFilterExtensionsTest (toggleCustomFilter three-way logic), CustomFilterRepoTest (persistence with temp dir, save/remove/import/export round-trips). 9 BUG-FIXME tests lock in current (buggy) behavior — each commented with the marker so the PR description can cross-reference. These are documented for follow-up, not fixed in this PR per scope agreement: BUG-FIXME-1 (dead ScanTask fields pkgIdFilter/isWatcherTask), BUG-FIXME-2 (ProcessingTask cross-filter smear, missing init { require }), BUG-FIXME-3 (exclude() identifier param logged but not used to scope the prune — applies to ALL filters), BUG-FIXME-4 (details VM emits zero-count ExclusionsCreated event vs list VM which suppresses), BUG-FIXME-5 (editor VM save() bypasses canSave gating — UI is the only safety net), BUG-FIXME-6 (CustomFilterRepo decode crash on corrupt JSON kills entire configs flow), BUG-FIXME-7 (CustomFilterRepo listFiles()!! NPE when mkdirs silently fails), BUG-FIXME-8 (CustomFilterConfig.isUnderdefined ignores pathRegexes — regex-only filters unsaveable), BUG-FIXME-9 (SystemCleanerListViewModel navUp fires on null transition during scan-restart; FilterContentDetailsViewModel has the mapNotNull fix already). Crawler integration tests (filter-set first-match race, area orchestration, exclusion second pass) and live-search in CustomFilterEditorViewModel are explicitly out of scope — they require real APathGateway/FileForensics mocking that exceeds the parity goal. Zero changes outside app-tool-systemcleaner/src/test/.
… 9 pre-existing bugs SystemCleaner: Add unit and Compose UI tests, document 9 pre-existing bugs
Brings in 23 commits from main (1.7.2/1.7.3-rc cycle): storage rescue, Huawei TAF inventory fix, ShellOps streaming, root binder fixes, Honor/OneUI/Samsung AppCleaner specs, Squeezer video timestamp preservation, Media3 1.10, Crowdin translation refresh, Actions-driven release flow. Conflict resolutions: - App.kt: kept both new @Inject fields (taskResultNotifier + storageRescue) - RootSetupModule.isComplete: kept compose-rewrite's when-block (equivalent logic to main's expression — both fix the same dashboard card visibility bug) - SetupFragment.kt: accepted delete; Compose SetupScreen already refreshes setup on resume via LifecycleEventEffect(ON_RESUME), which covers the ShowOurDetailsPage refresh case main was fixing - 9 translation XML files (fr/et/be/nl/uk/az): took main's Crowdin updates and dropped orphan keys no longer present in the English source after compose-rewrite cleanup commits
The debug-log card was force-inserted at index 1 when a recording started, jumping it to the top and reflowing the adaptive grid on multi-column layouts. Append it at its normal position instead so the active-recording card occupies the same slot as the idle start card.
The dashboard bottom-bar one-click/delete path submitted DeduplicatorDeleteTask and DeduplicatorOneClickTask without checking the Pro upgrade, unlike AppCleaner which gates the equivalent actions. Pro-gating for cleaning tools is enforced only in the UI layer (the task processors don't check), so a non-Pro user with Deduplicator one-click enabled could delete duplicates for free. Gate both DELETE and ONECLICK on upgradeRepo.isPro(), and route non-Pro users to the upgrade screen from ONECLICK when Deduplicator is the only actionable tool (mirroring AppCleaner). DELETE gets no upgrade-nav branch because Deduplicator data isn't part of the DELETE trigger condition, making that case unreachable there.
…t boundary Defense-in-depth so a UI mistake can't run a Pro-only task for free. The UI already gates Pro features before submitting; this refuses Pro-only tasks at the backend SDMTool.submit() boundary too, throwing a typed UpgradeRequiredException (HasLocalizedError) with a localized title + description. The app-level error dialog customizer attaches the UpgradeRoute fix action, so core code stays free of UI-route coupling. Adds UpgradeRepo.isProSettled() for backend gates: it is deliberately generous/fail-open. It fast-paths to allow when already Pro (active purchase or grace period), otherwise triggers refresh() and waits briefly for billing to settle (rescuing the GPlay cold-start billing-connect race for real customers), and allows on any exception. Only a settled no-purchase reading denies. Existing UI isPro() calls are unchanged. The gate runs before acquiring toolLock so the (possibly waiting) check never holds the lock. Premium-ness is a private per-tool exhaustive 'val Task.requiresPro' (scan is free; delete/processing/one-click/scheduler are gated), so new task types force a compile-time decision. Adds unit tests for both tools.
Navigation/route-binding: NavigationController queues cold-start shortcuts until the back stack is ready; Upgrade/Setup/Swiper/CorpseFinder/Preview routes use the bindRoute pattern instead of SavedStateHandle.toRoute (fixes MissingFieldException crashes); Upgrade forced-flag honored. Lifecycle/state: Settings collects via LaunchedEffect (no leaked vmScope collector); GPLAY Upgrade uses SharingStarted.Lazily to stop offer reloads on resume; nullable isPro/setupDone avoids a sponsor-card flash. Legacy parity restored: Picker selected-paths bottom sheet + Coil folder-preview rows + home-folder area-root icon; Deduplicator open-in-external-app action and cluster-level exclude; Deduplicator details file rows stack the delete marker above a right-aligned file size. List/details correctness: eager selection-clear and single-item Show-details guard across CorpseFinder/SystemCleaner/AppCleaner/AppControl; AffectedPaths composite path:action key; Analyzer exitOnNotFound auto-navUp; AppCleaner Category targetContents kept null to avoid the single() crash. Coil PreviewScreen: per-page zoom lock, immersive system bars, broken-image error state. SdmFastScroller RTL offset. Dead code removed (DelayedState, unused ViewModel state/params, route companions, PickerResult bundle helpers). Added @preview2 coverage with bespoke mock data for Analyzer/Squeezer/Picker/CorpseFinder. Cosmetic/localization: ProgressOverlay fade, ThemePicker scroll, DashboardCardConfig state source, SupportScreen WIKI/DISCORD links, new localized strings.
The main-action FAB applied combinedClickable to the Surface's outer modifier, which sits outside the Surface's shape clip. The click indication (hover highlight / ripple) was drawn as a square, ignoring the 16.dp rounded corners. Move combinedClickable onto the inner content Box (now fillMaxSize so the full 56.dp stays tappable), which renders inside the Surface's shape clip so the indication is clipped to the rounded corners. shadowElevation stays on the Surface; the enabled!=WORKING guard moves with the click handler.
The dashboard list is a combine of ~22 source flows and shows only a loading spinner until every source emits at least once. Several sources re-run async work (Room, DataStore, a filesystem scan) on every ViewModel recreation because they're shared with stopTimeout=0/replayExpiration=0, so a single slow or blocked source could wedge the whole dashboard on the spinner after a warm Activity recreation (e.g. background->foreground under memory pressure). Give every combine input a guaranteed immediate first value via onStart fallbacks (the pattern already used on the tool-state sources) and emit upgradeInfo null-first so title/upgrade-gated cards don't gate on it. The dashboard now renders immediately and fills cards in as data arrives. Each source is tagged with a privacy-safe VERBOSE diagnostic (type/size only) to pinpoint a laggard on reproduction. Adds DashboardViewModelResilienceTest covering that listState still emits when any single source never emits.
…ashboard The read-only session scan (DebugLogSessionManager.sessions) acquired the same fsMutex that zipSessionAsync/zipSession hold for the entire duration of a log zip. A long-running zip therefore blocked the scan indefinitely, and because the dashboard's listState combine waits for the sessions flow, this could wedge the whole dashboard on its loading spinner (observed after background->foreground under memory pressure, where orphaned recordings trigger an auto-zip on relaunch). Run the scan lock-free (a reader must never be blocked by a writer) and keep fsMutex only on the zip/delete writers. scanSessions now skips the stale-temp cleanup for sessions currently being zipped, so the lock-free scan can't delete a .zip.tmp mid-write; the manual zipSession path registers in zippingIds for the same protection. Adds a real-concurrency DeadlockRegression test (a long zip holds the lock while the scan must still complete) plus scanSessions temp-skip tests.
…ing spinner Resilient dashboard listState (no single stalled card source can wedge the screen) plus the root-cause fix: DebugLogSessionManager's read-only scan no longer shares fsMutex with the long log-zip, which could block it indefinitely.
Floating summary card above the bottom bar that cradles the main-action FAB via a cutout mirrored from the bar's notch (shared, concentric geometry in DashboardChromeGeometry). It surfaces what the one-tap action will free after a scan (FREEABLE) and what was freed after a delete/one-click (FREED), action-truthfully: only tools whose one-click toggle is on, that have data, with AppCleaner additionally gated on Pro, and Deduplicator contributing redundantSize/clusters. Interactions: per-tool chips open that tool's findings list; the card is dismissed via the X button or a downward swipe (BottomBar-owned drag so it composes with the existing slide/fade exit); a compact summary chip in the bar restores a user-dismissed hero. Chrome polish: the bar now draws flush to the screen edge (fills the nav inset), the FAB cutout is dropped while the dashboard is loading (no empty notch), and the cradle notch is a uniform offset of the FAB. SdmInfoChip gains an optional onClick (clickable on the inner row; the Material3 Surface(onClick) overload dropped the chip inside the hero's alpha/offset graphicsLayer on-device). Tests: hero-summary construction, chrome shape selection, and hero-card UI (chip navigation, restore, swipe-to-dismiss).
Brings the hero card feature up to date with compose-rewrite's dashboard loading-stuck fix, listState hardening, Pro-gating at the submit boundary, and FAB ripple clipping. Conflict resolutions: the Deduplicator main-action keeps the Pro gate and upgrade-nav while wrapping submit in the freed-result accumulator; the main-action FAB keeps the inner-Box clickable (ripple clipped to the rounded shape) plus long-press; the grid's bottom content padding reuses the chromeBottom max (equivalent to the inlined nav-inset floor).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
The entire UI layer is being rewritten and polished from scratch. This is a long-running effort — the branch will be merged to main once all screens are converted and the app runs fully on the new UI stack.
Why?
Current progress:
This PR will be updated as work progresses. See COMPOSE_REWRITE_PLAN.md in the branch for the full status and remaining work.