Implement proper Markdown link styling

We weren't using the TextLinkStyles provided by the LinkAnnotation API, and as a result, our links weren't stateful. We were also not properly setting a disabled colour — now we do. Also changed, we force the Markdown text to not be focusable, even if it is clickable, since we don't want it to get focused. Now only links are focused while tabbing through Markdown. This also removes some testing harness left around from #425, and that we don't need anymore. Note: links should have a border around them when they are focused, but that's not possible with the Compose APIs. What we do instead is show a subtle background color, taken from the ActionButtons' hover and pressed states, for our focused and pressed states, respectively.
JetBrains · Jul 18, 2024 · 948d8f0 · 948d8f0
1 parent 4dcf63b
commit 948d8f0
Show file tree

Hide file tree

Showing 13 changed files with 131 additions and 70 deletions.
diff --git a/markdown/core/api/core.api b/markdown/core/api/core.api
@@ -509,12 +509,17 @@ public final class org/jetbrains/jewel/markdown/rendering/InlineMarkdownRenderer
 public final class org/jetbrains/jewel/markdown/rendering/InlinesStyling {
 	public static final field $stable I
 	public static final field Companion Lorg/jetbrains/jewel/markdown/rendering/InlinesStyling$Companion;
-	public fun <init> (Landroidx/compose/ui/text/TextStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Z)V
+	public fun <init> (Landroidx/compose/ui/text/TextStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Z)V
 	public fun equals (Ljava/lang/Object;)Z
 	public final fun getEmphasis ()Landroidx/compose/ui/text/SpanStyle;
 	public final fun getInlineCode ()Landroidx/compose/ui/text/SpanStyle;
 	public final fun getInlineHtml ()Landroidx/compose/ui/text/SpanStyle;
 	public final fun getLink ()Landroidx/compose/ui/text/SpanStyle;
+	public final fun getLinkDisabled ()Landroidx/compose/ui/text/SpanStyle;
+	public final fun getLinkFocused ()Landroidx/compose/ui/text/SpanStyle;
+	public final fun getLinkHovered ()Landroidx/compose/ui/text/SpanStyle;
+	public final fun getLinkPressed ()Landroidx/compose/ui/text/SpanStyle;
+	public final fun getLinkVisited ()Landroidx/compose/ui/text/SpanStyle;
 	public final fun getRenderInlineHtml ()Z
 	public final fun getStrongEmphasis ()Landroidx/compose/ui/text/SpanStyle;
 	public final fun getTextStyle ()Landroidx/compose/ui/text/TextStyle;

diff --git a/markdown/core/src/main/kotlin/org/jetbrains/jewel/markdown/Markdown.kt b/markdown/core/src/main/kotlin/org/jetbrains/jewel/markdown/Markdown.kt
@@ -15,6 +15,7 @@ import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.remember
 import androidx.compose.runtime.setValue
 import androidx.compose.ui.Modifier
+import androidx.compose.ui.focus.focusProperties
 import androidx.compose.ui.semantics.semantics
 import androidx.compose.ui.unit.dp
 import kotlinx.coroutines.CoroutineDispatcher
@@ -78,7 +79,11 @@ public fun Markdown(
     blockRenderer: MarkdownBlockRenderer = DefaultMarkdownBlockRenderer(markdownStyling),
 ) {
     if (selectable) {
-        SelectionContainer(modifier.semantics { rawMarkdown = markdown }) {
+        SelectionContainer(
+            Modifier.focusProperties { canFocus = false }
+                .then(modifier)
+                .semantics { rawMarkdown = markdown },
+        ) {
             Column(verticalArrangement = Arrangement.spacedBy(markdownStyling.blockVerticalSpacing)) {
                 for (block in markdownBlocks) {
                     blockRenderer.render(block, enabled, onUrlClick, onTextClick)
@@ -87,7 +92,9 @@ public fun Markdown(
         }
     } else {
         Column(
-            modifier.semantics { rawMarkdown = markdown },
+            Modifier.focusProperties { canFocus = false }
+                .then(modifier)
+                .semantics { rawMarkdown = markdown },
             verticalArrangement = Arrangement.spacedBy(markdownStyling.blockVerticalSpacing),
         ) {
             for (block in markdownBlocks) {

diff --git a/...e/src/main/kotlin/org/jetbrains/jewel/markdown/rendering/DefaultInlineMarkdownRenderer.kt b/...e/src/main/kotlin/org/jetbrains/jewel/markdown/rendering/DefaultInlineMarkdownRenderer.kt
@@ -6,6 +6,7 @@ import androidx.compose.ui.text.AnnotatedString
 import androidx.compose.ui.text.AnnotatedString.Builder
 import androidx.compose.ui.text.LinkAnnotation
 import androidx.compose.ui.text.SpanStyle
+import androidx.compose.ui.text.TextLinkStyles
 import androidx.compose.ui.text.buildAnnotatedString
 import org.commonmark.renderer.text.TextContentRenderer
 import org.jetbrains.jewel.foundation.ExperimentalJewelApi
@@ -40,6 +41,15 @@ public open class DefaultInlineMarkdownRenderer(
         enabled: Boolean,
         onUrlClicked: ((String) -> Unit)? = null,
     ) {
+        // TODO move to InlineMarkdown to avoid recomputing after #416 is done
+        val linkStyling =
+            TextLinkStyles(
+                styling.link,
+                styling.linkFocused,
+                styling.linkHovered,
+                styling.linkPressed,
+            )
+
         for (child in inlineMarkdown) {
             when (child) {
                 is InlineMarkdown.Text -> append(child.nativeNode.literal)
@@ -62,18 +72,21 @@ public open class DefaultInlineMarkdownRenderer(
                 }
 
                 is InlineMarkdown.Link -> {
-                    withStyles(styling.link.withEnabled(enabled), child) {
+                    val index =
                         if (enabled) {
-                            val destination = it.nativeNode.destination
+                            val destination = child.nativeNode.destination
                             val link =
                                 LinkAnnotation.Clickable(
                                     tag = destination,
                                     linkInteractionListener = { _ -> onUrlClicked?.invoke(destination) },
+                                    styles = linkStyling,
                                 )
                             pushLink(link)
+                        } else {
+                            pushStyle(styling.linkDisabled)
                         }
-                        appendInlineMarkdownFrom(it.children, styling, enabled)
-                    }
+                    appendInlineMarkdownFrom(child.children, styling, enabled)
+                    pop(index)
                 }
 
                 is InlineMarkdown.Code -> {

diff --git a/...re/src/main/kotlin/org/jetbrains/jewel/markdown/rendering/DefaultMarkdownBlockRenderer.kt b/...re/src/main/kotlin/org/jetbrains/jewel/markdown/rendering/DefaultMarkdownBlockRenderer.kt
@@ -34,6 +34,7 @@ import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.draw.alpha
 import androidx.compose.ui.draw.drawBehind
+import androidx.compose.ui.focus.focusProperties
 import androidx.compose.ui.geometry.Offset
 import androidx.compose.ui.graphics.Color
 import androidx.compose.ui.graphics.isSpecified
@@ -141,11 +142,13 @@ public open class DefaultMarkdownBlockRenderer(
 
         Text(
             modifier =
-                Modifier.clickable(
-                    interactionSource = interactionSource,
-                    indication = null,
-                    onClick = onTextClick,
-                ),
+                Modifier
+                    .focusProperties { canFocus = false }
+                    .clickable(
+                        interactionSource = interactionSource,
+                        indication = null,
+                        onClick = onTextClick,
+                    ),
             text = renderedContent,
             style = mergedStyle,
         )
@@ -212,6 +215,7 @@ public open class DefaultMarkdownBlockRenderer(
             Text(
                 text = renderedContent,
                 style = mergedStyle,
+                modifier = Modifier.focusProperties { canFocus = false },
             )
 
             if (underlineWidth > 0.dp && underlineColor.isSpecified) {
@@ -295,6 +299,7 @@ public open class DefaultMarkdownBlockRenderer(
                         color = styling.numberStyle.color.takeOrElse { LocalContentColor.current },
                         modifier =
                             Modifier
+                                .focusProperties { canFocus = false }
                                 .widthIn(min = styling.numberMinWidth)
                                 .pointerHoverIcon(PointerIcon.Default, overrideDescendants = true),
                         textAlign = styling.numberTextAlign,
@@ -333,7 +338,9 @@ public open class DefaultMarkdownBlockRenderer(
                         text = styling.bullet.toString(),
                         style = styling.bulletStyle,
                         color = styling.bulletStyle.color.takeOrElse { LocalContentColor.current },
-                        modifier = Modifier.pointerHoverIcon(PointerIcon.Default, overrideDescendants = true),
+                        modifier =
+                            Modifier.focusProperties { canFocus = false }
+                                .pointerHoverIcon(PointerIcon.Default, overrideDescendants = true),
                     )
 
                     Spacer(Modifier.width(styling.bulletContentGap))
@@ -384,7 +391,7 @@ public open class DefaultMarkdownBlockRenderer(
                 style = styling.editorTextStyle,
                 color = styling.editorTextStyle.color.takeOrElse { LocalContentColor.current },
                 modifier =
-                    Modifier
+                    Modifier.focusProperties { canFocus = false }
                         .padding(styling.padding)
                         .pointerHoverIcon(PointerIcon.Default, overrideDescendants = true),
             )
@@ -418,7 +425,9 @@ public open class DefaultMarkdownBlockRenderer(
                     text = block.content,
                     style = styling.editorTextStyle,
                     color = styling.editorTextStyle.color.takeOrElse { LocalContentColor.current },
-                    modifier = Modifier.pointerHoverIcon(PointerIcon.Default, overrideDescendants = true),
+                    modifier =
+                        Modifier.focusProperties { canFocus = false }
+                            .pointerHoverIcon(PointerIcon.Default, overrideDescendants = true),
                 )
 
                 if (block.mimeType != null && styling.infoPosition.verticalAlignment == Alignment.Bottom) {
@@ -447,6 +456,7 @@ public open class DefaultMarkdownBlockRenderer(
                     text = infoText,
                     style = textStyle,
                     color = textStyle.color.takeOrElse { LocalContentColor.current },
+                    modifier = Modifier.focusProperties { canFocus = false },
                 )
             }
         }

diff --git a/markdown/core/src/main/kotlin/org/jetbrains/jewel/markdown/rendering/MarkdownStyling.kt b/markdown/core/src/main/kotlin/org/jetbrains/jewel/markdown/rendering/MarkdownStyling.kt
@@ -269,6 +269,11 @@ public class InlinesStyling(
     public val textStyle: TextStyle,
     public val inlineCode: SpanStyle,
     public val link: SpanStyle,
+    public val linkDisabled: SpanStyle,
+    public val linkFocused: SpanStyle,
+    public val linkHovered: SpanStyle,
+    public val linkPressed: SpanStyle,
+    public val linkVisited: SpanStyle,
     public val emphasis: SpanStyle,
     public val strongEmphasis: SpanStyle,
     public val inlineHtml: SpanStyle,

diff --git a/markdown/ide-laf-bridge-styling/api/ide-laf-bridge-styling.api b/markdown/ide-laf-bridge-styling/api/ide-laf-bridge-styling.api
@@ -8,12 +8,12 @@ public final class org/jetbrains/jewel/intui/markdown/bridge/BridgeProvideMarkdo
 }
 
 public final class org/jetbrains/jewel/intui/markdown/bridge/styling/BridgeMarkdownStylingKt {
-	public static final fun create (Lorg/jetbrains/jewel/markdown/rendering/InlinesStyling$Companion;Landroidx/compose/ui/text/TextStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Z)Lorg/jetbrains/jewel/markdown/rendering/InlinesStyling;
+	public static final fun create (Lorg/jetbrains/jewel/markdown/rendering/InlinesStyling$Companion;Landroidx/compose/ui/text/TextStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Z)Lorg/jetbrains/jewel/markdown/rendering/InlinesStyling;
 	public static final fun create (Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$Code$Companion;Landroidx/compose/ui/text/TextStyle;Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$Code$Indented;Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$Code$Fenced;)Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$Code;
 	public static final fun create (Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$Heading$Companion;Landroidx/compose/ui/text/TextStyle;Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$Heading$H1;Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$Heading$H2;Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$Heading$H3;Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$Heading$H4;Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$Heading$H5;Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$Heading$H6;)Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$Heading;
 	public static final fun create (Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$List$Companion;Landroidx/compose/ui/text/TextStyle;Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$List$Ordered;Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$List$Unordered;)Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$List;
 	public static final fun create (Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$Paragraph$Companion;Lorg/jetbrains/jewel/markdown/rendering/InlinesStyling;)Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$Paragraph;
-	public static synthetic fun create$default (Lorg/jetbrains/jewel/markdown/rendering/InlinesStyling$Companion;Landroidx/compose/ui/text/TextStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;ZILjava/lang/Object;)Lorg/jetbrains/jewel/markdown/rendering/InlinesStyling;
+	public static synthetic fun create$default (Lorg/jetbrains/jewel/markdown/rendering/InlinesStyling$Companion;Landroidx/compose/ui/text/TextStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;Landroidx/compose/ui/text/SpanStyle;ZILjava/lang/Object;)Lorg/jetbrains/jewel/markdown/rendering/InlinesStyling;
 	public static synthetic fun create$default (Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$Code$Companion;Landroidx/compose/ui/text/TextStyle;Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$Code$Indented;Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$Code$Fenced;ILjava/lang/Object;)Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$Code;
 	public static synthetic fun create$default (Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$Heading$Companion;Landroidx/compose/ui/text/TextStyle;Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$Heading$H1;Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$Heading$H2;Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$Heading$H3;Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$Heading$H4;Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$Heading$H5;Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$Heading$H6;ILjava/lang/Object;)Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$Heading;
 	public static synthetic fun create$default (Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$List$Companion;Landroidx/compose/ui/text/TextStyle;Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$List$Ordered;Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$List$Unordered;ILjava/lang/Object;)Lorg/jetbrains/jewel/markdown/rendering/MarkdownStyling$List;

diff --git a/...rc/main/kotlin/org/jetbrains/jewel/intui/markdown/bridge/styling/BridgeMarkdownStyling.kt b/...rc/main/kotlin/org/jetbrains/jewel/intui/markdown/bridge/styling/BridgeMarkdownStyling.kt
@@ -371,19 +371,37 @@ public fun InlinesStyling.Companion.create(
             color = JBUI.CurrentTheme.Link.Foreground.ENABLED.toComposeColor(),
             textDecoration = TextDecoration.Underline,
         ).toSpanStyle(),
+    linkDisabled: SpanStyle = link.copy(color = JBUI.CurrentTheme.Link.Foreground.DISABLED.toComposeColor()),
+    linkHovered: SpanStyle = link.copy(color = JBUI.CurrentTheme.Link.Foreground.HOVERED.toComposeColor()),
+    linkFocused: SpanStyle =
+        link.copy(
+            color = JBUI.CurrentTheme.Link.Foreground.ENABLED.toComposeColor(),
+            background = JBUI.CurrentTheme.ActionButton.hoverBackground().toComposeColor(),
+        ),
+    linkPressed: SpanStyle =
+        link.copy(
+            color = JBUI.CurrentTheme.Link.Foreground.PRESSED.toComposeColor(),
+            background = JBUI.CurrentTheme.ActionButton.pressedBackground().toComposeColor(),
+        ),
+    linkVisited: SpanStyle = link.copy(color = JBUI.CurrentTheme.Link.Foreground.VISITED.toComposeColor()),
     emphasis: SpanStyle = textStyle.copy(fontStyle = FontStyle.Italic).toSpanStyle(),
     strongEmphasis: SpanStyle = textStyle.copy(fontWeight = FontWeight.Bold).toSpanStyle(),
     inlineHtml: SpanStyle = textStyle.toSpanStyle(),
     renderInlineHtml: Boolean = false,
 ): InlinesStyling =
     InlinesStyling(
-        textStyle,
-        inlineCode,
-        link,
-        emphasis,
-        strongEmphasis,
-        inlineHtml,
-        renderInlineHtml,
+        textStyle = textStyle,
+        inlineCode = inlineCode,
+        link = link,
+        linkDisabled = linkDisabled,
+        linkHovered = linkHovered,
+        linkFocused = linkFocused,
+        linkPressed = linkPressed,
+        linkVisited = linkVisited,
+        emphasis = emphasis,
+        strongEmphasis = strongEmphasis,
+        inlineHtml = inlineHtml,
+        renderInlineHtml = renderInlineHtml,
     )
 
 private val defaultTextStyle