Fix: EmptyContentScreen visibility issue in landscape mode#2009
Fix: EmptyContentScreen visibility issue in landscape mode#2009rahul31124 wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThe first overload of Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes 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 unit tests (beta)
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}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
core/ui/src/commonMain/kotlin/org/mifospay/core/ui/EmptyContentScreen.kt (2)
95-161:⚠️ Potential issue | 🟠 MajorSame landscape visibility issue not fixed in the button overload.
This second overload (
EmptyContentScreenwithbtnText/btnIcon/onClick) renders an even taller layout (extra spacer +MifosButton) but did not receive theverticalScrollchange. In landscape it will exhibit the same content-cut-off behavior described in MW-399. Apply the same scrolling treatment here for consistency, otherwise users hitting empty states that include an action button will still see clipped content.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/ui/src/commonMain/kotlin/org/mifospay/core/ui/EmptyContentScreen.kt` around lines 95 - 161, The second overload of EmptyContentScreen (the composable that accepts btnText, btnIcon, onClick and renders imageContent plus MifosButton) lacks the vertical scrolling added elsewhere and can be clipped in landscape; fix it by making the inner Column that currently has .padding(...).fillMaxSize().testTag("mifos:empty") scrollable — add .verticalScroll(rememberScrollState()) (importing rememberScrollState) to that Column (or wrap its contents in a Column/Box with verticalScroll) so the imageContent, Spacers, Texts and MifosButton are scrollable in constrained heights. Ensure you reference EmptyContentScreen, imageContent, and MifosButton when locating the change.
52-92:⚠️ Potential issue | 🟠 Major
verticalScrollwith innerfillMaxSize()silently breaks vertical centering.
verticalScrollmeasures children with unbounded max height. Two consequences in this layout:
- The outer
Column'sverticalArrangement = Arrangement.Center(line 56) becomes a no-op — there is no "remaining" space to distribute inside an unbounded scroll viewport.- The inner
Column'sModifier.fillMaxSize()(line 61) cannot fill height under unbounded constraints, so it collapses to wrap-content height; its ownverticalArrangement = Arrangement.Center(line 63) also stops centering.Net effect: when content fits on screen (the common portrait case), the title/subtitle/image will now hug the top instead of being vertically centered as before. To keep centering when content fits and scroll when it overflows, give the scroll content a min height equal to the viewport, e.g. via
BoxWithConstraints/heightIn(min = maxHeight), and usefillMaxWidth()(notfillMaxSize()) on the inner column.♻️ Suggested fix to preserve centering while supporting scroll
- Column( - modifier = modifier.fillMaxSize() - .verticalScroll(rememberScrollState()), - horizontalAlignment = Alignment.CenterHorizontally, - verticalArrangement = Arrangement.Center, - ) { - Column( - modifier = Modifier - .padding(KptTheme.spacing.md) - .fillMaxSize() - .testTag("mifos:empty"), - verticalArrangement = Arrangement.Center, - horizontalAlignment = Alignment.CenterHorizontally, - ) { + BoxWithConstraints(modifier = modifier.fillMaxSize()) { + val viewportHeight = maxHeight + Column( + modifier = Modifier + .fillMaxWidth() + .verticalScroll(rememberScrollState()) + .heightIn(min = viewportHeight) + .padding(KptTheme.spacing.md) + .testTag("mifos:empty"), + verticalArrangement = Arrangement.Center, + horizontalAlignment = Alignment.CenterHorizontally, + ) { ... } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/ui/src/commonMain/kotlin/org/mifospay/core/ui/EmptyContentScreen.kt` around lines 52 - 92, The outer Column uses verticalScroll(rememberScrollState()) which makes height unbounded so Arrangement.Center becomes a no-op and the inner Column's Modifier.fillMaxSize() collapses; to fix, wrap the scroll content in a viewport-aware container (e.g., BoxWithConstraints) and apply Modifier.heightIn(min = maxHeight) to the scrolling child so it gets at least the viewport height, and change the inner Column's Modifier.fillMaxSize() to Modifier.fillMaxWidth() (keep padding/testTag) so its verticalArrangement = Arrangement.Center can actually center when content fits while still allowing scrolling when content overflows; update the composable that contains imageContent(), title and subTitle accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@core/ui/src/commonMain/kotlin/org/mifospay/core/ui/EmptyContentScreen.kt`:
- Around line 95-161: The second overload of EmptyContentScreen (the composable
that accepts btnText, btnIcon, onClick and renders imageContent plus
MifosButton) lacks the vertical scrolling added elsewhere and can be clipped in
landscape; fix it by making the inner Column that currently has
.padding(...).fillMaxSize().testTag("mifos:empty") scrollable — add
.verticalScroll(rememberScrollState()) (importing rememberScrollState) to that
Column (or wrap its contents in a Column/Box with verticalScroll) so the
imageContent, Spacers, Texts and MifosButton are scrollable in constrained
heights. Ensure you reference EmptyContentScreen, imageContent, and MifosButton
when locating the change.
- Around line 52-92: The outer Column uses verticalScroll(rememberScrollState())
which makes height unbounded so Arrangement.Center becomes a no-op and the inner
Column's Modifier.fillMaxSize() collapses; to fix, wrap the scroll content in a
viewport-aware container (e.g., BoxWithConstraints) and apply
Modifier.heightIn(min = maxHeight) to the scrolling child so it gets at least
the viewport height, and change the inner Column's Modifier.fillMaxSize() to
Modifier.fillMaxWidth() (keep padding/testTag) so its verticalArrangement =
Arrangement.Center can actually center when content fits while still allowing
scrolling when content overflows; update the composable that contains
imageContent(), title and subTitle accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f191bf32-693b-4695-b02d-787c4f52b15e
📒 Files selected for processing (1)
core/ui/src/commonMain/kotlin/org/mifospay/core/ui/EmptyContentScreen.kt
2 participants
Issue Fix
Fixes #{MW-399}
Jira Task: MW-399
Screenshots
Record_2026-04-26-18-06-16_6a5d239ecc870d7d1b844aa672071987.mp4
Description
Updated the
EmptyContentScreencomponent to support scrolling in landscape orientation.Apply the
AndroidStyle.xmlstyle template to your code in Android Studio.Run the unit tests with
./gradlew checkto make sure you didn't break anythingIf you have multiple commits please combine them into one commit by squashing them.
Summary by CodeRabbit
Bug Fixes