Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import androidx.compose.material3.VerticalDragHandle
import androidx.compose.material3.adaptive.ExperimentalMaterial3AdaptiveApi
import androidx.compose.material3.adaptive.layout.ListDetailPaneScaffold
import androidx.compose.material3.adaptive.layout.ListDetailPaneScaffoldRole
import androidx.compose.material3.adaptive.layout.PaneExpansionAnchor
import androidx.compose.material3.adaptive.layout.PaneExpansionState
import androidx.compose.material3.adaptive.layout.rememberPaneExpansionState
import androidx.compose.material3.adaptive.navigation.ThreePaneScaffoldNavigator
Expand All @@ -52,10 +53,15 @@ import androidx.compose.runtime.rememberCoroutineScope
import androidx.compose.runtime.saveable.rememberSaveable
import androidx.compose.runtime.setValue
import androidx.compose.ui.Modifier
import androidx.compose.ui.draw.clipToBounds
import androidx.compose.ui.layout.layout

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Add the import for clipToBounds to support the custom minimum width layout modifier.

Suggested change
import androidx.compose.ui.layout.layout
import androidx.compose.ui.draw.clipToBounds
import androidx.compose.ui.layout.layout

import androidx.compose.ui.platform.LocalContext
import androidx.compose.ui.platform.testTag
import androidx.compose.ui.res.painterResource
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.unit.Dp
import androidx.compose.ui.unit.dp
import androidx.compose.ui.zIndex
import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel
import androidx.ink.strokes.Stroke
import androidx.lifecycle.compose.collectAsStateWithLifecycle
Expand All @@ -70,6 +76,31 @@ import com.example.cahier.features.home.viewmodel.HomeScreenViewModel
import com.example.cahier.features.home.viewmodel.NoteListUiState
import kotlinx.coroutines.launch

private val MinPaneWidth = 250.dp

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

To avoid duplicating the complex .layout modifier logic in both listPane and detailPane, and to fix potential layout bugs, we should extract it into a reusable Modifier extension function.

Key Improvements:

  1. Avoid Duplication: Keeps the code DRY and maintainable.
  2. Prevent Overlap/Bleeding: When a pane's allocated width is smaller than MinPaneWidth (e.g., during resizing or when collapsing), the child would normally draw outside its bounds and overlap adjacent panes. Applying clipToBounds() prevents this visual bleeding.
  3. Handle Collapsed State: If constraints.maxWidth == 0 (fully collapsed), we should avoid measuring and drawing the child at MinPaneWidth to prevent it from overlapping other visible content.
private val MinPaneWidth = 250.dp

private fun Modifier.minimumWidthLayout(minWidth: androidx.compose.ui.unit.Dp): Modifier = this
    .clipToBounds()
    .layout { measurable, constraints ->
        if (constraints.maxWidth == 0) {
            layout(0, 0) {}
        } else {
            val fixedWidth = minWidth.roundToPx()
            val placeable = measurable.measure(
                constraints.copy(
                    minWidth = fixedWidth,
                    maxWidth = maxOf(fixedWidth, constraints.maxWidth)
                )
            )
            val reportedWidth = constraints.maxWidth
            layout(reportedWidth, placeable.height) {
                placeable.placeRelative(0, 0)
            }
        }
    }



// The layout modifier implements a minimum width for the pane while
// allowing it to report its original width to the scaffold, preventing
// content from being overly compressed during resizing.
private fun Modifier.minimumWidthLayout(minWidth: Dp): Modifier = this
.clipToBounds()
.layout { measurable, constraints ->
if (constraints.maxWidth == 0) {
layout(0, 0) {}
} else {
val fixedWidth = minWidth.roundToPx()
val placeable = measurable.measure(
constraints.copy(
minWidth = fixedWidth,
maxWidth = maxOf(fixedWidth, constraints.maxWidth)
)
)
val reportedWidth = constraints.maxWidth
layout(reportedWidth, placeable.height) {
placeable.placeRelative(0, 0)
}
}
}

object HomeDestination : NavigationDestination {
override val route = "home"
Expand Down Expand Up @@ -108,16 +139,31 @@ fun HomePane(
modifier: Modifier = Modifier,
forceCompact: Boolean? = null,
homeScreenViewModel: HomeScreenViewModel = hiltViewModel(),
) {
) {
var currentDestination by rememberSaveable { mutableStateOf(AppDestinations.Home) }
val navigator = rememberListDetailPaneScaffoldNavigator<Note>()
val noteList by homeScreenViewModel.noteList.collectAsStateWithLifecycle()
val selectedNoteUIState by homeScreenViewModel.uiState.collectAsStateWithLifecycle()
val coroutineScope = rememberCoroutineScope()
val activity = LocalActivity.current
val windowSizeClass = activity?.let { calculateWindowSizeClass(it) }
val paneExpansionState = rememberPaneExpansionState()
var hasSetInitialProportion by remember {
val paneExpansionState = rememberPaneExpansionState(
keyProvider = navigator.scaffoldValue,
anchors = listOf(
// 1. Fully collapsed state (0% screen fraction)
PaneExpansionAnchor.Proportion(proportion = 0f),

// 2. The 25 % minimum width snapping boundary anchor
PaneExpansionAnchor.Proportion(proportion = 0.25f),

// 3. Middle split state (50% screen fraction)
PaneExpansionAnchor.Proportion(proportion = 0.5f),

// 4. Fully expanded list state (100% screen fraction, detail collapsed)
PaneExpansionAnchor.Proportion(proportion = 1f)
)
)
Comment on lines +150 to +165

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

There is a discrepancy between the defined anchors (0f, 0.25f, 0.5f, 1f) and the initial pane proportion of 0.3f set in the LaunchedEffect (line 175).

Because 0.3f is not one of the snapping anchors, the pane might immediately snap to 0.25f or 0.5f as soon as the user starts dragging the handle, leading to a jumpy user experience. Consider aligning the initial proportion with one of the defined anchors (e.g., 0.25f or 0.5f), or adding 0.3f as an anchor if it is a desired snap point.

var hasSetInitialProportion by rememberSaveable {
mutableStateOf(false)
}
val isCompact = forceCompact
Expand Down Expand Up @@ -153,7 +199,7 @@ fun HomePane(

LaunchedEffect(isCompact, hasSetInitialProportion) {
if (!isCompact && !hasSetInitialProportion) {
paneExpansionState.setFirstPaneProportion(0.3f)
paneExpansionState.setFirstPaneProportion(0.5f)
hasSetInitialProportion = true
}
}
Expand Down Expand Up @@ -236,23 +282,29 @@ private fun CahierNavigationSuite(
when (currentDestination) {
AppDestinations.Home -> {
ListDetailPaneScaffold(
modifier = Modifier.fillMaxSize(),
directive = navigator.scaffoldDirective,
value = navigator.scaffoldValue,
paneExpansionState = paneExpansionState,
paneExpansionDragHandle = { state ->
val interactionSource = remember { MutableInteractionSource() }
VerticalDragHandle(
modifier =
Modifier.paneExpansionDraggable(
state,
LocalMinimumInteractiveComponentSize
.current,
interactionSource,
),
Modifier
.paneExpansionDraggable(
state,
LocalMinimumInteractiveComponentSize
.current,
interactionSource,
)
.zIndex(2f), // Specify z-index to ensure the drag handle is drawn on top of the panes
)
},
listPane = {
ListPaneContent(
modifier = Modifier
.fillMaxSize()
.minimumWidthLayout(MinPaneWidth),
noteList = noteList.noteList,
isCompact = isCompact,
selectedNoteId = if (isCompact) null
Expand Down Expand Up @@ -294,6 +346,11 @@ private fun CahierNavigationSuite(
if (!isCompact) {
selectedNoteUIState.note.let { note ->
DetailPaneContent(
modifier = Modifier
.fillMaxSize()
// Specify z-index to ensure the detail pane content is correctly layered
.zIndex(1f)
.minimumWidthLayout(MinPaneWidth),
note = note,
strokes = selectedNoteUIState.strokes,
onClickToEdit = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,8 @@ fun NoteDetail(
Surface(
modifier = modifier.fillMaxSize(),
shape = RoundedCornerShape(12.dp),
tonalElevation = 6.dp
tonalElevation = 6.dp,
shadowElevation = 8.dp
) {
Spacer(modifier = Modifier.height(16.dp))
when (note.type) {
Expand Down
Binary file modified screenshots/reference_screenshot_navrail.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading