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

Context menu height does not account for windowMargin #3513

Open
dzirbel opened this issue Aug 16, 2023 · 0 comments
Open

Context menu height does not account for windowMargin #3513

dzirbel opened this issue Aug 16, 2023 · 0 comments
Assignees
Labels
bug Something isn't working desktop

Comments

@dzirbel
Copy link

dzirbel commented Aug 16, 2023

Describe the bug
Popups have no mechanism to reduce their maximum width or height based on a provided margin around the window. For context menus where rememberPopupPositionProviderAtPosition() accepts an argument windowMargin, this means that (in 1.4.3) a context menu with a height exceeding the height of the window will be shifted down by this windowMargin but its height will still be the window height.

In 1.5.0-dev1152 the behavior is changed (because PopupLayout now does position.x.coerceIn(0, windowSize.width - contentSize.width)), so that the popup is not cut off, but the windowMargin is no longer respected when the popup size is larger than the window size. Ideally it would coerce the size to windowSize.width - contentSize.width) - 2 * windowMargin at most.

Affected platforms

  • Desktop

Versions

  • Kotlin version*: 1.9.0
  • Compose Multiplatform version*: 1.4.3 and 1.5.0-dev1152
  • OS version(s)* (required for Desktop and iOS issues): Linux Mint 21.2 Cinnamon
  • OS architecture (x86 or arm64): x86
  • JDK (for desktop issues): 11

To Reproduce
This isn't trivial to clearly reproduce since DefaultContextMenuRepresentation doesn't allow configuring the windowMargin and its default of 4 dp is too small to be obviously noticeable. The issue can be demonstrated on the bug/windowMargin branch of my project: https://github.com/dzirbel/compose-material-context-menu/tree/bug/windowMargin with ./gradlew :demo:run.

Of note:

  • windowMargin is provided at 30 dp here
  • it is passed to the PopupPositionProvider here

Expected behavior
Context menu sizes should be bounded by windowSize - 2 * windowMargin. This could be done in a few ways:

  • Popup() accepts a windowMargin parameter and context menus provide it (from their PopupPositionProviders)
  • PopupPositionProvider.calculatePosition() returns a Rect instead of an IntOffset and Popup uses the Rect size, allowing arbitrary margins (for example, if a popup wanted more margin on the top than the bottom)
  • etc

Screenshots
With a windowMargin of 30 dp to make the discrepancy clear (the scrollbar is all the way at the bottom); in 1.4.3:

Screenshot from 2023-08-16 14-49-07

and in 1.5.0-dev1152:

Screenshot from 2023-08-16 15-12-22

Additional context
See also discussion in #3493; cc @m-sasha

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working desktop
Projects
None yet
Development

No branches or pull requests

3 participants