From 64465f4c44547dee5e9ebc841c81c2a12c6c8ec1 Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Wed, 12 Feb 2025 10:28:32 +0000 Subject: [PATCH] [JEWEL-761] Fix modifier handling in Link We had a bug in Link where, when it was enabled, it would essentially ignore the passed in Modifier. The fix is simple, but the issue was tricky to spot, as it's quite subtle! This also: * Adds a test to avoid regressions * Restores the ui-tests module, which was removed from the Gradle build by mistake (probably during a rebase/merge) * Restores the git hook setup on non-Windows OSes (it is only broken on Windows) and proposes a solution for Windows for Kuba to test (cherry picked from commit 4253a1f4d1effaaa54222490bc6321e87be8d7da) (cherry picked from commit 5a267be50ffd4eabb18c8fa3c794e49222f703c5) IJ-MR-155570 GitOrigin-RevId: 1abe870f00e7df1e44116f2feee4b6e0221d3509 --- platform/jewel/settings.gradle.kts | 36 ++++++++----------- .../jewel/ui/component/LinkUiTest.kt | 21 +++++++++++ .../org/jetbrains/jewel/ui/component/Link.kt | 4 +-- 3 files changed, 37 insertions(+), 24 deletions(-) create mode 100644 platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/LinkUiTest.kt diff --git a/platform/jewel/settings.gradle.kts b/platform/jewel/settings.gradle.kts index f090945ebd97..b09111f43ce0 100644 --- a/platform/jewel/settings.gradle.kts +++ b/platform/jewel/settings.gradle.kts @@ -43,7 +43,6 @@ include( ":markdown:core", ":markdown:extension:autolink", ":markdown:extension:gfm-alerts", - ":markdown:extension:gfm-strikethrough", ":markdown:extension:gfm-tables", ":markdown:int-ui-standalone-styling", ":markdown:ide-laf-bridge-styling", @@ -66,45 +65,40 @@ val isWindows get() = System.getProperty("os.name").contains("win", true) val gradleCommand: String by - lazy(LazyThreadSafetyMode.NONE) { - val gradlewFilename = - if (isWindows) { - "gradlew.bat" - } else { - "gradlew" - } - - val gradlew = File(rootProject.projectDir, gradlewFilename) - if (gradlew.exists() && gradlew.isFile && gradlew.canExecute()) { - logger.info("Using gradlew wrapper at ${gradlew.invariantSeparatorsPath}") - gradlew.invariantSeparatorsPath +lazy(LazyThreadSafetyMode.NONE) { + val gradlewFilename = + if (isWindows) { + "gradlew.bat" } else { - "gradle" + "gradlew" } + + val gradlew = File(rootProject.projectDir, gradlewFilename) + if (gradlew.exists() && gradlew.isFile && gradlew.canExecute()) { + logger.info("Using gradlew wrapper at ${gradlew.invariantSeparatorsPath}") + gradlew.invariantSeparatorsPath + } else { + "gradle" } +} val shebang = if (isWindows) "" else "#!/bin/sh" /* -// This is broken on Windows, please do not enable it again until it is fixed. +// This is broken on Windows, please do not enable it again until it is fixed. gitHooks { hook("pre-push") { from(shebang) { // language=Shell Script """ |#### Note: this hook was autogenerated. You can edit it in settings.gradle.kts - |OLD_DIR=$(pwd) - |cd ${rootDir.absolutePath} - |GRADLEW=./gradlew + |GRADLEW=$gradleCommand |if ! ${'$'}GRADLEW ktfmtCheck ; then | ${'$'}GRADLEW ktfmtFormat | echo 1>&2 "\nktfmt found problems; commit the result and re-push" - | cd ${'$'}OLD_DIR | exit 1 |fi | - |cd ${'$'}OLD_DIR - | """ .trimMargin() } diff --git a/platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/LinkUiTest.kt b/platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/LinkUiTest.kt new file mode 100644 index 000000000000..2cafde218a44 --- /dev/null +++ b/platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/LinkUiTest.kt @@ -0,0 +1,21 @@ +package org.jetbrains.jewel.ui.component + +import androidx.compose.ui.Modifier +import androidx.compose.ui.platform.testTag +import androidx.compose.ui.test.junit4.createComposeRule +import androidx.compose.ui.test.onNodeWithTag +import androidx.compose.ui.test.onNodeWithText +import org.jetbrains.jewel.intui.standalone.theme.IntUiTheme +import org.junit.Rule +import org.junit.Test + +class LinkUiTest { + @get:Rule val rule = createComposeRule() + + @Test + fun `should apply provided modifier correctly`() { + rule.setContent { IntUiTheme { Link("Whatever", {}, modifier = Modifier.testTag("123banana")) } } + rule.onNodeWithText("Whatever").assertExists() + rule.onNodeWithTag("123banana").assertExists() + } +} diff --git a/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/Link.kt b/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/Link.kt index 5a54e33e561c..f1e637a585df 100644 --- a/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/Link.kt +++ b/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/Link.kt @@ -202,12 +202,10 @@ private fun LinkImpl( textStyle.merge(textDecoration = decoration, color = textColor) } - val pointerChangeModifier = Modifier.pointerHoverIcon(PointerIcon(Cursor(Cursor.HAND_CURSOR))) - Row( modifier = modifier - .thenIf(linkState.isEnabled) { pointerChangeModifier } + .thenIf(linkState.isEnabled) { pointerHoverIcon(PointerIcon(Cursor(Cursor.HAND_CURSOR))) } .clickable( onClick = { linkState = linkState.copy(visited = true)