Skip to content

bottom sheet refactor - #569

Open
Kimblebee wants to merge 4 commits into
kim/refactor/quickSettings/button-rowsfrom
kim/refactor/quickSettings/bottomSheet-refactor
Open

bottom sheet refactor#569
Kimblebee wants to merge 4 commits into
kim/refactor/quickSettings/button-rowsfrom
kim/refactor/quickSettings/bottomSheet-refactor

Conversation

@Kimblebee

@Kimblebee Kimblebee commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Summary

Refactors Quick Settings to use Material 3's in-hierarchy BottomSheetScaffold instead of ModalBottomSheet.

Why the Transition from ModalBottomSheet to BottomSheetScaffold?

  • In-Hierarchy Composition: BottomSheetScaffold renders the sheet within the primary Compose layout tree, maintaining a single window lifecycle and preserving seamless viewfinder rendering.

Key Changes

  • In-Hierarchy Scaffolding: Replaced root Scaffold with BottomSheetScaffold in CaptureLayout.kt (sheetPeekHeight = 0.dp), unifying SnackbarHost and removing nested sheet wrappers.
  • Clickable Drag Handle: Added BottomSheetDefaults.DragHandle with onDismissQuickSettings click action and QUICK_SETTINGS_DRAG_HANDLE test tag.
  • State & Back Handling: Managed isQuickSettingsOpen in PreviewScreen.kt with LaunchedEffect sync and BackHandler(enabled = isQuickSettingsOpen).
  • Test Determinism: Added closeQuickSettings() and updated visitQuickSettings in ComposeTestRuleExt.kt to click the drag handle pill, eliminating flaky synthetic gesture timeouts.

Lifecycle Behavior & Trade-offs

  • Persistent Composition (Never Decomposed): Unlike ModalBottomSheet (which dynamically mounts/unmounts from composition upon opening/closing), BottomSheetScaffold is a persistent layout container. When hidden (sheetPeekHeight = 0.dp), the sheet container remains in the composition tree and is translated off-screen rather than decomposed. Automated tests therefore check visibility (.isNotDisplayed()) rather than complete semantics node absence.
  • State Synchronization: Traded the declarative onDismissRequest modal API for bidirectional LaunchedEffect synchronization between the boolean UI state (isQuickSettingsOpen) and SheetState.
  • Drag Handle Affordance: Material 3's default drag handle is purely a drag gesture affordance; a .clickable modifier was explicitly attached to provide a deterministic tap-to-dismiss target for accessibility and test automation.

# Conflicts:
#	app/src/androidTest/java/com/google/jetpackcamera/utils/ComposeTestRuleExt.kt
- Remove isQuickSettingsOpen and quickSettingsIsOpen from TrackedCaptureUiState, QuickSettingsUiState, and UI state adapters.
- Remove toggleQuickSettings() from QuickSettingsController and its implementations.
- Manage quick settings drawer visibility as local UI state in PreviewScreen via rememberSaveable.
- Clean up unused quickSettingsIsOpen and toggleQuickSettings tests.
…osable

- Make QuickSettingsContent internal and container-independent with modifier and showMoreSettingsButton parameters.
- Flatten internal layout structure by removing intermediate QuickSettingsLayout helper.
- Add additional top padding above the 'More settings' navigation button.
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@Kimblebee
Kimblebee changed the base branch from main to kim/refactor/quickSettings/button-rows August 27, 2026 22:12
@Kimblebee
Kimblebee force-pushed the kim/refactor/quickSettings/bottomSheet-refactor branch from 6b1fec0 to a4bf36f Compare August 27, 2026 22:39
@Kimblebee
Kimblebee requested a review from temcguir August 27, 2026 22:41
Integrate Material 3 BottomSheetScaffold into CaptureLayout to render
Quick Settings in-hierarchy and avoid window-spawning modal overlays.
Make the drag handle pill clickable for dismiss, and update test helpers
to close the sheet deterministically.
@Kimblebee
Kimblebee force-pushed the kim/refactor/quickSettings/bottomSheet-refactor branch from a4bf36f to 8544a5b Compare August 27, 2026 22:58
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