Implement collapsible top bar and improve state persistence#12
Merged
Conversation
- Hoist scroll state to manage dynamic top bar animations. - Refactor TopBar into modular components (FilterRow, FilterChip). - Use rememberSaveable for UI state persistence across configuration changes.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR adds reusable filter-chip components, refactors DemoTopBar to animate/collapse based on a hoisted ScrollState, and updates Application.kt to hoist the scroll state, persist selected UI fields with rememberSaveable, and adjust safe-drawing insets. ChangesScroll-Collapsing Header Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
example/src/commonMain/kotlin/io/github/komodgn/example/component/DemoTopBar.kt (1)
55-56: ⚡ Quick winConsider using density-aware threshold.
The
scrollThreshold = 120fuses raw pixels, which means the collapse behavior will feel different on screens with varying densities. For consistent UX across devices, consider converting dp to pixels usingLocalDensity.📏 Proposed fix for density-aware threshold
+import androidx.compose.ui.platform.LocalDensity + `@Suppress`("FrequentlyChangingValue") `@Composable` fun DemoTopBar( scrollState: ScrollState, isDark: Boolean, onToggleTheme: () -> Unit, selectedComponent: DemoComponent, onComponentSelect: (DemoComponent) -> Unit, ) { - val scrollThreshold = 120f + val density = LocalDensity.current + val scrollThreshold = with(density) { 120.dp.toPx() } val collapseFraction = (scrollState.value / scrollThreshold).coerceIn(0f, 1f)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@example/src/commonMain/kotlin/io/github/komodgn/example/component/DemoTopBar.kt` around lines 55 - 56, The fixed code should replace the hardcoded pixel threshold with a density-aware value: convert 120.dp to pixels using LocalDensity (e.g., within DemoTopBar's composable scope use LocalDensity.current to compute scrollThresholdPx = with(LocalDensity.current) { 120.dp.toPx() }) and then compute collapseFraction = (scrollState.value / scrollThresholdPx).coerceIn(0f, 1f); ensure you reference scrollThresholdPx instead of the old scrollThreshold and import/use LocalDensity and dp in DemoTopBar.kt.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@example/src/commonMain/kotlin/io/github/komodgn/example/component/DemoTopBar.kt`:
- Line 86: The text for the top-bar label has inconsistent spacing: when
selectedComponent == DemoComponent.CODE_VIEW it uses " CodeView" with a leading
space while the else branch uses "CodeEditor"; update the conditional in
DemoTopBar (the expression that builds text from selectedComponent) to remove
the leading space so both branches use consistently formatted strings (e.g.,
"CodeView" and "CodeEditor") ensuring uniform visual alignment.
- Line 46: The suppression on DemoTopBar hides a real Compose lint about reading
a frequently-changing value (likely scrollState) during composition; remove
`@Suppress`("FrequentlyChangingValue") and instead read the changing value via a
stable derived value or side-effect: e.g., create a
derivedStateOf(filter/threshold) based on scrollState.value and use that in the
composition, or observe scrollState with snapshotFlow inside a LaunchedEffect to
update a mutable state used for non-recomposing effects; locate the code in
DemoTopBar where scrollState (or other frequently-changing property) is read in
the composition lambda and replace the direct read with derivedStateOf or
snapshotFlow+LaunchedEffect so recompositions are limited.
---
Nitpick comments:
In
`@example/src/commonMain/kotlin/io/github/komodgn/example/component/DemoTopBar.kt`:
- Around line 55-56: The fixed code should replace the hardcoded pixel threshold
with a density-aware value: convert 120.dp to pixels using LocalDensity (e.g.,
within DemoTopBar's composable scope use LocalDensity.current to compute
scrollThresholdPx = with(LocalDensity.current) { 120.dp.toPx() }) and then
compute collapseFraction = (scrollState.value / scrollThresholdPx).coerceIn(0f,
1f); ensure you reference scrollThresholdPx instead of the old scrollThreshold
and import/use LocalDensity and dp in DemoTopBar.kt.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0d43c83e-1c1c-48e3-8476-20edc2ec40fa
📒 Files selected for processing (5)
.github/workflows/docs.ymlexample/src/commonMain/kotlin/io/github/komodgn/example/Application.ktexample/src/commonMain/kotlin/io/github/komodgn/example/component/DemoComponentFilterRow.ktexample/src/commonMain/kotlin/io/github/komodgn/example/component/DemoFilterChip.ktexample/src/commonMain/kotlin/io/github/komodgn/example/component/DemoTopBar.kt
…ent/DemoTopBar.kt Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
- Optimize top bar performance using derivedStateOf and deferred reading.
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
1 participant
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.
Summary by CodeRabbit
New Features
Improvements