Repository navigation
Conversation
- Redesign zoom controls with a unified stadium capsule track (56dp radius) and an animated sliding indicator pill (12dp radius). - Support both horizontal and vertical orientations via configurable parameter. - Implement label formatting (.5, 1×, 2, 5) with Unicode multiplication symbol on selected ratio. - Ensure buttons are flush with vertical sides in vertical view, with appropriate clearance in horizontal view. - Wrap zoom controls with OverlapAwareStyleProvider to dynamically toggle between 60% black (viewfinder overlap) and 20% white (outside viewfinder). - Preserve ZoomButtonRow as a backward-compatible delegate for external consumers. - Add ZoomBar unit tests and previews for both orientations and color themes.
- Reuse ThreadLocal DecimalFormat instances to eliminate object allocation churn during high-frequency pinch gestures. - Migrate animated slider offset to layout-phase IntOffset lambda to prevent full component recompositions during spring animations. - Isolate ZoomBarItem recomposition by passing currentRatio only to the active selection, enabling Compose skipping for unselected stops. - Fix sub-1.0 stop selection logic to support devices with multiple ultra-wide/macro ratios. - Add rounding boundary tests for sub-1.0 to 1.0 transitions.
… refine end padding - Replace sliding indicator offset animation with in-place container and content color transitions using animateColorAsState on individual zoom items. - Retain Modifier.animateContentSize() on label text for smooth suffix expansion. - Adjust HorizontalItemWidth to 40.dp and HorizontalEndPadding to 6.dp to ensure uniform clearance around stadium capsule end caps. - Preserve vertical orientation dimensions and flush side alignment.
Format lines exceeding max line length and wrap parameters according to ktlint and spotless standards.
…colors in ZoomButtonRow - Replace hardcoded mock colors (tan / #EBE1CF) in black60Colors and white20Colors with MaterialTheme.colorScheme.primaryFixed and onPrimaryFixed. - Expose colors parameter on ZoomButtonRow to allow external callers and implementing apps to customize track and indicator colors.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new ZoomBar component to replace the existing ZoomButtonRow, offering improved flexibility and accessibility for camera zoom controls. The review feedback focuses on optimizing performance by deferring high-frequency state reads using lambda providers to prevent unnecessary recompositions, fixing an accessibility bug where the parent container's content description obscured child text, applying restrictive internal visibility to helper functions, and ensuring test classes explicitly declare their test runners.
…y, and add test runner - Defer high-frequency primaryZoomRatio read in ZoomBar using a lambda provider and derivedStateOf in ZoomBarItem to prevent top-level recomposition during pinch gestures. - Clean up redundant contentDescription in ZoomBarItem semantics in favor of selected state. - Restrict formatZoomLabel visibility to internal. - Annotate ZoomBarComponentsTest with @RunWith(JUnit4::class).
…bility, and remove unused constant - Add Role.RadioButton to ZoomBarItem's clickable modifier to improve screen reader navigation. - Restrict all @Preview composable functions in ZoomBarComponents to private visibility. - Remove unused ZoomBarDefaults.EndPadding constant.
Invoke setRoundingMode directly on DecimalFormat instances instead of property assignment syntax to ensure compatibility across compiler toolchains and Android SDK stubs.
temcguir
left a comment
temcguir
left a comment
There was a problem hiding this comment.
Thanks for putting this together! The capsule track and the primaryFixed / onPrimaryFixed indicator look great, and the colors match the design under dynamic color. I tried it on a device and prototyped a few changes locally. The main things that came up:
- Motion: when the selection changes, the old pill disappears and a new one grows in place with a bouncy spring. It reads more naturally if a single indicator slides from one stop to the next.
- Sizing: the bar comes out smaller than the spec (172 × 44 vs 200 × 48 for four stops), and the stops end up below the 48dp touch target.
- A tap bug: after you tap a stop, the selection jumps back to the previous stop for a frame or two before settling.
- Rounding: the selected stop and its label can disagree near
.x5values.
I left inline comments with details and snippets. I also suggested Material 3 tokens in a few places where we have hard-coded values right now. There's also a broader question about whether ButtonGroup / ToggleButton could cover part of this.
Could ButtonGroup / ToggleButton cover this?
One bigger-picture question: Material 3 already has a single-select pattern that's close to this. A ButtonGroup with ToggleButtons and ButtonGroupDefaults.connectedLeadingButtonShapes() / connectedMiddleButtonShapes() / connectedTrailingButtonShapes() (we already use this in QuickSettingsComponents). There's also SingleChoiceSegmentedButtonRow. Either one would give us touch targets, semantics, theme colors, and M3 motion for free.
The part they don't do is a single indicator sliding across a shared translucent track. Each ToggleButton has its own container that morphs in place, which is basically the popping behavior I mentioned on the indicator. So I see two options:
- Use
ToggleButtonper stop and accept in-place morphing. Less custom code, and it stays closer to stock M3. - Keep the custom sliding indicator, if the slide is what design wants. In that case it's worth building it from M3 tokens (see the sizing comment) so it still feels consistent with the rest of the app.
Do you know which way design leans? I'm happy either way, but the slide is what felt right on device.
Selection churn and what TalkBack hears
This is related to the tap jump-back and animation-target comments. I expect the "jump back to the previous stop" bug to be audible as well as visible. When a TalkBack user double-taps 5, its state goes Selected, then Not selected for a frame or two, then Selected again, and TalkBack may announce each change. During the tap animation, the selected stop's text also counts up (1.1×, 1.2×, …), so if focus lands on it mid-animation, they'll hear an intermediate value. Selecting from the requested zoom ratio and showing the target label during the animation should keep both stable. I'd also like to double-check this with TalkBack on a device before we merge.
- Implement single sliding pill indicator with M3 spatial motion - Update slot and indicator dimensions for spec parity and 48dp touch targets - Decouple layout orientation from device rotation and rotate text in-place - Align vertical layout with 48dp flush-edge mock specification - Interpolate label color via lerp in draw phase based on indicator proximity - Cap system font scale at 1.3f to prevent label clipping/wrapping - Fix tap jump-back glitch by prioritizing primarySettingZoomRatio - Unify half-up decimal rounding and add localized decimal separator support - Add selectableGroup and accessible action semantics for TalkBack - Deprecate ZoomButtonRow and migrate PreviewScreen call site - Allow 1.0f tolerance for HAL quantization in CameraSession - Transfer interrupted zoom animations cleanly in ZoomStateManager
temcguir
left a comment
temcguir
left a comment
There was a problem hiding this comment.
Thanks for the update, this is a big improvement! I tried it on a device: the sliding indicator, the sizing, the tap jump-back fix, and the font-scale cap all look and behave right.
One heads up before you dig into anything: after the latest merge from main, tapping a zoom stop does nothing. That's not from this PR. It comes from #612: ZoomStateManager is now created while the loading skeleton is up, so its range is stuck at 1..1, and the new early return in animatedZoom makes that silent. A separate fix against main is in progress. With that fix applied locally, everything here works.
I left a few follow-ups inline, mostly on the existing threads.
…lity - Drop contentRotation from ZoomBar and ZoomButtonRow in favor of upright modifier - Remove unused IndicatorShape and alias IndicatorWidth to SlotSize - Update currentRatioProvider to prioritize target ratio during tap animation - Extract formatAccessibleZoomLabel with leading zero and localized decimal separator - Move multiplication symbol into zoom_stop_click_label string resource - Extract selectedZoomStopIndex and add unit tests for rounding and tie-breaking - Use imported abs in CameraSession
Summary
Refactors the camera zoom controls to match updated UI design specifications, adding configurable orientation support, overlap-aware dynamic styling, Material 3 theme-driven indicator coloring, refreshed label formatting, and memory/recomposition optimizations for pinch gestures.
Key Changes
🎨 UI & Design Parity
56dpcorner radius). Integrated withOverlapAwareStyleProviderto dynamically toggle between 60% black (viewfinder overlap) and 20% white (outside viewfinder).12dpcorner radius) styled with standard Material 3 theme tokens (primaryFixedcontainer andonPrimaryFixedcontent) across all color palettes, allowing implementing apps to theme the indicator naturally.ZoomBarColorsconfiguration exposed on bothZoomBarandZoomButtonRow, giving callers total control over track, indicator, and label colors..5), omit suffixes on unselected stops (.5,1,2,5), and append the Unicode multiplication symbol only to the active ratio (1×,2×).Orientation.HorizontalandOrientation.Verticallayouts:HorizontalClearance = 4.dp,HorizontalEndPadding = 6.dp) to ensure even spacing around the curved end caps.40.dpwidth,0.dphorizontal padding).ZoomButtonRowas an inline delegate to ensure existing call sites and external consumers continue to work seamlessly.⚡ Performance & Recomposition Optimizations
DecimalFormatinstantiations withThreadLocalformatters, eliminating object allocation churn during high-frequency pinch-to-zoom updates.Testing
ZoomBarComponentsTestcovering label formatting, fractional stops, and boundary conditions