From 119c8567c688e16f84ca3fe53ea82eabdb1cef43 Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Mon, 15 Jul 2024 12:57:15 +0200 Subject: [PATCH] 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 --------- Signed-off-by: Ivan Morgillo Co-authored-by: Ivan Morgillo --- .../jewel/bridge/theme/IntUiBridge.kt | 4 +- .../styling/IntUiIconButtonStyling.kt | 6 +- .../standalone/view/component/Buttons.kt | 48 +++++++++++++++ ui/api/ui.api | 4 +- .../jewel/ui/component/IconButton.kt | 58 +++++++++++-------- 5 files changed, 89 insertions(+), 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 index 983f82f88..3a02562c4 100644 --- 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/int-ui/int-ui-standalone/src/main/kotlin/org/jetbrains/jewel/intui/standalone/styling/IntUiIconButtonStyling.kt b/int-ui/int-ui-standalone/src/main/kotlin/org/jetbrains/jewel/intui/standalone/styling/IntUiIconButtonStyling.kt index 5e09d392e..b56bd034a 100644 --- a/int-ui/int-ui-standalone/src/main/kotlin/org/jetbrains/jewel/intui/standalone/styling/IntUiIconButtonStyling.kt +++ b/int-ui/int-ui-standalone/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/samples/standalone/src/main/kotlin/org/jetbrains/jewel/samples/standalone/view/component/Buttons.kt b/samples/standalone/src/main/kotlin/org/jetbrains/jewel/samples/standalone/view/component/Buttons.kt index aa29ac29f..866f93da7 100644 --- a/samples/standalone/src/main/kotlin/org/jetbrains/jewel/samples/standalone/view/component/Buttons.kt +++ b/samples/standalone/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 index 9d82334a0..9b7ba4d9c 100644 --- 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 index 6a8f30aee..a8f07877e 100644 --- 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, 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) }, )