Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix DesktopDropdownMenuPositionProvider to align with the correct horizontal side of the window #555

Merged
merged 2 commits into from
May 11, 2023
Merged
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 @@ -129,8 +129,7 @@ fun DropdownMenu(
}

/**
* A variant of a dropdown menu that accepts a [DropdownMenuState] instead of directly using the
* mouse position.
* A variant of a dropdown menu that accepts a [DropdownMenuState] to allow precise positioning.
*
* Typically, it should be combined with [Modifier.contextMenuOpenDetector] via state-hoisting.
*
Expand Down Expand Up @@ -362,7 +361,7 @@ fun Modifier.contextMenuOpenDetector(
return if (enabled) {
this.contextMenuOpenDetector(
key = state,
enabled = enabled && (state.status is DropdownMenuState.Status.Closed)
enabled = state.status is DropdownMenuState.Status.Closed
) { pointerPosition ->
state.status = DropdownMenuState.Status.Open(pointerPosition)
}
Expand All @@ -371,6 +370,9 @@ fun Modifier.contextMenuOpenDetector(
}
}

/**
* Positions a dropdown relative to another widget (its anchor).
*/
@Immutable
internal data class DesktopDropdownMenuPositionProvider(
val contentOffset: DpOffset,
Expand All @@ -383,31 +385,53 @@ internal data class DesktopDropdownMenuPositionProvider(
layoutDirection: LayoutDirection,
popupContentSize: IntSize
): IntOffset {

val isLtr = layoutDirection == LayoutDirection.Ltr

// Coerce such that this..this+size fits into min..max; if impossible, align with min
fun Int.coerceWithSizeIntoRangePreferMin(size: Int, min: Int, max: Int) = when {
this < min -> min
this + size > max -> max - size
else -> this
}

// Coerce such that this..this+size fits into min..max; if impossible, align with max
fun Int.coerceWithSizeIntoRangePreferMax(size: Int, min: Int, max: Int) = when {
this + size > max -> max - size
this < min -> min
else -> this
}

fun Int.coerceWithSizeIntoRange(size: Int, min: Int, max: Int) = when {
isLtr -> coerceWithSizeIntoRangePreferMin(size, min, max)
else -> coerceWithSizeIntoRangePreferMax(size, min, max)
}

// The min margin above and below the menu, relative to the screen.
val verticalMargin = with(density) { MenuVerticalMargin.roundToPx() }
// The content offset specified using the dropdown offset parameter.
val contentOffsetX = with(density) { contentOffset.x.roundToPx() }
val contentOffsetY = with(density) { contentOffset.y.roundToPx() }

// Compute horizontal position.
val toRight = anchorBounds.left + contentOffsetX
val toLeft = anchorBounds.right - contentOffsetX - popupContentSize.width
val toDisplayRight = windowSize.width - popupContentSize.width
val toDisplayLeft = 0
val x = if (layoutDirection == LayoutDirection.Ltr) {
sequenceOf(toRight, toLeft, toDisplayRight)
} else {
sequenceOf(toLeft, toRight, toDisplayLeft)
}.firstOrNull {
it >= 0 && it + popupContentSize.width <= windowSize.width
} ?: toLeft
val preferredX = if (isLtr) {
anchorBounds.left + contentOffsetX
}
else {
anchorBounds.right - contentOffsetX - popupContentSize.width
}
val x = preferredX.coerceWithSizeIntoRange(
size = popupContentSize.width,
min = 0,
max = windowSize.width
)

// Compute vertical position.
val toBottom = maxOf(anchorBounds.bottom + contentOffsetY, verticalMargin)
val toTop = anchorBounds.top - contentOffsetY - popupContentSize.height
val toCenter = anchorBounds.top - popupContentSize.height / 2
val toDisplayBottom = windowSize.height - popupContentSize.height - verticalMargin
var y = sequenceOf(toBottom, toTop, toCenter, toDisplayBottom).firstOrNull {
val toWindowBottom = windowSize.height - popupContentSize.height - verticalMargin
var y = sequenceOf(toBottom, toTop, toCenter, toWindowBottom).firstOrNull {
it >= verticalMargin &&
it + popupContentSize.height <= windowSize.height - verticalMargin
} ?: toTop
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,41 +55,130 @@ class DesktopMenuTest {
val rule = createComposeRule()

private val windowSize = IntSize(100, 100)
private val anchorPosition = IntOffset(10, 10)
private val anchorSize = IntSize(80, 20)

// Standard case: enough room to position below the anchor and align left
@Test
fun menu_positioning_vertical_underAnchor() {
val popupSize = IntSize(80, 70)
fun menu_positioning_alignLeft_belowAnchor() {
val anchorBounds = IntRect(
offset = IntOffset(10, 10),
size = IntSize(50, 20)
)
val popupSize = IntSize(70, 70)

val position = DesktopDropdownMenuPositionProvider(
DpOffset.Zero,
Density(1f)
).calculatePosition(
anchorBounds,
windowSize,
LayoutDirection.Ltr,
popupSize
)

assertThat(position).isEqualTo(anchorBounds.bottomLeft)
}

// Standard RTL case: enough room to position below the anchor and align right
@Test
fun menu_positioning_rtl_alignRight_belowAnchor() {
val anchorBounds = IntRect(
offset = IntOffset(30, 10),
size = IntSize(50, 20)
)
val popupSize = IntSize(70, 70)

val position = DesktopDropdownMenuPositionProvider(
DpOffset.Zero,
Density(1f)
).calculatePosition(
anchorBounds,
windowSize,
LayoutDirection.Rtl,
popupSize
)

assertThat(position).isEqualTo(
IntOffset(
x = anchorBounds.right - popupSize.width,
y = anchorBounds.bottom
)
)
}

// Not enough room to position the popup below the anchor, but enough room above
@Test
fun menu_positioning_alignLeft_aboveAnchor() {
val anchorBounds = IntRect(
offset = IntOffset(10, 50),
size = IntSize(50, 30)
)
val popupSize = IntSize(70, 30)

val position = DesktopDropdownMenuPositionProvider(
DpOffset.Zero,
Density(1f)
).calculatePosition(
IntRect(anchorPosition, anchorSize),
anchorBounds,
windowSize,
LayoutDirection.Ltr,
popupSize
)

assertThat(position).isEqualTo(IntOffset(10, 30))
assertThat(position).isEqualTo(
IntOffset(
x = anchorBounds.left,
y = anchorBounds.top - popupSize.height
)
)
}

// Anchor left is at negative coordinates, so align popup to the left of the window
@Test
fun menu_positioning_vertical_windowTop() {
val popupSize = IntSize(80, 100)
fun menu_positioning_windowLeft_belowAnchor() {
val anchorBounds = IntRect(
offset = IntOffset(-10, 10),
size = IntSize(50, 20)
)
val popupSize = IntSize(70, 50)

val position = DesktopDropdownMenuPositionProvider(
DpOffset.Zero,
Density(1f)
).calculatePosition(
IntRect(anchorPosition, anchorSize),
anchorBounds = anchorBounds,
windowSize,
LayoutDirection.Ltr,
popupSize
)

assertThat(position).isEqualTo(IntOffset(10, 0))
assertThat(position).isEqualTo(IntOffset(0, anchorBounds.bottom))
}

// (RTL) Anchor right is beyond the right of the window, so align popup to the window right
@Test
fun menu_positioning_rtl_windowRight_belowAnchor() {
val anchorBounds = IntRect(
offset = IntOffset(30, 10),
size = IntSize(80, 20)
)
val popupSize = IntSize(50, 70)

val position = DesktopDropdownMenuPositionProvider(
DpOffset.Zero,
Density(1f)
).calculatePosition(
anchorBounds,
windowSize,
LayoutDirection.Rtl,
popupSize
)

assertThat(position).isEqualTo(
IntOffset(
x = windowSize.width - popupSize.width,
y = anchorBounds.bottom
)
)
}

@OptIn(ExperimentalComposeUiApi::class)
Expand Down