Skip to content

Commit

Permalink
Fix or try fixing JS DOM display issues in BoxWithConstraints, `com…
Browse files Browse the repository at this point in the history
….huanshankeji.compose.material2.ext.TopAppBarScaffold`, and `com.huanshankeji.compose.material3.lazy.ext.List`

1. fix the bug that a direct child with `fillMaxSizeStretch` (CSS `stretch`) doesn't work properly in the `content` of `TopAppBarScaffold` that it's sometimes rendered with the height 0

   This can be triggered either when used with `BoxWithConstraints` or by changing the window size, and the expected correct behavior can be restored by inspecting the element. The 0-height issue in `BoxWithConstraints` turns out to be caused by this. The root cause for this is that `fillMaxSizeStretch`/`stretch` seems buggy when used directly in a `position: absolute` parent. This may be a bug in the browser engines used. Consider reporting this.

1. try fixing an issue that a scrollable `List` with many items take all the space of a parent `Column` and hide components in the same `Column` before it, but fail
  • Loading branch information
ShreckYe committed Dec 20, 2024
1 parent 171dd5b commit fb269bd
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package com.huanshankeji.compose.foundation

import androidx.compose.runtime.Composable
import androidx.compose.runtime.Stable
import com.huanshankeji.compose.ExperimentalApi
import com.huanshankeji.compose.foundation.layout.BoxScope
import com.huanshankeji.compose.ui.Alignment
import com.huanshankeji.compose.ui.Modifier
Expand All @@ -24,6 +25,7 @@ See https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollTop.
* it applies to the target as a whole on `androidx` targets, but applies to its content on JS DOM.
* For consistency on different platforms, [VerticalScrollBox] is recommended over this modifier.
*/
@ExperimentalApi
expect fun Modifier.verticalScroll(
state: ScrollState
/*
Expand All @@ -37,6 +39,7 @@ expect fun Modifier.verticalScroll(
* For consistency on different platforms, [HorizontalScrollBox] is recommended over this modifier.
* @see verticalScroll
*/
@ExperimentalApi
expect fun Modifier.horizontalScroll(state: ScrollState): Modifier

@Composable
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package com.huanshankeji.compose.ui.unit.ext

import androidx.annotation.IntRange
import com.huanshankeji.compose.ExperimentalApi

// not used yet
@ExperimentalApi
sealed class DpOrPercentage {
class Dp(val dp: androidx.compose.ui.unit.Dp) : DpOrPercentage()
class Percentage(@IntRange(0, 100) val percentage: Int) : DpOrPercentage()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ actual object ScrollState
actual fun Modifier.verticalScroll(state: ScrollState): Modifier =
platformModify { verticalScroll() }


actual fun Modifier.horizontalScroll(state: ScrollState): Modifier =
platformModify { horizontalScroll() }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,36 +18,30 @@ actual fun BoxWithConstraints(
contentAlignment: Alignment,
content: @Composable BoxWithConstraintsScope.() -> Unit
) {
// `DpClientSize.unspecified` instead of null can be used by default to prevent the content from not rendering when `clientSize` is not set
var clientSize by remember { mutableStateOf<ClientSize?>(null) }
// `DivBox` doesn't work here either, so it should not be Kobweb's problem.
Box(
Modifier.fillMaxSizeStretch()
.platformModify {
attrsModifier {
ref {
//clientSize = ClientSize(it.clientWidth, it.clientHeight) // Adding this doesn't make a difference to solve the issue below.
//console.log("Initial client size: ${it.clientWidth}, ${it.clientHeight}")
// Adding this doesn't make a difference in solving the issue below.
//clientSize = ClientSize(it.clientWidth, it.clientHeight)
val resizeObserver = ResizeObserver { entries, _ ->
val element = entries.first().target
val element = entries.single().target

/*
console.log("width: ${element.clientWidth}, height: ${element.clientHeight}")
console.log(entries.first().contentBoxSize.first())
console.log(entries.first().borderBoxSize.first())
console.log(entries.first().devicePixelContentBoxSize.first())
console.log(entries.first().devicePixelContentBoxSize.first()) // If there is zoom this one is different from the 2 above.
*/

/* FIXME The height is sometimes 0 when resizing,
a non-zero size (as filtered through below) is not observed in time,
and sometimes a child element doesn't show,
until it's inspected with the Chrome Dev Tools.
I don't know whether this is a browser bug or a bug in our implementation.
Therefore, the 0 size changes are filtered out.
Uncomment the commented `console.log` debug code above to debug this further. */
with(element) {
if (clientWidth != 0 && clientHeight != 0) {
//console.log("width: ${element.clientWidth}, height: ${element.clientHeight}")
clientSize = ClientSize(clientWidth, clientHeight)
}
//console.log("width: $clientWidth, height: $clientHeight")
clientSize = ClientSize(clientWidth, clientHeight)
}
}
resizeObserver.observe(it)
Expand Down Expand Up @@ -78,3 +72,13 @@ class BoxWithConstraintsScopeImpl(
) : BoxWithConstraintsScope

private class ClientSize(val clientWidth: Int, val clientHeight: Int)

// removed if not used
/**
* Made in [Dp] so [Dp.Unspecified] can be used.
*/
private class DpClientSize(val clientWidth: Dp, val clientHeight: Dp) {
companion object {
val unspecified = DpClientSize(Dp.Unspecified, Dp.Unspecified)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,8 @@ actual fun TopAppBarScaffold(
actions,
Modifier.weight(1f).fillMaxWidthStretch()
) {
// This part has a lot of nested `Div`s but works. Do not change unless you are sure that expected behavior is not broken.

// The content gets hidden behind the top app bar if this div is not added.
Div({
style {
Expand All @@ -134,9 +136,16 @@ actual fun TopAppBarScaffold(
//overflow(Overflow.Auto) // This seems not needed. TODO remove if confirmed to be not needed
}
}) {
// see `ScaffoldLayoutWithMeasureFix`
val innerPadding = PaddingValues()
content(innerPadding)
// This nested `Div` is here so that a child using `fillMaxSizeStretch` works properly. `fillMaxSizeStretch` seems buggy when used directly in the `position: absolute` parent.
Div({
style {
height(100.percent)
}
}) {
// see `ScaffoldLayoutWithMeasureFix`
val innerPadding = PaddingValues()
content(innerPadding)
}
}

floatingActionButton?.let { fabWithPosition(it) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,12 @@ actual class ListScope(val mdListScope: MdListScope) {

actual class ItemScope(val mdListItemScope: MdListItemScope)

/*
@Composable
fun PrimitiveList() =
TODO() as Unit
*/

@Composable
actual fun List(
modifier: Modifier,
Expand Down

0 comments on commit fb269bd

Please sign in to comment.