[{"id":4453266713,"body":"๐Ÿ” **Coverage baseline may need a refresh.**\n\nThis PR touches `app/src/main/**`. If the changes affect what smoke covers, please refresh the baseline before merging:\n\n```bash\n./gradlew generateGmdCoverage\ngit add app/coverage-baselines/gmd_smoke.ec\ngit commit -m \"chore: refresh smoke coverage baseline\"\n```\n\n_Last checked: 2026-05-14 17:50 UTC ยท commit `0b37987`_\n\u003c!-- Sticky Pull Request Commentbaseline-reminder-bot --\u003e","html_url":"https://github.com/Mesya82/Service-Book/pull/91#issuecomment-4453266713","user":{"login":"github-actions[bot]","id":41898282,"profile_url":"https://github.com/apps/github-actions","avatar_url":"https://avatars.githubusercontent.com/in/15368?v=4"},"author_association":"NONE","reactions":{"total_count":0,"+1":0,"-1":0,"laugh":0,"confused":0,"heart":0,"hooray":0,"rocket":0,"eyes":0},"created_at":"2026-05-14T17:50:30Z","updated_at":"2026-05-14T17:50:30Z"},{"id":4453323475,"body":"## Code review\n\n### Overview\nExtracts privacy-disclosure UI into reusable components. Settings + Setup get a `PrivacyPolicyDialog` (popup). Consent dialog and Report Bug stay inline-expand but adopt synced `expandVertically + fadeIn` specs (`tween(250)`, `Alignment.Top`) to fix the visual glitch. Also flips Setup's crash-reporting default to `true`.\n\n---\n\n### Deviations from the approved plan\n\n- **Report Bug not converted to popup.** Plan was to extract `DeviceInfoDialog.kt`, drop `ReportBugState.showDeviceInfo` / `ToggleDeviceInfo`, and open a dialog. Instead the PR keeps the inline expand and only fixes the animation specs. That does resolve the visible bug, but it leaves Report Bug inconsistent with Settings/Setup (both popup now) and skips the VM cleanup. Either accept the simplification deliberately or finish the plan โ€” currently it's half-way.\n- **`DeviceInfoDialog.kt` not created**, `TestTags.REPORT_BUG_DEVICE_INFO_DIALOG` not added โ€” consistent with the above.\n\n### Concerns\n\n- **Privacy-sensitive default change is undisclosed scope creep.** `SetupScreen` flips `crashReportingEnabled` from `false` to `true`. This is a separate UX/privacy decision (opt-out vs opt-in for Sentry), not part of the issue #83 fix, and the PR body's \"as requested\" claim has no corresponding instruction in the plan or the issue. Suggest splitting into its own PR with explicit rationale โ€” opt-out crash reporting is the kind of change reviewers and users will want to be explicit about.\n- **`VehicleListConsentDialogTest` asserts different strings in expand vs. collapse tests.**\n - `clicking Learn more expands disclosure` asserts `settings_privacy_description` is displayed.\n - `clicking Show less collapses disclosure` asserts `settings_privacy_captured_title` does not exist.\n \n Use the same anchor string in both tests so the pair actually proves the same content appears/disappears.\n- **Test names are now misleading.** `settings_privacy_expanded` and `setup_step2_sentry_privacy_expanded` capture a dialog, not an expansion. Plan called for renaming to `settings_privacy_dialog` (and same for setup) โ€” please rename so the screenshot file/test names match the UX.\n- **`Column { ExpandablePrivacyDisclosure() }`** in `VehicleListScreen.kt` wraps a single child in a `Column` โ€” leftover from the deleted `AnimatedVisibility` block. Drop the wrapper.\n- **`heightIn(max = 400.dp)`** in `PrivacyPolicyDialog` is a magic number. `AlertDialog`'s `text` slot already scrolls; consider letting it size naturally or use a density-aware value. Minor.\n\n### Smaller notes\n\n- `ExpandablePrivacyDisclosure` looks good: matched `tween(250)` for size + fade, `expandFrom` / `shrinkTowards = Alignment.Top` โ€” this is the right fix for the inline-collapse glitch.\n- Imports cleaned in `SettingsScreen.kt`, `AppNavGraph.kt`, `VehicleListScreen.kt`.\n- Test tag `PRIVACY_LEARN_MORE` added and used consistently โ€” good for stable selection.\n- New screenshot baseline added for the Setup expanded state; the Settings baseline reuses the existing filename (will need re-recording since the UI changed from inline to dialog โ€” confirm `verifyRoborazziDebug` is green).\n\n### Suggested actions before merge\n\n1. Either complete the Report Bug โ†’ dialog conversion or update the PR description to declare the simplification (and remove the unused `state.showDeviceInfo` / `ToggleDeviceInfo` if keeping it inline โ€” currently still in the VM).\n2. Split out the `crashReportingEnabled = true` default change, or justify it in the PR body with a link to the decision.\n3. Rename `*_privacy_expanded` tests/snapshots to `*_privacy_dialog`.\n4. Align the two `VehicleListConsentDialogTest` assertions on the same string.\n5. Drop the unnecessary `Column` wrapper in `VehicleListScreen`.","html_url":"https://github.com/Mesya82/Service-Book/pull/91#issuecomment-4453323475","user":{"login":"Mesya82","id":32867735,"profile_url":"https://github.com/Mesya82","avatar_url":"https://avatars.githubusercontent.com/u/32867735?v=4"},"author_association":"OWNER","reactions":{"total_count":0,"+1":0,"-1":0,"laugh":0,"confused":0,"heart":0,"hooray":0,"rocket":0,"eyes":0},"created_at":"2026-05-14T17:57:52Z","updated_at":"2026-05-14T17:57:52Z"}]