Skip to content

Commit

Permalink
Fix menu dividers in bridge (#361)
Browse files Browse the repository at this point in the history
The main root cause was a missing return statement in retrieveIntAsDp(),
but we also had terribly inconsistent values for the menus compared to
the actual appearance. I have tweaked the bridge menu style defaults to
align better, but we'll have to come back to revisit the whole menu
implementation to actually use the right values in the right way. The
current implementation is not satisfactory.
  • Loading branch information
rock3r authored Apr 22, 2024
1 parent 35bd7af commit d797601
Show file tree
Hide file tree
Showing 10 changed files with 67 additions and 77 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public fun List<Color>.createVerticalBrush(

public fun retrieveIntAsDp(key: String): Dp {
val rawValue = UIManager.get(key)
if (rawValue is Int) rawValue.dp
if (rawValue is Int) return rawValue.dp

keyNotFound(key, "Int")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -474,9 +474,9 @@ private fun readDefaultDropdownStyle(
metrics = DropdownMetrics(
arrowMinSize = DpSize(arrowWidth, minimumSize.height.dp),
minSize = DpSize(minimumSize.width.dp + arrowWidth, minimumSize.height.dp),
cornerSize = CornerSize(DarculaUIUtil.COMPONENT_ARC.dp),
cornerSize = CornerSize(DarculaUIUtil.COMPONENT_ARC.dp / 2),
contentPadding = retrieveInsetsAsPaddingValues("ComboBox.padding"),
borderWidth = DarculaUIUtil.BW.dp,
borderWidth = DarculaUIUtil.LW.dp,
),
icons = DropdownIcons(chevronDown = bridgePainterProvider("general/chevron-down.svg")),
textStyle = dropdownTextStyle,
Expand Down Expand Up @@ -648,24 +648,27 @@ private fun readMenuStyle(): MenuStyle {
metrics = MenuMetrics(
cornerSize = CornerSize(IdeaPopupMenuUI.CORNER_RADIUS.dp),
menuMargin = PaddingValues(0.dp),
contentPadding = PaddingValues(horizontal = 8.dp, vertical = 12.dp),
contentPadding = PaddingValues(horizontal = 0.dp, vertical = 6.dp),
offset = DpOffset(0.dp, 2.dp),
shadowSize = 12.dp,
borderWidth = retrieveIntAsDpOrUnspecified("Popup.borderWidth").takeOrElse { 1.dp },
borderWidth = retrieveIntAsDpOrUnspecified("Popup.borderWidth").takeOrElse { 2.dp },
itemMetrics = MenuItemMetrics(
selectionCornerSize = CornerSize(JBUI.CurrentTheme.PopupMenu.Selection.ARC.dp),
outerPadding = PaddingValues(horizontal = 6.dp),
contentPadding = PaddingValues(horizontal = 10.dp, vertical = 4.dp),
selectionCornerSize = CornerSize(JBUI.CurrentTheme.PopupMenu.Selection.ARC.dp / 2),
outerPadding = PaddingValues(horizontal = 7.dp),
contentPadding = PaddingValues(horizontal = 14.dp, vertical = 4.dp),
keybindingsPadding = PaddingValues(start = 36.dp),
separatorPadding = PaddingValues(
horizontal = retrieveIntAsDpOrUnspecified("PopupMenuSeparator.withToEdge")
.takeOrElse { 0.dp },
.takeOrElse { 1.dp },
vertical = retrieveIntAsDpOrUnspecified("PopupMenuSeparator.stripeIndent")
.takeOrElse { 0.dp },
.takeOrElse { 1.dp },
),
separatorThickness = retrieveIntAsDpOrUnspecified("PopupMenuSeparator.stripeWidth")
.takeOrElse { 0.dp },
.takeOrElse { 1.dp },
separatorHeight = retrieveIntAsDpOrUnspecified("PopupMenuSeparator.height")
.takeOrElse { 3.dp },
iconSize = 16.dp,
minHeight = if (isNewUiTheme()) JBUI.CurrentTheme.List.rowHeight().dp else Dp.Unspecified,
),
submenuMetrics = SubmenuMetrics(offset = DpOffset(0.dp, (-8).dp)),
),
Expand Down
4 changes: 2 additions & 2 deletions int-ui/int-ui-standalone/api/int-ui-standalone.api
Original file line number Diff line number Diff line change
Expand Up @@ -207,12 +207,12 @@ public final class org/jetbrains/jewel/intui/standalone/styling/IntUiMenuStyling
public static final fun dark-a3tHFA8 (Lorg/jetbrains/jewel/ui/component/styling/MenuItemColors$Companion;JJJJJJJJJJJJJJJJJJJJJLandroidx/compose/runtime/Composer;IIII)Lorg/jetbrains/jewel/ui/component/styling/MenuItemColors;
public static final fun defaults (Lorg/jetbrains/jewel/ui/component/styling/MenuIcons$Companion;Lorg/jetbrains/jewel/ui/painter/PainterProvider;)Lorg/jetbrains/jewel/ui/component/styling/MenuIcons;
public static synthetic fun defaults$default (Lorg/jetbrains/jewel/ui/component/styling/MenuIcons$Companion;Lorg/jetbrains/jewel/ui/painter/PainterProvider;ILjava/lang/Object;)Lorg/jetbrains/jewel/ui/component/styling/MenuIcons;
public static final fun defaults-AWlRVLg (Lorg/jetbrains/jewel/ui/component/styling/MenuItemMetrics$Companion;Landroidx/compose/foundation/shape/CornerSize;Landroidx/compose/foundation/layout/PaddingValues;Landroidx/compose/foundation/layout/PaddingValues;Landroidx/compose/foundation/layout/PaddingValues;Landroidx/compose/foundation/layout/PaddingValues;FF)Lorg/jetbrains/jewel/ui/component/styling/MenuItemMetrics;
public static synthetic fun defaults-AWlRVLg$default (Lorg/jetbrains/jewel/ui/component/styling/MenuItemMetrics$Companion;Landroidx/compose/foundation/shape/CornerSize;Landroidx/compose/foundation/layout/PaddingValues;Landroidx/compose/foundation/layout/PaddingValues;Landroidx/compose/foundation/layout/PaddingValues;Landroidx/compose/foundation/layout/PaddingValues;FFILjava/lang/Object;)Lorg/jetbrains/jewel/ui/component/styling/MenuItemMetrics;
public static final fun defaults-BkVx2pU (Lorg/jetbrains/jewel/ui/component/styling/SubmenuMetrics$Companion;J)Lorg/jetbrains/jewel/ui/component/styling/SubmenuMetrics;
public static synthetic fun defaults-BkVx2pU$default (Lorg/jetbrains/jewel/ui/component/styling/SubmenuMetrics$Companion;JILjava/lang/Object;)Lorg/jetbrains/jewel/ui/component/styling/SubmenuMetrics;
public static final fun defaults-ORMxH6s (Lorg/jetbrains/jewel/ui/component/styling/MenuMetrics$Companion;Landroidx/compose/foundation/shape/CornerSize;Landroidx/compose/foundation/layout/PaddingValues;Landroidx/compose/foundation/layout/PaddingValues;JFFLorg/jetbrains/jewel/ui/component/styling/MenuItemMetrics;Lorg/jetbrains/jewel/ui/component/styling/SubmenuMetrics;)Lorg/jetbrains/jewel/ui/component/styling/MenuMetrics;
public static synthetic fun defaults-ORMxH6s$default (Lorg/jetbrains/jewel/ui/component/styling/MenuMetrics$Companion;Landroidx/compose/foundation/shape/CornerSize;Landroidx/compose/foundation/layout/PaddingValues;Landroidx/compose/foundation/layout/PaddingValues;JFFLorg/jetbrains/jewel/ui/component/styling/MenuItemMetrics;Lorg/jetbrains/jewel/ui/component/styling/SubmenuMetrics;ILjava/lang/Object;)Lorg/jetbrains/jewel/ui/component/styling/MenuMetrics;
public static final fun defaults-r9IS1ZE (Lorg/jetbrains/jewel/ui/component/styling/MenuItemMetrics$Companion;Landroidx/compose/foundation/shape/CornerSize;Landroidx/compose/foundation/layout/PaddingValues;Landroidx/compose/foundation/layout/PaddingValues;Landroidx/compose/foundation/layout/PaddingValues;Landroidx/compose/foundation/layout/PaddingValues;FFFF)Lorg/jetbrains/jewel/ui/component/styling/MenuItemMetrics;
public static synthetic fun defaults-r9IS1ZE$default (Lorg/jetbrains/jewel/ui/component/styling/MenuItemMetrics$Companion;Landroidx/compose/foundation/shape/CornerSize;Landroidx/compose/foundation/layout/PaddingValues;Landroidx/compose/foundation/layout/PaddingValues;Landroidx/compose/foundation/layout/PaddingValues;Landroidx/compose/foundation/layout/PaddingValues;FFFFILjava/lang/Object;)Lorg/jetbrains/jewel/ui/component/styling/MenuItemMetrics;
public static final fun light (Lorg/jetbrains/jewel/ui/component/styling/MenuStyle$Companion;Lorg/jetbrains/jewel/ui/component/styling/MenuColors;Lorg/jetbrains/jewel/ui/component/styling/MenuMetrics;Lorg/jetbrains/jewel/ui/component/styling/MenuIcons;Landroidx/compose/runtime/Composer;II)Lorg/jetbrains/jewel/ui/component/styling/MenuStyle;
public static final fun light-Jy8F4Js (Lorg/jetbrains/jewel/ui/component/styling/MenuColors$Companion;JJJLorg/jetbrains/jewel/ui/component/styling/MenuItemColors;Landroidx/compose/runtime/Composer;II)Lorg/jetbrains/jewel/ui/component/styling/MenuColors;
public static final fun light-a3tHFA8 (Lorg/jetbrains/jewel/ui/component/styling/MenuItemColors$Companion;JJJJJJJJJJJJJJJJJJJJJLandroidx/compose/runtime/Composer;IIII)Lorg/jetbrains/jewel/ui/component/styling/MenuItemColors;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,9 @@ public fun MenuItemMetrics.Companion.defaults(
separatorPadding: PaddingValues = PaddingValues(horizontal = 12.dp, vertical = 4.dp),
keybindingsPadding: PaddingValues = PaddingValues(start = 36.dp),
separatorThickness: Dp = 1.dp,
separatorHeight: Dp = 9.dp,
iconSize: Dp = 16.dp,
minHeight: Dp = 20.dp,
): MenuItemMetrics =
MenuItemMetrics(
selectionCornerSize,
Expand All @@ -196,7 +198,9 @@ public fun MenuItemMetrics.Companion.defaults(
separatorPadding,
keybindingsPadding,
separatorThickness,
separatorHeight,
iconSize,
minHeight,
)

public fun SubmenuMetrics.Companion.defaults(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import androidx.compose.foundation.shape.RoundedCornerShape
import androidx.compose.foundation.verticalScroll
import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableIntStateOf
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.setValue
Expand All @@ -42,6 +43,7 @@ import org.jetbrains.jewel.ui.component.CircularProgressIndicator
import org.jetbrains.jewel.ui.component.CircularProgressIndicatorBig
import org.jetbrains.jewel.ui.component.DefaultButton
import org.jetbrains.jewel.ui.component.Divider
import org.jetbrains.jewel.ui.component.Dropdown
import org.jetbrains.jewel.ui.component.Icon
import org.jetbrains.jewel.ui.component.IconButton
import org.jetbrains.jewel.ui.component.LazyTree
Expand All @@ -52,6 +54,7 @@ import org.jetbrains.jewel.ui.component.Text
import org.jetbrains.jewel.ui.component.TextField
import org.jetbrains.jewel.ui.component.Tooltip
import org.jetbrains.jewel.ui.component.Typography
import org.jetbrains.jewel.ui.component.separator

@Composable
internal fun ComponentShowcaseTab() {
Expand Down Expand Up @@ -86,6 +89,23 @@ private fun RowScope.ColumnOne() {
style = Typography.h3TextStyle(),
)

var selectedItem by remember { mutableIntStateOf(-1) }
Dropdown(
menuContent = {
selectableItem(selectedItem == 0, onClick = { selectedItem = 0 }) {
Text("Hello")
}

separator()

selectableItem(selectedItem == 1, onClick = { selectedItem = 1 }) {
Text("World")
}
},
) {
Text("Selected item $selectedItem")
}

Row(
horizontalArrangement = Arrangement.spacedBy(16.dp),
verticalAlignment = Alignment.CenterVertically,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ fun DecoratedWindowScope.TitleBarView() {
val painterProvider =
rememberResourcePainterProvider(it.icon, StandaloneSampleIcons::class.java)
val painter by painterProvider.getPainter(Size(20))
Icon(painter, "icon")
Icon(painter, "icon", modifier = Modifier.size(20.dp))
Text(it.title)
}
}
Expand Down
4 changes: 3 additions & 1 deletion ui/api/ui.api
Original file line number Diff line number Diff line change
Expand Up @@ -1546,13 +1546,15 @@ public final class org/jetbrains/jewel/ui/component/styling/MenuItemColors$Compa
public final class org/jetbrains/jewel/ui/component/styling/MenuItemMetrics {
public static final field $stable I
public static final field Companion Lorg/jetbrains/jewel/ui/component/styling/MenuItemMetrics$Companion;
public synthetic fun <init> (Landroidx/compose/foundation/shape/CornerSize;Landroidx/compose/foundation/layout/PaddingValues;Landroidx/compose/foundation/layout/PaddingValues;Landroidx/compose/foundation/layout/PaddingValues;Landroidx/compose/foundation/layout/PaddingValues;FFLkotlin/jvm/internal/DefaultConstructorMarker;)V
public synthetic fun <init> (Landroidx/compose/foundation/shape/CornerSize;Landroidx/compose/foundation/layout/PaddingValues;Landroidx/compose/foundation/layout/PaddingValues;Landroidx/compose/foundation/layout/PaddingValues;Landroidx/compose/foundation/layout/PaddingValues;FFFFLkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun equals (Ljava/lang/Object;)Z
public final fun getContentPadding ()Landroidx/compose/foundation/layout/PaddingValues;
public final fun getIconSize-D9Ej5fM ()F
public final fun getKeybindingsPadding ()Landroidx/compose/foundation/layout/PaddingValues;
public final fun getMinHeight-D9Ej5fM ()F
public final fun getOuterPadding ()Landroidx/compose/foundation/layout/PaddingValues;
public final fun getSelectionCornerSize ()Landroidx/compose/foundation/shape/CornerSize;
public final fun getSeparatorHeight-D9Ej5fM ()F
public final fun getSeparatorPadding ()Landroidx/compose/foundation/layout/PaddingValues;
public final fun getSeparatorThickness-D9Ej5fM ()F
public fun hashCode ()I
Expand Down
63 changes: 8 additions & 55 deletions ui/src/main/kotlin/org/jetbrains/jewel/ui/component/Dropdown.kt
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,17 @@ import androidx.compose.runtime.CompositionLocalProvider
import androidx.compose.runtime.Immutable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableIntStateOf
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.setValue
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.focus.FocusManager
import androidx.compose.ui.focus.focusProperties
import androidx.compose.ui.input.InputMode
import androidx.compose.ui.input.InputModeManager
import androidx.compose.ui.layout.onSizeChanged
import androidx.compose.ui.platform.LocalDensity
import androidx.compose.ui.platform.LocalFocusManager
import androidx.compose.ui.platform.LocalInputModeManager
import androidx.compose.ui.semantics.Role
import androidx.compose.ui.window.Popup
import androidx.compose.ui.window.PopupProperties
import org.jetbrains.jewel.foundation.Stroke
import org.jetbrains.jewel.foundation.modifier.border
import org.jetbrains.jewel.foundation.state.CommonStateBitMask.Active
Expand All @@ -47,8 +43,6 @@ import org.jetbrains.jewel.foundation.theme.JewelTheme
import org.jetbrains.jewel.foundation.theme.LocalContentColor
import org.jetbrains.jewel.ui.Outline
import org.jetbrains.jewel.ui.component.styling.DropdownStyle
import org.jetbrains.jewel.ui.component.styling.LocalMenuStyle
import org.jetbrains.jewel.ui.component.styling.MenuStyle
import org.jetbrains.jewel.ui.focusOutline
import org.jetbrains.jewel.ui.outline
import org.jetbrains.jewel.ui.painter.hints.Stateful
Expand Down Expand Up @@ -98,6 +92,7 @@ public fun Dropdown(
val borderColor by colors.borderFor(dropdownState)
val hasNoOutline = outline == Outline.None

var componentWidth by remember { mutableIntStateOf(-1) }
Box(
modifier = modifier
.clickable(
Expand All @@ -120,7 +115,8 @@ public fun Dropdown(
.thenIf(outline == Outline.None) { focusOutline(dropdownState, shape) }
.outline(dropdownState, outline, shape)
.width(IntrinsicSize.Max)
.defaultMinSize(minSize.width, minSize.height.coerceAtLeast(arrowMinSize.height)),
.defaultMinSize(minSize.width, minSize.height.coerceAtLeast(arrowMinSize.height))
.onSizeChanged { componentWidth = it.width },
contentAlignment = Alignment.CenterStart,
) {
CompositionLocalProvider(LocalContentColor provides colors.contentFor(dropdownState).value) {
Expand All @@ -146,6 +142,7 @@ public fun Dropdown(
}

if (expanded) {
val density = LocalDensity.current
PopupMenu(
onDismissRequest = {
expanded = false
Expand All @@ -154,7 +151,8 @@ public fun Dropdown(
}
true
},
modifier = menuModifier.focusProperties { canFocus = true },
modifier = menuModifier.focusProperties { canFocus = true }
.defaultMinSize(minWidth = with(density) { componentWidth.toDp() }),
style = style.menuStyle,
horizontalAlignment = Alignment.Start,
content = menuContent,
Expand All @@ -163,51 +161,6 @@ public fun Dropdown(
}
}

@Composable
internal fun DropdownMenu(
onDismissRequest: (InputMode) -> Boolean,
horizontalAlignment: Alignment.Horizontal,
modifier: Modifier = Modifier,
style: MenuStyle,
content: MenuScope.() -> Unit,
) {
val density = LocalDensity.current

val popupPositionProvider = AnchorVerticalMenuPositionProvider(
contentOffset = style.metrics.offset,
contentMargin = style.metrics.menuMargin,
alignment = horizontalAlignment,
density = density,
)

var focusManager: FocusManager? by remember { mutableStateOf(null) }
var inputModeManager: InputModeManager? by remember { mutableStateOf(null) }
val menuManager = remember(onDismissRequest) { MenuManager(onDismissRequest = onDismissRequest) }

Popup(
popupPositionProvider = popupPositionProvider,
onDismissRequest = { onDismissRequest(InputMode.Touch) },
properties = PopupProperties(focusable = true),
onPreviewKeyEvent = { false },
onKeyEvent = {
val currentFocusManager = checkNotNull(focusManager) { "FocusManager must not be null" }
val currentInputModeManager =
checkNotNull(inputModeManager) { "InputModeManager must not be null" }
handlePopupMenuOnKeyEvent(it, currentFocusManager, currentInputModeManager, menuManager)
},
) {
focusManager = LocalFocusManager.current
inputModeManager = LocalInputModeManager.current

CompositionLocalProvider(
LocalMenuManager provides menuManager,
LocalMenuStyle provides style,
) {
MenuContent(modifier, content = content)
}
}
}

@Immutable
@JvmInline
public value class DropdownState(public val state: ULong) : FocusableComponentState {
Expand Down
20 changes: 13 additions & 7 deletions ui/src/main/kotlin/org/jetbrains/jewel/ui/component/Menu.kt
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@ import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.IntrinsicSize
import androidx.compose.foundation.layout.PaddingValues
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.defaultMinSize
import androidx.compose.foundation.layout.fillMaxHeight
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.layout.size
import androidx.compose.foundation.layout.width
Expand Down Expand Up @@ -158,7 +160,7 @@ internal fun MenuContent(
ambientColor = colors.shadow,
spotColor = colors.shadow,
)
.border(Stroke.Alignment.Center, style.metrics.borderWidth, colors.border, menuShape)
.border(Stroke.Alignment.Inside, style.metrics.borderWidth, colors.border, menuShape)
.background(colors.background, menuShape)
.width(IntrinsicSize.Max)
.onHover { localMenuManager.onHoveredChange(it) },
Expand Down Expand Up @@ -336,12 +338,14 @@ public fun MenuSeparator(
metrics: MenuItemMetrics = JewelTheme.menuStyle.metrics.itemMetrics,
colors: MenuItemColors = JewelTheme.menuStyle.colors.itemColors,
) {
Divider(
orientation = Orientation.Horizontal,
modifier = modifier.padding(metrics.separatorPadding),
color = colors.separator,
thickness = metrics.separatorThickness,
)
Box(modifier.height(metrics.separatorHeight)) {
Divider(
orientation = Orientation.Horizontal,
modifier = Modifier.fillMaxWidth().padding(metrics.separatorPadding),
color = colors.separator,
thickness = metrics.separatorThickness,
)
}
}

@Composable
Expand Down Expand Up @@ -422,9 +426,11 @@ internal fun MenuItem(

Row(
modifier = Modifier.fillMaxWidth()
.defaultMinSize(minHeight = itemMetrics.minHeight)
.drawItemBackground(itemMetrics, backgroundColor)
.padding(itemMetrics.contentPadding),
horizontalArrangement = Arrangement.spacedBy(4.dp),
verticalAlignment = Alignment.CenterVertically,
) {
if (canShowIcon) {
val iconModifier = Modifier.size(style.metrics.itemMetrics.iconSize)
Expand Down
Loading

0 comments on commit d797601

Please sign in to comment.