Skip to content

Commit

Permalink
[JEWEL-761] Fix modifier handling in Link
Browse files Browse the repository at this point in the history
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
  • Loading branch information
rock3r committed Feb 13, 2025
1 parent 797c6cc commit 6f178db
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 41 deletions.
80 changes: 42 additions & 38 deletions platform/jewel/settings.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@ pluginManagement {
gradlePluginPortal()
mavenCentral()
}
plugins {
kotlin("jvm") version "2.1.0"
}
plugins { kotlin("jvm") version "2.1.0" }
}

dependencyResolutionManagement {
Expand Down Expand Up @@ -52,7 +50,7 @@ include(
":samples:showcase",
":samples:standalone",
":ui",
":ui-test",
":ui-tests",
)

gradleEnterprise {
Expand All @@ -66,45 +64,51 @@ gradleEnterprise {
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
} else {
"gradle"
}
}

val shebang = if (isWindows) "" else "#!/bin/sh"

/*
// 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
|GRADLEW=$gradleCommand
|if ! ${'$'}GRADLEW ktfmtCheck ; then
| ${'$'}GRADLEW ktfmtFormat
| echo 1>&2 "\nktfmt found problems; commit the result and re-push"
| exit 1
|fi
|
"""
.trimMargin()
if (!isWindows) {
from(shebang) {
"""
|set -x
|#### Note: this hook was autogenerated. You can edit it in Jewel's settings.gradle.kts
|GRADLEW="./gradlew"
|OLD_DIR=$(pwd)
|cd "${rootDir.absolutePath}"
|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()
}
} else {
// TODO @jakub fix this for Windows — the following _should_ work but I can't test it
// from(shebang) {
// """
// |:: Note: this hook was autogenerated. You can edit it in Jewel's settings.gradle.kts
// |SET GRADLEW=gradlew.bat
// |
// |SET OLD_DIR=%CD%
// |CD /D ${rootDir.absolutePath}
// |%GRADLEW% ktfmtCheck || (
// | %GRADLEW% ktfmtFormat
// | echo "ktfmt found problems; commit the result and re-push"
// | CD /D %OLD_DIR%
// | EXIT 1
// |)
// |
// |CD /D %OLD_DIR%
// """
// .trimMargin()
// }
}
}

createHooks(overwriteExisting = true)
}*/
}
Original file line number Diff line number Diff line change
@@ -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()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 6f178db

Please sign in to comment.