Merged
Conversation
The UnifiedPlayerSheet was calculating its own system window insets, which caused an issue where an extra space would appear at the bottom of the screen after a system theme/color change. This was especially noticeable when using gesture navigation. This change fixes the issue by hoisting the inset calculation up to the MainActivity, which then passes the correct bottom inset as a parameter to the UnifiedPlayerSheet. This ensures the component always uses the correct, up-to-date inset value provided by the system via the Scaffold's padding.
The previous fix of hoisting the inset calculation was not sufficient, as the insets provided by Scaffold's padding were still incorrect after a theme change. This new approach takes a more direct route, getting the navigation bar insets directly from the `WindowInsets.navigationBars` API within MainActivity. This value is then passed down to the UnifiedPlayerSheet. This ensures the component uses the raw, correct inset value from the system, which should be more reliable and not affected by timing issues during recomposition caused by theme changes.
This commit fixes a persistent UI bug where an extra space would appear at the bottom of the screen after a theme change. The root cause was identified as a stale window inset value being provided by the system. To definitively solve this, the layout has been refactored: 1. The `PlayerInternalNavigationBar` has been removed from the `UnifiedPlayerSheet`. 2. The `PlayerInternalNavigationBar` is now placed in the `bottomBar` of the main `Scaffold` in `MainActivity`. This delegates inset handling to the Scaffold, which is the idiomatic and robust way to handle this in Compose. 3. The animation state (`playerContentExpansionFraction`) has been hoisted to the `PlayerViewModel` to allow the now-separate player sheet and nav bar to synchronize their animations. This new architecture makes the layout resilient to the underlying system bug and aligns the implementation with Compose best practices.
After refactoring the player sheet and navigation bar into separate components, the spacing between them was too large. This commit fixes the issue by: 1. Removing a redundant `Spacer` from within the `UnifiedPlayerSheet` component. 2. Introducing a new `MiniPlayerBottomSpacer` constant (6.dp) in `MainActivity` to control the spacing. 3. Adjusting the `translationY` calculation for the `UnifiedPlayerSheet` to use this new constant, ensuring the correct spacing is applied.
This commit provides a definitive fix for a persistent UI bug related to the UnifiedPlayerSheet's positioning and spacing. The bug manifested as a large gap appearing at the bottom of the screen on theme changes, and incorrect spacing after a major layout refactoring. The solution involved several steps: 1. **Decoupling the Navigation Bar:** The `PlayerInternalNavigationBar` was removed from the `UnifiedPlayerSheet` and placed in the `Scaffold`'s `bottomBar` in `MainActivity`. This delegates all system inset handling to the `Scaffold`, which is the robust and idiomatic approach in Jetpack Compose. 2. **Hoisting Animation State:** To keep the animations of the now-separate player sheet and navigation bar synchronized, the `playerContentExpansionFraction` `Animatable` was hoisted into the `PlayerViewModel`. 3. **Centralizing Position Calculation:** The `UnifiedPlayerSheet` was refactored to be a 'dumb' component regarding its position. It no longer calculates its own target Y position. Instead, `MainActivity` is now the single source of truth, calculating the `sheetCollapsedTargetY` and passing it as a parameter. 4. **Fixing Spacing:** A configurable `MiniPlayerBottomSpacer` (6.dp) was introduced in `MainActivity` to precisely control the gap between the collapsed mini-player and the top of the bottom navigation bar, as per the user's request. Redundant internal spacers in the player sheet were removed. This new architecture is resilient to the underlying system inset bugs and provides a clean, predictable, and maintainable UI structure.
This commit addresses two follow-up layout issues after a major refactoring of the player sheet and navigation bar: 1. The spacing between the collapsed mini-player and the bottom navigation bar was too large. 2. The fully expanded player did not extend to the bottom of the screen, leaving a gap. The fixes are as follows: - **Spacing:** A redundant `Spacer` was removed from `UnifiedPlayerSheet`. A new `MiniPlayerBottomSpacer` constant (6.dp) was introduced in `MainActivity` to precisely control the collapsed sheet's position, making the spacing correct and configurable. - **Expanded Height:** The root cause of the gap below the expanded player was that the sheet was animating its height relative to the full screen, not its container within the `Scaffold`. This was fixed by using `BoxWithConstraints` in `MainActivity` to measure the correct available height and passing it to `UnifiedPlayerSheet` to use as its animation target. - **Final Cleanup:** The `UnifiedPlayerSheet` component was further simplified by removing now-unused parameters and logic, making it a truly 'dumb' component concerning its position and size, which are fully controlled by `MainActivity`.
This commit fixes a final layout issue where a small portion of the bottom navigation bar remained visible when the player was fully expanded. The root cause was a minor discrepancy between the height calculated in `MainActivity` for the animation and the actual rendered height of the `PlayerInternalNavigationBar` component. The fix is to make the animation logic more robust: - The `PlayerInternalNavigationBar` now measures its own height using `Modifier.onSizeChanged`. - This self-measured, exact height is then used to calculate the `translationY` for the hiding animation. - This ensures the component always translates itself by its own precise height, guaranteeing it moves completely off-screen. - The now-redundant height calculation was removed from `MainActivity`.
This commit fixes a layout issue where the floating-style navigation bar (`NavBarStyle.DEFAULT`) did not have the correct bottom padding to account for the system navigation area (either gestures or 3-button bar). The fix involves changing the content alignment within the `PlayerInternalNavigationBar`'s container `Box` to `Alignment.TopCenter`. Previously, it was using `Alignment.Center`, which incorrectly distributed the space for the system inset (`navBarInset`) both above and below the navigation items. By aligning the content to the top, the `navBarInset` space is now correctly applied only to the bottom of the component, effectively acting as the required bottom padding.
This commit fixes the final layout issue with the floating navigation bar. The padding for the system navigation area was being applied inside the bar's background, which is incorrect for a floating component. The fix refactors the `bottomBar` in `MainActivity.kt`: - An outer `Box` is now responsible for the hide animation and measuring the component's size. - An inner `Surface` acts as the visible, floating navigation bar. - The crucial change is that the bottom padding (`systemNavBarInset`) and horizontal padding are now applied to the outer `Box`, ensuring the `Surface` is positioned correctly away from the screen edges, achieving the desired floating effect. - The `PlayerInternalNavigationBar` composable has been simplified to just render the row of items, with all container and animation logic handled in `MainActivity`.
This commit fixes a layout regression where the bottom padding for the system navigation area was being applied to all navigation bar styles. The padding should only be applied to the `DEFAULT` (floating) style, not the `FULL_WIDTH` style. The fix introduces a conditional check in `MainActivity`'s `bottomBar` composable. The bottom padding is now only applied to the navigation bar's container `Surface` when `navBarStyle` is `DEFAULT`. For all other styles, the bottom padding is `0.dp`.
…ion bar This commit fixes the final layout issue with the navigation bar, which required different padding strategies for its two modes: - **DEFAULT (floating) mode:** Now it has padding applied *outside* its container. This elevates the entire floating component above the system navigation area, as expected. - **FULL_WIDTH mode:** Now you have padding applied *inside* your container. This allows the component's background to extend to the edge of the screen, while keeping its content (icons) correctly separated from the edge. This is achieved by passing the `systemNavBarInset` to the `PlayerInternalNavigationBar` and then applying the padding conditionally in two different places: 1. Externally to the `Surface` in `MainActivity` for `DEFAULT` mode. 2. Internally to the `Row` of icons in `PlayerInternalNavigationBar` for `FULL_WIDTH` mode.
This commit fixes the final layout issue where the navigation bar in "default" (floating) mode appeared shorter than its specified height (`NavBarContentHeight`). The issue was that the `height` modifier was being applied to the content of the navigation bar's `Surface` container, not the `Surface` itself. This caused the `Surface` to wrap its content, ignoring the intended height. The fix moves the `.height()` modifier to the `Surface` in `MainActivity`. The `PlayerInternalNavigationBar` composable inside now uses `Modifier.fillMaxSize()` to correctly fill the space of its container, ensuring the visual height of the component is correct.
This commit refactors how the bottom spacing for the `MiniPlayer` is handled and updates the heights of the internal navigation bar. The main changes are: - `MiniPlayerBottomSpacer` (8.dp) is now defined in `UnifiedPlayerSheet.kt` instead of `MainActivity.kt`. - The `MiniPlayer`'s bottom margin calculation in `MainActivity.kt` now uses this new constant. - The `NavBarContentHeight` has been increased from 66.dp to 106.dp. - The `NavBarContentHeightFullWidth` has been increased from 84.dp to 94.dp. - `CollapsedPlayerContentSpacerHeight` has been removed from `MainActivity.kt` as it's no longer used.
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
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.
No description provided.