Skip to content

Commit

Permalink
Use a line-height of 1.3x for the default text styles (#587)
Browse files Browse the repository at this point in the history 
* Use a line-height of 1.3x for the default text styles

It looks like Swing uses a line height that is 1.3 times the font size,
but that is not expressed anywhere I could find. I got the value by
experimentation and pixel counting, like a proper DF review would 🙃
Using this value seems to work at both default and h[0-5] text sizes.

Having a non-default line height is a bit of a pain for users, since
they need to remember updating the line height too when they customize
the font size in a text style; however, we can't do anything about it,
and in general, folks should stick to the predefined text styles.

To help, we now provide a `TextStyle.copyWithSize()` function that
automatically sets the correct line height, making this slightly less
painful.

* Actually use the line height constant in the bridge

Also solves the issue that we were ignoring the passed in line height;
now we use the default only as fallback if a height is not passed in.
  • Loading branch information
@rock3r
rock3r authored Sep 9, 2024
1 parent dea5c80 commit 0c0680e
Show file tree
Hide file tree
Showing 8 changed files with 246 additions and 16 deletions.
4 changes: 2 additions & 2 deletions ide-laf-bridge/api/ide-laf-bridge.api
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@ public final class org/jetbrains/jewel/bridge/BridgeUtilsKt {
public static final fun retrieveIntAsDp-3F_vd3o (Ljava/lang/String;Landroidx/compose/ui/unit/Dp;)F
public static synthetic fun retrieveIntAsDp-3F_vd3o$default (Ljava/lang/String;Landroidx/compose/ui/unit/Dp;ILjava/lang/Object;)F
public static final fun retrieveIntAsDpOrUnspecified (Ljava/lang/String;)F
public static final fun retrieveTextStyle (Ljava/lang/String;Ljava/lang/String;)Landroidx/compose/ui/text/TextStyle;
public static synthetic fun retrieveTextStyle$default (Ljava/lang/String;Ljava/lang/String;ILjava/lang/Object;)Landroidx/compose/ui/text/TextStyle;
public static final fun retrieveTextStyle-WdJyH8Q (Ljava/lang/String;JJZIJ)Landroidx/compose/ui/text/TextStyle;
public static synthetic fun retrieveTextStyle-WdJyH8Q$default (Ljava/lang/String;JJZIJILjava/lang/Object;)Landroidx/compose/ui/text/TextStyle;
public static final fun retrieveTextStyle-tD9LlGs (Ljava/lang/String;Ljava/lang/String;JZIJ)Landroidx/compose/ui/text/TextStyle;
public static synthetic fun retrieveTextStyle-tD9LlGs$default (Ljava/lang/String;Ljava/lang/String;JZIJILjava/lang/Object;)Landroidx/compose/ui/text/TextStyle;
public static final fun toComposeColor (Ljava/awt/Color;)J
public static final fun toComposeColorOrUnspecified (Ljava/awt/Color;)J
public static final fun toDpSize (Lcom/intellij/util/ui/JBDimension;)J
Expand Down
15 changes: 13 additions & 2 deletions ide-laf-bridge/src/main/kotlin/org/jetbrains/jewel/bridge/BridgeUtils.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import com.intellij.util.ui.JBValue
import java.awt.Dimension
import java.awt.Insets
import javax.swing.UIManager
import org.jetbrains.jewel.ui.component.Typography

private val logger = Logger.getInstance("JewelBridge")

Expand Down Expand Up @@ -148,9 +149,19 @@ public fun retrieveArcAsCornerSizeWithFallbacks(vararg keys: String): CornerSize
keysNotFound(keys.toList(), "Int")
}

public fun retrieveTextStyle(fontKey: String, colorKey: String? = null): TextStyle {
public fun retrieveTextStyle(
fontKey: String,
colorKey: String? = null,
lineHeight: TextUnit = TextUnit.Unspecified,
bold: Boolean = false,
fontStyle: FontStyle = FontStyle.Normal,
size: TextUnit = TextUnit.Unspecified,
): TextStyle {
val baseColor = colorKey?.let { retrieveColorOrUnspecified(colorKey) } ?: Color.Unspecified
return retrieveTextStyle(fontKey, color = baseColor)
val resolvedStyle = retrieveTextStyle(fontKey, color = baseColor, lineHeight, bold, fontStyle, size)
return resolvedStyle.copy(
lineHeight = lineHeight.takeOrElse { resolvedStyle.fontSize * Typography.DefaultLineHeightMultiplier }
)
}

@OptIn(ExperimentalTextApi::class)
Expand Down
3 changes: 2 additions & 1 deletion ide-laf-bridge/src/main/kotlin/org/jetbrains/jewel/bridge/SwingBridgeService.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import org.jetbrains.jewel.bridge.theme.createBridgeComponentStyling
import org.jetbrains.jewel.bridge.theme.createBridgeThemeDefinition
import org.jetbrains.jewel.foundation.theme.ThemeDefinition
import org.jetbrains.jewel.ui.ComponentStyling
import org.jetbrains.jewel.ui.component.copyWithSize

@Service(Level.APP)
internal class SwingBridgeService(scope: CoroutineScope) {
Expand Down Expand Up @@ -55,7 +56,7 @@ internal class SwingBridgeService(scope: CoroutineScope) {
internal data class BridgeThemeData(val themeDefinition: ThemeDefinition, val componentStyling: ComponentStyling) {
companion object {
val DEFAULT = run {
val textStyle = TextStyle.Default.copy(fontSize = 13.sp)
val textStyle = TextStyle.Default.copyWithSize(fontSize = 13.sp)
val monospaceTextStyle = textStyle.copy(fontFamily = FontFamily.Monospace)
val themeDefinition =
createBridgeThemeDefinition(
Expand Down
5 changes: 3 additions & 2 deletions ide-laf-bridge/src/main/kotlin/org/jetbrains/jewel/bridge/Typography.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import com.intellij.openapi.util.SystemInfo
import com.intellij.ui.NewUiValue
import org.jetbrains.jewel.foundation.theme.JewelTheme
import org.jetbrains.jewel.ui.component.Typography
import org.jetbrains.jewel.ui.component.copyWithSize
import org.jetbrains.jewel.ui.component.minus

/**
Expand All @@ -29,7 +30,7 @@ public fun Typography.medium(): TextStyle =
if (mediumAndSmallFontsAsRegular()) {
labelTextStyle()
} else {
labelTextStyle().copy(fontSize = labelTextSize() - 1.sp)
labelTextStyle().copyWithSize(fontSize = labelTextSize() - 1.sp)
}

/**
Expand All @@ -46,7 +47,7 @@ public fun Typography.small(): TextStyle =
if (mediumAndSmallFontsAsRegular()) {
labelTextStyle()
} else {
labelTextStyle().copy(fontSize = labelTextSize() - 2.sp)
labelTextStyle().copyWithSize(fontSize = labelTextSize() - 2.sp)
}

// Copied from JBFont — current as of IJP 233.
Expand Down
9 changes: 5 additions & 4 deletions ...nt-ui-standalone/src/main/kotlin/org/jetbrains/jewel/intui/standalone/theme/TextStyles.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import androidx.compose.ui.unit.sp
import org.jetbrains.jewel.foundation.theme.JewelTheme
import org.jetbrains.jewel.intui.standalone.Inter
import org.jetbrains.jewel.intui.standalone.JetBrainsMono
import org.jetbrains.jewel.ui.component.Typography

public fun JewelTheme.Companion.createDefaultTextStyle(
color: Color = Color.Unspecified,
Expand All @@ -45,7 +46,7 @@ public fun JewelTheme.Companion.createDefaultTextStyle(
drawStyle: DrawStyle? = null,
textAlign: TextAlign = TextAlign.Unspecified,
textDirection: TextDirection = TextDirection.Unspecified,
lineHeight: TextUnit = TextUnit.Unspecified,
lineHeight: TextUnit = fontSize * Typography.DefaultLineHeightMultiplier,
textIndent: TextIndent? = null,
platformStyle: PlatformTextStyle? = null,
lineHeightStyle: LineHeightStyle? = null,
Expand Down Expand Up @@ -99,7 +100,7 @@ public fun JewelTheme.Companion.createDefaultTextStyle(
drawStyle: DrawStyle? = null,
textAlign: TextAlign = TextAlign.Unspecified,
textDirection: TextDirection = TextDirection.Unspecified,
lineHeight: TextUnit = TextUnit.Unspecified,
lineHeight: TextUnit = fontSize * Typography.DefaultLineHeightMultiplier,
textIndent: TextIndent? = null,
platformStyle: PlatformTextStyle? = null,
lineHeightStyle: LineHeightStyle? = null,
Expand Down Expand Up @@ -153,7 +154,7 @@ public fun JewelTheme.Companion.createEditorTextStyle(
drawStyle: DrawStyle? = null,
textAlign: TextAlign = TextAlign.Unspecified,
textDirection: TextDirection = TextDirection.Unspecified,
lineHeight: TextUnit = TextUnit.Unspecified,
lineHeight: TextUnit = fontSize * Typography.EditorLineHeightMultiplier,
textIndent: TextIndent? = null,
platformStyle: PlatformTextStyle? = null,
lineHeightStyle: LineHeightStyle? = null,
Expand Down Expand Up @@ -207,7 +208,7 @@ public fun JewelTheme.Companion.createEditorTextStyle(
drawStyle: DrawStyle? = null,
textAlign: TextAlign = TextAlign.Unspecified,
textDirection: TextDirection = TextDirection.Unspecified,
lineHeight: TextUnit = TextUnit.Unspecified,
lineHeight: TextUnit = fontSize * Typography.EditorLineHeightMultiplier,
textIndent: TextIndent? = null,
platformStyle: PlatformTextStyle? = null,
lineHeightStyle: LineHeightStyle? = null,
Expand Down
45 changes: 45 additions & 0 deletions ...e-plugin/src/main/kotlin/org/jetbrains/jewel/samples/ideplugin/SwingComparisonTabPanel.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@ package org.jetbrains.jewel.samples.ideplugin

import androidx.compose.foundation.border
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.BoxWithConstraints
import androidx.compose.foundation.layout.PaddingValues
import androidx.compose.foundation.layout.calculateEndPadding
import androidx.compose.foundation.layout.calculateStartPadding
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.layout.size
import androidx.compose.foundation.layout.width
import androidx.compose.foundation.text.input.rememberTextFieldState
import androidx.compose.runtime.Composable
import androidx.compose.runtime.remember
Expand Down Expand Up @@ -103,6 +105,49 @@ internal class SwingComparisonTabPanel : BorderLayoutPanel() {
}
}
.layout(RowLayout.PARENT_GRID)

val longText = "WordWrapInsideWordsIsSupported:" + ("NoSpace".repeat(20) + " ").repeat(5) + "End"
row("Long text (Swing)") { text(longText, maxLineLength = 100) }
row("Long text (Compose)") {
compose {
BoxWithConstraints {
Text(
longText,
modifier =
Modifier.width(
with(LocalDensity.current) {
// Guesstimate how wide this should be — we can't tell it to be
// "fill", as it crashes natively
JewelTheme.defaultTextStyle.fontSize.toDp() * 60
}
),
)
}
}
}

row("Titles (Swing)") {
text("This will wrap over a couple rows", maxLineLength = 30).component.font = JBFont.h1()
}
row("Titles (Compose)") {
compose {
BoxWithConstraints {
val style = Typography.h1TextStyle()
Text(
"This will wrap over a couple rows",
modifier =
Modifier.width(
with(LocalDensity.current) {
// Guesstimate how wide this should be — we can't tell it to be
// "fill", as it crashes natively
style.fontSize.toDp() * 10
}
),
style = style,
)
}
}
}
}

private fun Panel.iconsRow() {
Expand Down
6 changes: 6 additions & 0 deletions ui/api/ui.api
View file
Original file line number Diff line number Diff line change
Expand Up @@ -952,6 +952,8 @@ public final class org/jetbrains/jewel/ui/component/TooltipKt {

public final class org/jetbrains/jewel/ui/component/Typography {
public static final field $stable I
public static final field DefaultLineHeightMultiplier F
public static final field EditorLineHeightMultiplier F
public static final field INSTANCE Lorg/jetbrains/jewel/ui/component/Typography;
public final fun consoleTextStyle (Landroidx/compose/runtime/Composer;I)Landroidx/compose/ui/text/TextStyle;
public final fun editorTextStyle (Landroidx/compose/runtime/Composer;I)Landroidx/compose/ui/text/TextStyle;
Expand All @@ -965,6 +967,10 @@ public final class org/jetbrains/jewel/ui/component/Typography {
}

public final class org/jetbrains/jewel/ui/component/TypographyKt {
public static final fun copyWithSize-Ce1Lr_4 (Landroidx/compose/ui/text/TextStyle;JJLandroidx/compose/ui/text/font/FontWeight;Landroidx/compose/ui/text/font/FontStyle;Landroidx/compose/ui/text/font/FontSynthesis;Landroidx/compose/ui/text/font/FontFamily;Ljava/lang/String;JLandroidx/compose/ui/text/style/BaselineShift;Landroidx/compose/ui/text/style/TextGeometricTransform;Landroidx/compose/ui/text/intl/LocaleList;JLandroidx/compose/ui/text/style/TextDecoration;Landroidx/compose/ui/graphics/Shadow;Landroidx/compose/ui/graphics/drawscope/DrawStyle;IILandroidx/compose/ui/text/style/TextIndent;Landroidx/compose/ui/text/PlatformTextStyle;Landroidx/compose/ui/text/style/LineHeightStyle;IILandroidx/compose/ui/text/style/TextMotion;)Landroidx/compose/ui/text/TextStyle;
public static synthetic fun copyWithSize-Ce1Lr_4$default (Landroidx/compose/ui/text/TextStyle;JJLandroidx/compose/ui/text/font/FontWeight;Landroidx/compose/ui/text/font/FontStyle;Landroidx/compose/ui/text/font/FontSynthesis;Landroidx/compose/ui/text/font/FontFamily;Ljava/lang/String;JLandroidx/compose/ui/text/style/BaselineShift;Landroidx/compose/ui/text/style/TextGeometricTransform;Landroidx/compose/ui/text/intl/LocaleList;JLandroidx/compose/ui/text/style/TextDecoration;Landroidx/compose/ui/graphics/Shadow;Landroidx/compose/ui/graphics/drawscope/DrawStyle;IILandroidx/compose/ui/text/style/TextIndent;Landroidx/compose/ui/text/PlatformTextStyle;Landroidx/compose/ui/text/style/LineHeightStyle;IILandroidx/compose/ui/text/style/TextMotion;ILjava/lang/Object;)Landroidx/compose/ui/text/TextStyle;
public static final fun copyWithSize-nfnV_I0 (Landroidx/compose/ui/text/TextStyle;JLandroidx/compose/ui/graphics/Brush;FLandroidx/compose/ui/text/font/FontWeight;Landroidx/compose/ui/text/font/FontStyle;Landroidx/compose/ui/text/font/FontSynthesis;Landroidx/compose/ui/text/font/FontFamily;Ljava/lang/String;JLandroidx/compose/ui/text/style/BaselineShift;Landroidx/compose/ui/text/style/TextGeometricTransform;Landroidx/compose/ui/text/intl/LocaleList;JLandroidx/compose/ui/text/style/TextDecoration;Landroidx/compose/ui/graphics/Shadow;Landroidx/compose/ui/graphics/drawscope/DrawStyle;IILandroidx/compose/ui/text/style/TextIndent;Landroidx/compose/ui/text/PlatformTextStyle;Landroidx/compose/ui/text/style/LineHeightStyle;IILandroidx/compose/ui/text/style/TextMotion;)Landroidx/compose/ui/text/TextStyle;
public static synthetic fun copyWithSize-nfnV_I0$default (Landroidx/compose/ui/text/TextStyle;JLandroidx/compose/ui/graphics/Brush;FLandroidx/compose/ui/text/font/FontWeight;Landroidx/compose/ui/text/font/FontStyle;Landroidx/compose/ui/text/font/FontSynthesis;Landroidx/compose/ui/text/font/FontFamily;Ljava/lang/String;JLandroidx/compose/ui/text/style/BaselineShift;Landroidx/compose/ui/text/style/TextGeometricTransform;Landroidx/compose/ui/text/intl/LocaleList;JLandroidx/compose/ui/text/style/TextDecoration;Landroidx/compose/ui/graphics/Shadow;Landroidx/compose/ui/graphics/drawscope/DrawStyle;IILandroidx/compose/ui/text/style/TextIndent;Landroidx/compose/ui/text/PlatformTextStyle;Landroidx/compose/ui/text/style/LineHeightStyle;IILandroidx/compose/ui/text/style/TextMotion;ILjava/lang/Object;)Landroidx/compose/ui/text/TextStyle;
public static final fun minus-NB67dxo (JJ)J
public static final fun plus-NB67dxo (JJ)J
}
Expand Down
Loading

0 comments on commit 0c0680e

Please sign in to comment.