Fix various components (#123)

* Use focusOutline modifier in Links * Makes disabled icons look more like they do in Swing * Remove indication from links * Fix outlines for checkboxes * Fix outlines for radio buttons * Add alignment to focusOutline modifier * Clean up InputField code * Fix several issues in dropdown: 1. State mutation was broken (no focus, etc.) 2. Min size and padding are closer to Swing now 3. Focus ring was missing 4. Now we pretend it's focused when the menu is open (like Swing does) * Fix bridge, tweak standalone checkbox metrics * Only show focus on InputField when it's decorated * Fixed bug on Link Interaction --------- Co-authored-by: fscarponi <[email protected]>
JetBrains · Sep 21, 2023 · b70eac8 · b70eac8
1 parent 9f755f5
commit b70eac8
Show file tree

Hide file tree

Showing 11 changed files with 83 additions and 83 deletions.
diff --git a/core/src/main/kotlin/org/jetbrains/jewel/Checkbox.kt b/core/src/main/kotlin/org/jetbrains/jewel/Checkbox.kt
@@ -9,6 +9,7 @@ import androidx.compose.foundation.layout.Arrangement
 import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.Row
 import androidx.compose.foundation.layout.RowScope
+import androidx.compose.foundation.layout.offset
 import androidx.compose.foundation.layout.size
 import androidx.compose.foundation.selection.triStateToggleable
 import androidx.compose.foundation.shape.RoundedCornerShape
@@ -284,25 +285,32 @@ private fun CheckboxImpl(
     )
 
     val checkBoxImageModifier = Modifier.size(metrics.checkboxSize)
+    val outlineModifier = Modifier.size(metrics.outlineSize)
+        .offset(metrics.outlineOffset.x, metrics.outlineOffset.y)
         .outline(
             state = checkboxState,
             outline = outline,
             outlineShape = RoundedCornerShape(metrics.checkboxCornerSize),
             alignment = Stroke.Alignment.Center,
-            outlineWidth = metrics.outlineWidth,
         )
 
     val checkboxPainter by icons.checkbox.getPainter(resourceLoader, checkboxState)
 
     if (content == null) {
-        CheckBoxImage(wrapperModifier, checkboxPainter, checkBoxImageModifier)
+        Box(contentAlignment = Alignment.TopStart) {
+            CheckBoxImage(wrapperModifier, checkboxPainter, checkBoxImageModifier)
+            Box(outlineModifier)
+        }
     } else {
         Row(
             wrapperModifier,
             horizontalArrangement = Arrangement.spacedBy(metrics.iconContentGap),
             verticalAlignment = Alignment.CenterVertically,
         ) {
-            CheckBoxImage(Modifier, checkboxPainter, checkBoxImageModifier)
+            Box(contentAlignment = Alignment.TopStart) {
+                CheckBoxImage(Modifier, checkboxPainter, checkBoxImageModifier)
+                Box(outlineModifier)
+            }
 
             val contentColor by colors.contentFor(checkboxState)
             CompositionLocalProvider(

diff --git a/core/src/main/kotlin/org/jetbrains/jewel/DisabledColorFilter.kt b/core/src/main/kotlin/org/jetbrains/jewel/DisabledColorFilter.kt
@@ -8,13 +8,12 @@ import androidx.compose.ui.graphics.ColorMatrix
 private val disabledColorMatrixGammaEncoded = ColorMatrix().apply {
     val saturation = .5f
 
-    // We use NTSC luminance weights like Swing does as it's gamma-encoded RGB
-    val redFactor = .299f * saturation
-    val greenFactor = .587f * saturation
-    val blueFactor = .114f * saturation
-
-    // TODO we should also be scaling the brightness but it's not possible
-    //  with a matrix transformation as far as I can tell
+    // We use NTSC luminance weights like Swing does as it's gamma-encoded RGB,
+    // and add some brightness to emulate Swing's "brighter" approach, which is
+    // not representable with a ColorMatrix alone as it's a non-linear op.
+    val redFactor = .299f * saturation + .25f
+    val greenFactor = .587f * saturation + .25f
+    val blueFactor = .114f * saturation + .25f
     this[0, 0] = redFactor
     this[0, 1] = greenFactor
     this[0, 2] = blueFactor

diff --git a/core/src/main/kotlin/org/jetbrains/jewel/Dropdown.kt b/core/src/main/kotlin/org/jetbrains/jewel/Dropdown.kt
@@ -45,6 +45,7 @@ import org.jetbrains.jewel.foundation.border
 import org.jetbrains.jewel.styling.DropdownStyle
 import org.jetbrains.jewel.styling.LocalMenuStyle
 import org.jetbrains.jewel.styling.MenuStyle
+import org.jetbrains.jewel.util.appendIf
 
 @Composable
 fun Dropdown(
@@ -93,6 +94,10 @@ fun Dropdown(
         val arrowMinSize = style.metrics.arrowMinSize
         val borderColor by colors.borderFor(dropdownState)
 
+        val outlineState = remember(dropdownState, expanded) {
+            dropdownState.copy(focused = dropdownState.isFocused || expanded)
+        }
+
         Box(
             modifier.clickable(
                 onClick = {
@@ -109,7 +114,8 @@ fun Dropdown(
             )
                 .background(colors.backgroundFor(dropdownState).value, shape)
                 .border(Stroke.Alignment.Center, style.metrics.borderWidth, borderColor, shape)
-                .outline(dropdownState, outline, shape)
+                .appendIf(outline == Outline.None) { focusOutline(outlineState, shape) }
+                .outline(outlineState, outline, shape)
                 .defaultMinSize(minSize.width, minSize.height.coerceAtLeast(arrowMinSize.height)),
             contentAlignment = Alignment.CenterStart,
         ) {
@@ -262,15 +268,11 @@ value class DropdownState(val state: ULong) : FocusableComponentState {
             hovered: Boolean = false,
             active: Boolean = false,
         ) = DropdownState(
-            if (enabled) {
-                Enabled
-            } else {
-                0UL or
-                    (if (focused) Focused else 0UL) or
-                    (if (pressed) Pressed else 0UL) or
-                    (if (hovered) Hovered else 0UL) or
-                    (if (active) Active else 0UL)
-            },
+            (if (enabled) Enabled else 0UL) or
+                (if (focused) Focused else 0UL) or
+                (if (hovered) Hovered else 0UL) or
+                (if (pressed) Pressed else 0UL) or
+                (if (active) Active else 0UL),
         )
     }
 }
diff --git a/core/src/main/kotlin/org/jetbrains/jewel/InputField.kt b/core/src/main/kotlin/org/jetbrains/jewel/InputField.kt
@@ -22,6 +22,11 @@ import androidx.compose.ui.text.TextLayoutResult
 import androidx.compose.ui.text.TextStyle
 import androidx.compose.ui.text.input.TextFieldValue
 import androidx.compose.ui.text.input.VisualTransformation
+import org.jetbrains.jewel.CommonStateBitMask.Active
+import org.jetbrains.jewel.CommonStateBitMask.Enabled
+import org.jetbrains.jewel.CommonStateBitMask.Focused
+import org.jetbrains.jewel.CommonStateBitMask.Hovered
+import org.jetbrains.jewel.CommonStateBitMask.Pressed
 import org.jetbrains.jewel.foundation.Stroke
 import org.jetbrains.jewel.foundation.border
 import org.jetbrains.jewel.styling.InputFieldStyle
@@ -88,7 +93,7 @@ internal fun InputField(
         value = value,
         modifier = modifier.then(backgroundModifier)
             .then(borderModifier)
-            .focusOutline(inputState, shape)
+            .appendIf(!undecorated) { focusOutline(inputState, shape) }
             .outline(inputState, outline, shape),
         onValueChange = onValueChange,
         enabled = enabled,
@@ -114,23 +119,23 @@ value class InputFieldState(val state: ULong) : FocusableComponentState {
 
     @Stable
     override val isActive: Boolean
-        get() = state and CommonStateBitMask.Active != 0UL
+        get() = state and Active != 0UL
 
     @Stable
     override val isEnabled: Boolean
-        get() = state and CommonStateBitMask.Enabled != 0UL
+        get() = state and Enabled != 0UL
 
     @Stable
     override val isFocused: Boolean
-        get() = state and CommonStateBitMask.Focused != 0UL
+        get() = state and Focused != 0UL
 
     @Stable
     override val isHovered: Boolean
-        get() = state and CommonStateBitMask.Hovered != 0UL
+        get() = state and Hovered != 0UL
 
     @Stable
     override val isPressed: Boolean
-        get() = state and CommonStateBitMask.Pressed != 0UL
+        get() = state and Pressed != 0UL
 
     fun copy(
         enabled: Boolean = isEnabled,
@@ -159,11 +164,11 @@ value class InputFieldState(val state: ULong) : FocusableComponentState {
             hovered: Boolean = false,
             active: Boolean = false,
         ) = InputFieldState(
-            state = (if (enabled) CommonStateBitMask.Enabled else 0UL) or
-                (if (focused) CommonStateBitMask.Focused else 0UL) or
-                (if (hovered) CommonStateBitMask.Hovered else 0UL) or
-                (if (pressed) CommonStateBitMask.Pressed else 0UL) or
-                (if (active) CommonStateBitMask.Active else 0UL),
+            state = (if (enabled) Enabled else 0UL) or
+                (if (focused) Focused else 0UL) or
+                (if (hovered) Hovered else 0UL) or
+                (if (pressed) Pressed else 0UL) or
+                (if (active) Active else 0UL),
         )
     }
 }
diff --git a/core/src/main/kotlin/org/jetbrains/jewel/Link.kt b/core/src/main/kotlin/org/jetbrains/jewel/Link.kt
@@ -1,6 +1,5 @@
 package org.jetbrains.jewel
 
-import androidx.compose.foundation.Indication
 import androidx.compose.foundation.clickable
 import androidx.compose.foundation.interaction.FocusInteraction
 import androidx.compose.foundation.interaction.HoverInteraction
@@ -42,8 +41,6 @@ import org.jetbrains.jewel.CommonStateBitMask.Focused
 import org.jetbrains.jewel.CommonStateBitMask.Hovered
 import org.jetbrains.jewel.CommonStateBitMask.Pressed
 import org.jetbrains.jewel.IntelliJTheme.Companion.isSwingCompatMode
-import org.jetbrains.jewel.foundation.Stroke
-import org.jetbrains.jewel.foundation.border
 import org.jetbrains.jewel.foundation.onHover
 import org.jetbrains.jewel.styling.LinkStyle
 import org.jetbrains.jewel.styling.LocalLinkStyle
@@ -69,7 +66,6 @@ fun Link(
     overflow: TextOverflow = TextOverflow.Clip,
     lineHeight: TextUnit = TextUnit.Unspecified,
     interactionSource: MutableInteractionSource = remember { MutableInteractionSource() },
-    indication: Indication? = null,
     style: LinkStyle = LocalLinkStyle.current,
 ) {
     LinkImpl(
@@ -86,7 +82,6 @@ fun Link(
         overflow = overflow,
         lineHeight = lineHeight,
         interactionSource = interactionSource,
-        indication = indication,
         style = style,
         resourceLoader = resourceLoader,
         icon = null,
@@ -109,7 +104,6 @@ fun ExternalLink(
     overflow: TextOverflow = TextOverflow.Clip,
     lineHeight: TextUnit = TextUnit.Unspecified,
     interactionSource: MutableInteractionSource = remember { MutableInteractionSource() },
-    indication: Indication? = null,
     style: LinkStyle = LocalLinkStyle.current,
 ) {
     LinkImpl(
@@ -126,7 +120,6 @@ fun ExternalLink(
         overflow = overflow,
         lineHeight = lineHeight,
         interactionSource = interactionSource,
-        indication = indication,
         style = style,
         resourceLoader = resourceLoader,
         icon = style.icons.externalLink,
@@ -148,7 +141,6 @@ fun DropdownLink(
     overflow: TextOverflow = TextOverflow.Clip,
     lineHeight: TextUnit = TextUnit.Unspecified,
     interactionSource: MutableInteractionSource = remember { MutableInteractionSource() },
-    indication: Indication? = null,
     style: LinkStyle = LocalLinkStyle.current,
     menuModifier: Modifier = Modifier,
     menuStyle: MenuStyle = LocalMenuStyle.current,
@@ -157,11 +149,8 @@ fun DropdownLink(
     var expanded by remember { mutableStateOf(false) }
     var hovered by remember { mutableStateOf(false) }
     var skipNextClick by remember { mutableStateOf(false) }
-    Box(
-        Modifier.onHover {
-            hovered = it
-        },
-    ) {
+
+    Box(Modifier.onHover { hovered = it }) {
         LinkImpl(
             text = text,
             onClick = {
@@ -181,7 +170,6 @@ fun DropdownLink(
             overflow = overflow,
             lineHeight = lineHeight,
             interactionSource = interactionSource,
-            indication = indication,
             style = style,
             icon = style.icons.dropdownChevron,
             resourceLoader = resourceLoader,
@@ -198,7 +186,7 @@ fun DropdownLink(
                 },
                 modifier = menuModifier,
                 style = menuStyle,
-                horizontalAlignment = Alignment.Start, // TODO no idea what goes here
+                horizontalAlignment = Alignment.Start,
                 content = menuContent,
                 resourceLoader = resourceLoader,
             )
@@ -223,10 +211,9 @@ private fun LinkImpl(
     overflow: TextOverflow,
     lineHeight: TextUnit,
     interactionSource: MutableInteractionSource,
-    indication: Indication?,
     icon: PainterProvider<LinkState>?,
 ) {
-    var linkState by remember(interactionSource) {
+    var linkState by remember(interactionSource, enabled) {
         mutableStateOf(LinkState.of(enabled = enabled))
     }
     remember(enabled) {
@@ -248,9 +235,7 @@ private fun LinkImpl(
                     }
                 }
 
-                is FocusInteraction.Unfocus -> {
-                    linkState = linkState.copy(focused = false, pressed = false)
-                }
+                is FocusInteraction.Unfocus -> linkState = linkState.copy(focused = false, pressed = false)
             }
         }
     }
@@ -271,30 +256,23 @@ private fun LinkImpl(
             ),
         )
 
-    val clickable = Modifier.clickable(
-        onClick = {
-            linkState = linkState.copy(visited = true)
-            onClick()
-        },
-        enabled = enabled,
-        role = Role.Button,
-        interactionSource = interactionSource,
-        indication = indication,
-    )
-
-    val focusHaloModifier = Modifier.border(
-        alignment = Stroke.Alignment.Outside,
-        width = LocalGlobalMetrics.current.outlineWidth,
-        color = LocalGlobalColors.current.outlines.focused,
-        shape = RoundedCornerShape(style.metrics.focusHaloCornerSize),
-    )
-
     val pointerChangeModifier = Modifier.pointerHoverIcon(PointerIcon(Cursor(Cursor.HAND_CURSOR)))
 
     Row(
-        modifier = modifier.then(clickable)
-            .appendIf(linkState.isFocused) { focusHaloModifier }
-            .appendIf(linkState.isEnabled) { pointerChangeModifier },
+        modifier = modifier
+            .appendIf(linkState.isEnabled) { pointerChangeModifier }
+            .clickable(
+                onClick = {
+                    linkState = linkState.copy(visited = true)
+                    println("clicked Link")
+                    onClick()
+                },
+                enabled = enabled,
+                role = Role.Button,
+                interactionSource = interactionSource,
+                indication = null,
+            )
+            .focusOutline(linkState, RoundedCornerShape(style.metrics.focusHaloCornerSize)),
         horizontalArrangement = Arrangement.spacedBy(style.metrics.textIconGap),
         verticalAlignment = Alignment.CenterVertically,
     ) {

diff --git a/core/src/main/kotlin/org/jetbrains/jewel/Outline.kt b/core/src/main/kotlin/org/jetbrains/jewel/Outline.kt
@@ -29,13 +29,14 @@ enum class Outline {
 fun Modifier.focusOutline(
     state: FocusableComponentState,
     outlineShape: Shape,
+    alignment: Stroke.Alignment = Stroke.Alignment.Outside,
     outlineWidth: Dp = IntelliJTheme.globalMetrics.outlineWidth,
 ): Modifier {
     val outlineColors = IntelliJTheme.globalColors.outlines
 
     return thenIf(state.isFocused) {
         val outlineColor = outlineColors.focused
-        border(Stroke.Alignment.Outside, outlineWidth, outlineColor, outlineShape)
+        border(alignment, outlineWidth, outlineColor, outlineShape)
     }
 }
 

diff --git a/core/src/main/kotlin/org/jetbrains/jewel/RadioButton.kt b/core/src/main/kotlin/org/jetbrains/jewel/RadioButton.kt
@@ -34,6 +34,7 @@ import org.jetbrains.jewel.CommonStateBitMask.Enabled
 import org.jetbrains.jewel.CommonStateBitMask.Focused
 import org.jetbrains.jewel.CommonStateBitMask.Hovered
 import org.jetbrains.jewel.CommonStateBitMask.Pressed
+import org.jetbrains.jewel.foundation.Stroke
 import org.jetbrains.jewel.styling.RadioButtonStyle
 
 @Composable
@@ -164,8 +165,9 @@ private fun RadioButtonImpl(
 
     val colors = style.colors
     val metrics = style.metrics
-    val radioButtonModifier = Modifier.size(metrics.radioButtonSize)
-        .outline(radioButtonState, outline, outlineShape = CircleShape)
+    val radioButtonModifier = Modifier
+        .size(metrics.radioButtonSize)
+        .outline(radioButtonState, outline, outlineShape = CircleShape, alignment = Stroke.Alignment.Inside)
     val radioButtonPainter by style.icons.radioButton.getPainter(resourceLoader, radioButtonState)
 
     if (content == null) {

diff --git a/core/src/main/kotlin/org/jetbrains/jewel/styling/CheckboxStyling.kt b/core/src/main/kotlin/org/jetbrains/jewel/styling/CheckboxStyling.kt
@@ -8,6 +8,7 @@ import androidx.compose.runtime.staticCompositionLocalOf
 import androidx.compose.ui.graphics.Color
 import androidx.compose.ui.state.ToggleableState
 import androidx.compose.ui.unit.Dp
+import androidx.compose.ui.unit.DpOffset
 import androidx.compose.ui.unit.DpSize
 import org.jetbrains.jewel.CheckboxState
 
@@ -55,7 +56,8 @@ interface CheckboxMetrics {
 
     val checkboxSize: DpSize
     val checkboxCornerSize: CornerSize
-    val outlineWidth: Dp
+    val outlineSize: DpSize
+    val outlineOffset: DpOffset
     val iconContentGap: Dp
 }
 

diff --git a/ide-laf-bridge/src/main/kotlin/org/jetbrains/jewel/bridge/IntUiBridge.kt b/ide-laf-bridge/src/main/kotlin/org/jetbrains/jewel/bridge/IntUiBridge.kt
@@ -262,7 +262,8 @@ private fun readCheckboxStyle(iconData: IntelliJThemeIconData, svgLoader: SvgLoa
         metrics = IntUiCheckboxMetrics(
             checkboxSize = DarculaCheckBoxUI().defaultIcon.let { DpSize(it.iconWidth.dp, it.iconHeight.dp) },
             checkboxCornerSize = CornerSize(3.dp), // See DarculaCheckBoxUI#drawCheckIcon
-            outlineWidth = 3.dp, // See DarculaCheckBoxUI#drawCheckIcon
+            outlineSize = DpSize(15.dp, 15.dp), // Extrapolated from SVG
+            outlineOffset = DpOffset(2.5.dp, 1.5.dp), // Extrapolated from SVG
             iconContentGap = 5.dp, // See DarculaCheckBoxUI#textIconGap
         ),
         icons = IntUiCheckboxIcons(