Improve icon buttons by making them optionally focusable (#447)

* Improve icon buttons by making them optionally focusable While in the IJP ActionButtons aren't focusable by default when in toolbars, we do want them to be, because 1) it's the right thing to do to improve keyboard navigation, and b) Studio needs this. * add missing trailing comma in Buttons Signed-off-by: Ivan Morgillo <imorgillo@gmail.com> --------- Signed-off-by: Ivan Morgillo <imorgillo@gmail.com> Co-authored-by: Ivan Morgillo <imorgillo@gmail.com>
JetBrains · Jul 15, 2024 · 119c856 · 119c856
1 parent e3fa051
commit 119c856
Show file tree

Hide file tree

Showing 5 changed files with 89 additions and 31 deletions.
diff --git a/ide-laf-bridge/src/main/kotlin/org/jetbrains/jewel/bridge/theme/IntUiBridge.kt b/ide-laf-bridge/src/main/kotlin/org/jetbrains/jewel/bridge/theme/IntUiBridge.kt
@@ -1145,7 +1145,7 @@ private fun readIconButtonStyle(): IconButtonStyle =
                 cornerSize = CornerSize(DarculaUIUtil.BUTTON_ARC.dp / 2),
                 borderWidth = 1.dp,
                 padding = PaddingValues(0.dp),
-                minSize = DpSize(16.dp, 16.dp),
+                minSize = DpSize(24.dp, 24.dp),
             ),
         colors =
             IconButtonColors(
@@ -1154,9 +1154,9 @@ private fun readIconButtonStyle(): IconButtonStyle =
                 backgroundDisabled = Color.Unspecified,
                 backgroundSelected = retrieveColorOrUnspecified("ActionButton.pressedBackground"),
                 backgroundSelectedActivated = retrieveColorOrUnspecified("ToolWindow.Button.selectedBackground"),
-                backgroundFocused = Color.Unspecified,
                 backgroundPressed = retrieveColorOrUnspecified("ActionButton.pressedBackground"),
                 backgroundHovered = retrieveColorOrUnspecified("ActionButton.hoverBackground"),
+                backgroundFocused = retrieveColorOrUnspecified("ActionButton.hoverBackground"),
                 border = Color.Unspecified,
                 borderDisabled = Color.Unspecified,
                 borderSelected = retrieveColorOrUnspecified("ActionButton.pressedBackground"),

diff --git a/...ne/src/main/kotlin/org/jetbrains/jewel/intui/standalone/styling/IntUiIconButtonStyling.kt b/...ne/src/main/kotlin/org/jetbrains/jewel/intui/standalone/styling/IntUiIconButtonStyling.kt
@@ -32,9 +32,9 @@ public fun IconButtonColors.Companion.light(
     backgroundDisabled: Color = background,
     backgroundSelected: Color = IntUiLightTheme.colors.grey(11),
     backgroundSelectedActivated: Color = IntUiLightTheme.colors.blue(4),
-    backgroundFocused: Color = background,
     backgroundPressed: Color = IntUiLightTheme.colors.grey(11),
     backgroundHovered: Color = IntUiLightTheme.colors.grey(12),
+    backgroundFocused: Color = backgroundHovered,
     border: Color = background,
     borderDisabled: Color = backgroundDisabled,
     borderSelected: Color = backgroundSelected,
@@ -68,9 +68,9 @@ public fun IconButtonColors.Companion.dark(
     backgroundDisabled: Color = background,
     backgroundSelected: Color = IntUiDarkTheme.colors.grey(5),
     backgroundSelectedActivated: Color = IntUiDarkTheme.colors.blue(6),
-    backgroundFocused: Color = background,
     backgroundPressed: Color = IntUiDarkTheme.colors.grey(5),
     backgroundHovered: Color = IntUiDarkTheme.colors.grey(3),
+    backgroundFocused: Color = backgroundHovered,
     border: Color = background,
     borderDisabled: Color = backgroundDisabled,
     borderSelected: Color = backgroundSelected,
@@ -101,5 +101,5 @@ public fun IconButtonMetrics.Companion.defaults(
     cornerSize: CornerSize = CornerSize(4.dp),
     borderWidth: Dp = 1.dp,
     padding: PaddingValues = PaddingValues(0.dp),
-    minSize: DpSize = DpSize(16.dp, 16.dp),
+    minSize: DpSize = DpSize(24.dp, 24.dp),
 ): IconButtonMetrics = IconButtonMetrics(cornerSize, borderWidth, padding, minSize)
diff --git a/...andalone/src/main/kotlin/org/jetbrains/jewel/samples/standalone/view/component/Buttons.kt b/...andalone/src/main/kotlin/org/jetbrains/jewel/samples/standalone/view/component/Buttons.kt
@@ -1,9 +1,14 @@
 package org.jetbrains.jewel.samples.standalone.view.component
 
 import androidx.compose.foundation.layout.Arrangement
+import androidx.compose.foundation.layout.Column
 import androidx.compose.foundation.layout.Row
 import androidx.compose.foundation.layout.fillMaxWidth
 import androidx.compose.runtime.Composable
+import androidx.compose.runtime.getValue
+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.unit.dp
@@ -12,12 +17,26 @@ import org.jetbrains.jewel.ui.component.DefaultButton
 import org.jetbrains.jewel.ui.component.IconButton
 import org.jetbrains.jewel.ui.component.OutlinedButton
 import org.jetbrains.jewel.ui.component.PlatformIcon
+import org.jetbrains.jewel.ui.component.SelectableIconButton
 import org.jetbrains.jewel.ui.component.Text
+import org.jetbrains.jewel.ui.component.styling.LocalIconButtonStyle
 import org.jetbrains.jewel.ui.icons.AllIconsKeys
+import org.jetbrains.jewel.ui.painter.hints.Selected
+import org.jetbrains.jewel.ui.painter.hints.Stroke
 
 @Composable
 @View(title = "Buttons", position = 0, icon = "icons/components/button.svg")
 fun Buttons() {
+    Column(
+        verticalArrangement = Arrangement.spacedBy(16.dp),
+    ) {
+        NormalButtons()
+        IconButtons()
+    }
+}
+
+@Composable
+private fun NormalButtons() {
     Row(
         modifier = Modifier.fillMaxWidth(),
         horizontalArrangement = Arrangement.spacedBy(16.dp),
@@ -38,9 +57,38 @@ fun Buttons() {
         DefaultButton(onClick = {}, enabled = false) {
             Text("Default disabled")
         }
+    }
+}
+
+@Composable
+private fun IconButtons() {
+    Row(
+        modifier = Modifier.fillMaxWidth(),
+        horizontalArrangement = Arrangement.spacedBy(16.dp),
+        verticalAlignment = Alignment.CenterVertically,
+    ) {
+        Text("Focusable:")
 
         IconButton(onClick = {}) {
             PlatformIcon(AllIconsKeys.Actions.Close, contentDescription = "IconButton")
         }
+
+        Text("Not focusable:")
+
+        IconButton(onClick = {}, focusable = false) {
+            PlatformIcon(AllIconsKeys.Actions.Close, contentDescription = "IconButton")
+        }
+
+        Text("Selectable:")
+
+        var selected by remember { mutableStateOf(false) }
+        SelectableIconButton(onClick = { selected = !selected }, selected = selected) { state ->
+            val tint by LocalIconButtonStyle.current.colors.foregroundFor(state)
+            PlatformIcon(
+                key = AllIconsKeys.Actions.MatchCase,
+                contentDescription = "IconButton",
+                hints = arrayOf(Selected(selected), Stroke(tint)),
+            )
+        }
     }
 }
diff --git a/ui/api/ui.api b/ui/api/ui.api
@@ -300,8 +300,8 @@ public final class org/jetbrains/jewel/ui/component/GroupHeaderKt {
 }
 
 public final class org/jetbrains/jewel/ui/component/IconButtonKt {
-	public static final fun IconButton (Lkotlin/jvm/functions/Function0;Landroidx/compose/ui/Modifier;ZLorg/jetbrains/jewel/ui/component/styling/IconButtonStyle;Landroidx/compose/foundation/interaction/MutableInteractionSource;Lkotlin/jvm/functions/Function4;Landroidx/compose/runtime/Composer;II)V
-	public static final fun SelectableIconButton (ZLkotlin/jvm/functions/Function0;Landroidx/compose/ui/Modifier;ZLorg/jetbrains/jewel/ui/component/styling/IconButtonStyle;Landroidx/compose/foundation/interaction/MutableInteractionSource;Lkotlin/jvm/functions/Function4;Landroidx/compose/runtime/Composer;II)V
+	public static final fun IconButton (Lkotlin/jvm/functions/Function0;Landroidx/compose/ui/Modifier;ZZLorg/jetbrains/jewel/ui/component/styling/IconButtonStyle;Landroidx/compose/foundation/interaction/MutableInteractionSource;Lkotlin/jvm/functions/Function4;Landroidx/compose/runtime/Composer;II)V
+	public static final fun SelectableIconButton (ZLkotlin/jvm/functions/Function0;Landroidx/compose/ui/Modifier;ZZLorg/jetbrains/jewel/ui/component/styling/IconButtonStyle;Landroidx/compose/foundation/interaction/MutableInteractionSource;Lkotlin/jvm/functions/Function4;Landroidx/compose/runtime/Composer;II)V
 }
 
 public final class org/jetbrains/jewel/ui/component/IconButtonState : org/jetbrains/jewel/foundation/state/FocusableComponentState, org/jetbrains/jewel/foundation/state/SelectableComponentState {

diff --git a/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/IconButton.kt b/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/IconButton.kt
@@ -23,6 +23,7 @@ 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.focusProperties
 import androidx.compose.ui.semantics.Role
 import org.jetbrains.jewel.foundation.modifier.onActivated
 import org.jetbrains.jewel.foundation.state.CommonStateBitMask
@@ -31,12 +32,14 @@ import org.jetbrains.jewel.foundation.state.SelectableComponentState
 import org.jetbrains.jewel.foundation.theme.JewelTheme
 import org.jetbrains.jewel.ui.component.styling.IconButtonStyle
 import org.jetbrains.jewel.ui.theme.iconButtonStyle
+import org.jetbrains.jewel.ui.util.thenIf
 
 @Composable
 public fun IconButton(
     onClick: () -> Unit,
     modifier: Modifier = Modifier,
     enabled: Boolean = true,
+    focusable: Boolean = true,
     style: IconButtonStyle = JewelTheme.iconButtonStyle,
     interactionSource: MutableInteractionSource = remember { MutableInteractionSource() },
     content: @Composable (BoxScope.(IconButtonState) -> Unit),
@@ -48,14 +51,15 @@ public fun IconButton(
     IconButtonImpl(
         state = buttonState,
         modifier =
-            modifier.clickable(
-                onClick = onClick,
-                enabled = enabled,
-                role = Role.Button,
-                interactionSource = interactionSource,
-                indication = null,
-            ),
+        modifier.clickable(
+            onClick = onClick,
+            enabled = enabled,
+            role = Role.Button,
+            interactionSource = interactionSource,
+            indication = null,
+        ),
         style = style,
+        focusable = focusable,
         interactionSource = interactionSource,
         content = content,
     )
@@ -67,6 +71,7 @@ public fun SelectableIconButton(
     onClick: () -> Unit,
     modifier: Modifier = Modifier,
     enabled: Boolean = true,
+    focusable: Boolean = true,
     style: IconButtonStyle = JewelTheme.iconButtonStyle,
     interactionSource: MutableInteractionSource = remember { MutableInteractionSource() },
     content: @Composable (BoxScope.(IconButtonState) -> Unit),
@@ -83,19 +88,20 @@ public fun SelectableIconButton(
     IconButtonImpl(
         state = buttonState,
         modifier =
-            modifier
-                .selectable(
-                    onClick = onClick,
-                    enabled = enabled,
-                    role = Role.RadioButton,
-                    interactionSource = interactionSource,
-                    indication = null,
-                    selected = selected,
-                )
-                .onActivated(enabled = enabled) {
-                    buttonState.value = buttonState.value.copy(active = it)
-                },
+        modifier
+            .selectable(
+                onClick = onClick,
+                enabled = enabled,
+                role = Role.RadioButton,
+                interactionSource = interactionSource,
+                indication = null,
+                selected = selected,
+            )
+            .onActivated(enabled = enabled) {
+                buttonState.value = buttonState.value.copy(active = it)
+            },
         style = style,
+        focusable = focusable,
         interactionSource = interactionSource,
         content = content,
     )
@@ -106,6 +112,7 @@ private fun IconButtonImpl(
     state: MutableState<IconButtonState>,
     modifier: Modifier,
     style: IconButtonStyle,
+    focusable: Boolean,
     interactionSource: MutableInteractionSource,
     content: @Composable (BoxScope.(IconButtonState) -> Unit),
 ) {
@@ -121,7 +128,7 @@ private fun IconButtonImpl(
 
                 is HoverInteraction.Enter -> buttonState = buttonState.copy(hovered = true)
                 is HoverInteraction.Exit -> buttonState = buttonState.copy(hovered = false)
-                is FocusInteraction.Focus -> buttonState = buttonState.copy(focused = true)
+                is FocusInteraction.Focus -> buttonState = buttonState.copy(focused = focusable)
                 is FocusInteraction.Unfocus -> buttonState = buttonState.copy(focused = false)
             }
         }
@@ -131,10 +138,13 @@ private fun IconButtonImpl(
     val border by style.colors.borderFor(buttonState)
     Box(
         modifier =
-            modifier.defaultMinSize(style.metrics.minSize.width, style.metrics.minSize.height)
-                .padding(style.metrics.padding)
-                .background(background, shape)
-                .border(style.metrics.borderWidth, border, shape),
+        Modifier
+            .thenIf(!focusable) { focusProperties { canFocus = false } }
+            .then(modifier)
+            .defaultMinSize(style.metrics.minSize.width, style.metrics.minSize.height)
+            .padding(style.metrics.padding)
+            .background(background, shape)
+            .border(style.metrics.borderWidth, border, shape),
         contentAlignment = Alignment.Center,
         content = { content(buttonState) },
     )