Skip to content

chore(tweaks): settings bug-bash pass — 4 small fixes (E3 R5)#543

Merged
rainxchzed merged 3 commits intomainfrom
chore/e3-r5-settings-bugbash
May 8, 2026
Merged

chore(tweaks): settings bug-bash pass — 4 small fixes (E3 R5)#543
rainxchzed merged 3 commits intomainfrom
chore/e3-r5-settings-bugbash

Conversation

@rainxchzed
Copy link
Copy Markdown
Member

@rainxchzed rainxchzed commented May 8, 2026

Static-audit pass over the Tweaks surface (8 sections, ~2.5K LOC) per Foundation Sprint Task 3 / E3 R5 brief. Surfaced 4 concrete code-level findings; manual interactive bug-bash on real devices is queued separately.

Bugs fixed

# Severity Location Fix
1 low TweaksViewModel.kt (3 sites) Replaced println(...) with the injected GitHubStoreLogger. Failures in the installer-attribution flow now reach Desktop crash-log + Android Logcat through the same pipeline as the rest of the repo.
2 low (a11y) About.kt AboutItem Leading decorative icon was wrapped in an IconButton with onClick = { }, exposing an interactive Button role + ripple to TalkBack. Replaced with a non-clickable Box carrying the same shape + container colour.
3 low (dead code) TweaksAction.kt, TweaksViewModel.kt OnHelpClick action was defined + handled (opened the GitHub issues page) but never emitted from any composable. Action removed; dead BrowserHelper constructor dependency dropped.
4 low RestartApp.jvm.kt (2 sites) Two System.err.println log lines bypassed the Desktop crash-reporter pipeline. The "without adding a logging dep" rationale was stale — Kermit is already transitively available. Routed through co.touchlab.kermit.Logger.

Issues noted but skipped (with reasoning)

  • Telemetry UI is dead per CLAUDE.md "no production telemetry of any kind ships" — but the toggle does still flip a real DataStore flag that TelemetryRepositoryImpl.flushPending consults. Removing the UI is a strategic decision, not a bug. File a separate task if the call is to fully strip.
  • refreshCacheSize re-launch: verified safe — observeCacheSize() is a single-emit cold flow; the collect job completes after one emission, so the manual-refresh tap correctly relaunches.
  • OnProxyTypeSelected re-fires save when re-tapping the current type: idempotent DataStore write, no observable harm.

Tests deferred to interactive bug-bash

The static audit cannot prove these — the operator should run a device-side pass to verify:

  • Theme color change applies live (no app restart)
  • Proxy URL with invalid host surfaces clear error
  • Mirror picker auto-suggest connection-test refreshes after re-open
  • Update interval persists across app upgrade
  • Auto-update toggle actually disables the Worker
  • Cache-clear actually clears all caches
  • Translation provider switch from Google → Youdao with creds present commits + activates immediately
  • Send-feedback flow end-to-end on email + GitHub paths

Findings doc at roadmap/E3_R5_SETTINGS_BUGBASH.md (gitignored) tracks the full audit + checklist for the device pass.

Test plan

  • Compile clean: :feature:tweaks:presentation:compileDebugKotlinAndroid ✅, :feature:tweaks:presentation:compileKotlinJvm ✅, :composeApp:compileDebugKotlinAndroid ✅.
  • Manual smoke (Tweaks surface):
    • Open Tweaks → About → tap leading info icon → no ripple, no a11y "Button" announcement.
    • Switch installer attribution to Custom with invalid package name → error supportingText shows + Logcat now carries the failure tag (was a println before).
    • Desktop: trigger language change → Logcat shows the restart attempt through Kermit.

What's-new

Skipped — these are silent internal cleanups (no user-visible behaviour change). Per the headline-only what's-new philosophy.

Summary by CodeRabbit

  • Removed

    • Help button functionality has been removed from the Tweaks settings.
  • Style Changes

    • Help icon in the About section is now visually redesigned as a non-interactive element.
  • Improvements

    • Enhanced error logging mechanism for installer-related issues.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 8, 2026

Review Change Stack
No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5daabaeb-2808-4f71-bd11-b63be8a03aff

📥 Commits

Reviewing files that changed from the base of the PR and between 25f26a8 and 12f8500.

📒 Files selected for processing (4)
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksAction.kt
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksViewModel.kt
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/components/sections/About.kt
  • feature/tweaks/presentation/src/jvmMain/kotlin/zed/rainxch/tweaks/presentation/RestartApp.jvm.kt
💤 Files with no reviewable changes (1)
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksAction.kt

Walkthrough

This PR removes the help-click action from the tweaks UI, eliminates browser-helper integration, consolidates error reporting through a dedicated logger, and refactors the About section icon from an interactive button to a styled non-clickable circle.

Changes

Tweaks Presentation Refactoring

Layer / File(s) Summary
Action Contract
feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksAction.kt
OnHelpClick data object removed from sealed interface.
ViewModel Dependency Injection & Logging
feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksViewModel.kt
Constructor removes BrowserHelper, adds GitHubStoreLogger. Three error paths replace println with logger.error(). OnHelpClick handler branch removed.
About Section UI Styling
feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/components/sections/About.kt
IconButton replaced with circular non-clickable Box using clip(CircleShape). Icon tint updated to onSecondaryContainer. clip import added.
JVM Restart Error Logging
feature/tweaks/presentation/src/jvmMain/kotlin/zed/rainxch/tweaks/presentation/RestartApp.jvm.kt
System.err.println replaced with Logger.w() for command availability and exception fallback messages.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • OpenHub-Store/GitHub-Store#435: Both PRs modify TweaksAction and TweaksViewModel presentation surface—this PR removes OnHelpClick and adds logging consolidation, while the retrieved PR introduces OnAppLanguageSelected for language handling.

Poem

🐰 Help clicks fade to silent cheer,
Logs now speak where screens once peer,
Icons rest in circles bright,
Styling smooth, no clickable blight—
The tweaks flow cleaner, oh what a sight!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the PR as a bug-bash pass addressing four small fixes to the tweaks feature, which aligns with the changeset content.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/e3-r5-settings-bugbash

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@rainxchzed rainxchzed merged commit a09ab59 into main May 8, 2026
1 check passed
@rainxchzed rainxchzed deleted the chore/e3-r5-settings-bugbash branch May 8, 2026 11:26
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.

1 participant