Skip to content

Commit f87690a

Browse files
rock3rrock3r
committed
[JEWEL-920] Add Detekt Compose rules to the project
We now apply additional Compose rules to our Detekt checks. The rules are documented at https://mrmans0n.github.io/compose-rules/rules/ A couple of rules have been disabled as they do not apply to Jewel as a library. The rules JAR was added to the repository so that it can also be used by the IntelliJ Detekt plugin, and show issues in the IJ editor. This applies trivial fixes from the rules, leaving difficult ones for a later time (suppressed and filed tech debt issues to fix them). Notable changes from applying the rules: * Fixed a bug in `BasicLazyTree` where a derived state was not remembered. * Fixed a bug in `ButtonImpl` where the `onStateChange`, accessed in a `LaunchedEffect`, was not used as a key to the effect. We have recently fixed a similar bug in JEWEL-868. * Fixed the same kind of bug in `ListComboBox`. * Fixed the same kind of bug in `CircularProgressIndicator`. * Fixed several missing remember keys in `PopupMenu`'s `MenuContent`. * Fixed a bug in `ContextMenu` where the focus manager and input mode manager were not remembered. * Deprecated several mistakenly public APIs in Menu.kt, and made them to delegate to a now-private implementation, so the public one can be safely removed in the future. * Made sure there is no indication set by default, instead of the stock one provided by Compose. * Adjusted the signature of many internal composables. * Improved the SLC and tree samples in the showcase. * Added a simpler variant to Modifier.outline(). This also adds an artifact so that we get a JAR for our own Detekt rules plugin when the IDE is built from JPS.
1 parent 06f7f5c commit f87690a

File tree

6 files changed

+53
-37
lines changed

6 files changed

+53
-37
lines changed

‎platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/DefaultBanner.kt

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@ import androidx.compose.ui.text.style.TextOverflow
2121
import androidx.compose.ui.unit.dp
2222
import org.jetbrains.jewel.foundation.theme.JewelTheme
2323
import org.jetbrains.jewel.ui.Orientation
24-
import org.jetbrains.jewel.ui.component.banner.BannerActionsRow
24+
import org.jetbrains.jewel.ui.component.banner.BannerActionsRowImpl
2525
import org.jetbrains.jewel.ui.component.banner.BannerIconActionScope
26-
import org.jetbrains.jewel.ui.component.banner.BannerIconActionsRow
26+
import org.jetbrains.jewel.ui.component.banner.BannerIconActionsRowImpl
2727
import org.jetbrains.jewel.ui.component.banner.BannerLinkActionScope
2828
import org.jetbrains.jewel.ui.component.styling.DefaultBannerStyle
2929
import org.jetbrains.jewel.ui.icons.AllIconsKeys
@@ -879,8 +879,8 @@ private fun buildDefaultBannerActions(
879879
): (@Composable RowScope.() -> Unit)? =
880880
if (linkActions != null || iconActions != null) {
881881
{
882-
BannerActionsRow(8.dp, block = linkActions)
883-
BannerIconActionsRow(iconActions)
882+
BannerActionsRowImpl(8.dp, block = linkActions)
883+
BannerIconActionsRowImpl(iconActions)
884884
}
885885
} else {
886886
null

‎platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/InlineBanner.kt

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,9 @@ import androidx.compose.ui.unit.dp
3636
import org.jetbrains.jewel.foundation.modifier.thenIf
3737
import org.jetbrains.jewel.foundation.theme.JewelTheme
3838
import org.jetbrains.jewel.foundation.theme.LocalContentColor
39-
import org.jetbrains.jewel.ui.component.banner.BannerActionsRow
39+
import org.jetbrains.jewel.ui.component.banner.BannerActionsRowImpl
4040
import org.jetbrains.jewel.ui.component.banner.BannerIconActionScope
41-
import org.jetbrains.jewel.ui.component.banner.BannerIconActionsRow
41+
import org.jetbrains.jewel.ui.component.banner.BannerIconActionsRowImpl
4242
import org.jetbrains.jewel.ui.component.banner.BannerLinkActionScope
4343
import org.jetbrains.jewel.ui.component.styling.InlineBannerStyle
4444
import org.jetbrains.jewel.ui.icons.AllIconsKeys
@@ -1187,13 +1187,13 @@ private fun InlineBannerImpl(
11871187
content = content,
11881188
actions =
11891189
if (linkActions != null) {
1190-
{ BannerActionsRow(16.dp, block = linkActions) }
1190+
{ BannerActionsRowImpl(16.dp, block = linkActions) }
11911191
} else {
11921192
null
11931193
},
11941194
actionIcons =
11951195
if (iconActions != null) {
1196-
{ BannerIconActionsRow(iconActions) }
1196+
{ BannerIconActionsRowImpl(iconActions) }
11971197
} else {
11981198
null
11991199
},

‎platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/RadioButton.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ public fun RadioButtonRow(
151151
style = style,
152152
textStyle = textStyle,
153153
verticalAlignment = verticalAlignment,
154-
{ Text(text) },
154+
content = { Text(text) },
155155
modifier = modifier,
156156
)
157157
}

‎platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/ScrollableContainer.kt

Lines changed: 21 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -103,12 +103,9 @@ public fun VerticallyScrollableContainer(
103103
horizontalScrollbar = null,
104104
scrollbarStyle = style,
105105
modifier = modifier.withKeepVisible(style.scrollbarVisibility.lingerDuration, scope) { keepVisible = it },
106-
{
107-
Box(Modifier.layoutId(ID_CONTENT).verticalScroll(scrollState, reverseScrolling = reverseLayout)) {
108-
content()
109-
}
110-
},
111-
)
106+
) {
107+
Box(Modifier.layoutId(ID_CONTENT).verticalScroll(scrollState, reverseScrolling = reverseLayout)) { content() }
108+
}
112109
}
113110

114111
@Suppress("ModifierNaming")
@@ -134,8 +131,9 @@ internal fun TextAreaScrollableContainer(
134131
horizontalScrollbar = null,
135132
scrollbarStyle = style,
136133
modifier = Modifier.withKeepVisible(style.scrollbarVisibility.lingerDuration, scope) { keepVisible = it },
137-
{ Box(contentModifier.layoutId(ID_CONTENT)) { content() } },
138-
)
134+
) {
135+
Box(contentModifier.layoutId(ID_CONTENT)) { content() }
136+
}
139137
}
140138

141139
/**
@@ -193,8 +191,9 @@ public fun VerticallyScrollableContainer(
193191
horizontalScrollbar = null,
194192
scrollbarStyle = style,
195193
modifier = modifier.withKeepVisible(style.scrollbarVisibility.lingerDuration, scope) { keepVisible = it },
196-
{ Box(Modifier.layoutId(ID_CONTENT)) { content() } },
197-
)
194+
) {
195+
Box(Modifier.layoutId(ID_CONTENT)) { content() }
196+
}
198197
}
199198

200199
/**
@@ -252,8 +251,9 @@ public fun VerticallyScrollableContainer(
252251
horizontalScrollbar = null,
253252
scrollbarStyle = style,
254253
modifier = modifier.withKeepVisible(style.scrollbarVisibility.lingerDuration, scope) { keepVisible = it },
255-
{ Box(Modifier.layoutId(ID_CONTENT)) { content() } },
256-
)
254+
) {
255+
Box(Modifier.layoutId(ID_CONTENT)) { content() }
256+
}
257257
}
258258

259259
/**
@@ -310,12 +310,9 @@ public fun HorizontallyScrollableContainer(
310310
},
311311
scrollbarStyle = style,
312312
modifier = modifier.withKeepVisible(style.scrollbarVisibility.lingerDuration, scope) { keepVisible = it },
313-
{
314-
Box(Modifier.layoutId(ID_CONTENT).horizontalScroll(scrollState, reverseScrolling = reverseLayout)) {
315-
content()
316-
}
317-
},
318-
)
313+
) {
314+
Box(Modifier.layoutId(ID_CONTENT).horizontalScroll(scrollState, reverseScrolling = reverseLayout)) { content() }
315+
}
319316
}
320317

321318
/**
@@ -373,8 +370,9 @@ public fun HorizontallyScrollableContainer(
373370
},
374371
scrollbarStyle = style,
375372
modifier = modifier.withKeepVisible(style.scrollbarVisibility.lingerDuration, scope) { keepVisible = it },
376-
{ Box(Modifier.layoutId(ID_CONTENT)) { content() } },
377-
)
373+
) {
374+
Box(Modifier.layoutId(ID_CONTENT)) { content() }
375+
}
378376
}
379377

380378
/**
@@ -432,8 +430,9 @@ public fun HorizontallyScrollableContainer(
432430
},
433431
scrollbarStyle = style,
434432
modifier = modifier.withKeepVisible(style.scrollbarVisibility.lingerDuration, scope) { keepVisible = it },
435-
{ Box(Modifier.layoutId(ID_CONTENT)) { content() } },
436-
)
433+
) {
434+
Box(Modifier.layoutId(ID_CONTENT)) { content() }
435+
}
437436
}
438437

439438
private fun Modifier.withKeepVisible(

‎platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/banner/BannerIconActionScope.kt

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package org.jetbrains.jewel.ui.component.banner
22

3+
import androidx.compose.foundation.layout.RowScope
34
import androidx.compose.runtime.Composable
45
import androidx.compose.runtime.derivedStateOf
56
import androidx.compose.runtime.getValue
@@ -23,8 +24,15 @@ public interface BannerIconActionScope {
2324
public fun iconAction(icon: IconKey, contentDescription: String?, tooltipText: String? = null, onClick: () -> Unit)
2425
}
2526

27+
@Suppress("UnusedReceiverParameter")
28+
@Deprecated("Use BannerIconActionsRowImpl.")
2629
@Composable
27-
internal fun BannerIconActionsRow(block: (BannerIconActionScope.() -> Unit)?) {
30+
internal fun RowScope.BannerIconActionsRow(block: (BannerIconActionScope.() -> Unit)?) {
31+
BannerIconActionsRowImpl(block)
32+
}
33+
34+
@Composable
35+
internal fun BannerIconActionsRowImpl(block: (BannerIconActionScope.() -> Unit)?) {
2836
val allActions by remember { derivedStateOf { block?.toList().orEmpty() } }
2937
for (iconAction in allActions) {
3038
if (!iconAction.tooltipText.isNullOrBlank()) {

‎platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/banner/BannerLinkActionScope.kt

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package org.jetbrains.jewel.ui.component.banner
22

3+
import androidx.compose.foundation.layout.RowScope
34
import androidx.compose.runtime.Composable
45
import androidx.compose.runtime.derivedStateOf
56
import androidx.compose.runtime.getValue
@@ -34,21 +35,28 @@ public interface BannerLinkActionScope {
3435
public fun action(text: String, onClick: () -> Unit)
3536
}
3637

38+
@Suppress("UnusedReceiverParameter")
39+
@Deprecated("Use BannerActionsRowImpl.")
3740
@Composable
38-
internal fun BannerActionsRow(
41+
internal fun RowScope.BannerActionsRow(
3942
spaceBetweenItems: Dp,
40-
maxVisibleItems: Int = BANNER_MAX_LINK_ACTIONS,
43+
@Suppress("UnusedParameter", "RedundantSuppression") maxVisibleItems: Int = BANNER_MAX_LINK_ACTIONS,
4144
block: (BannerLinkActionScope.() -> Unit)?,
4245
) {
43-
var visibleItems by remember { mutableIntStateOf(maxVisibleItems) }
46+
BannerActionsRowImpl(spaceBetweenItems, block)
47+
}
48+
49+
@Composable
50+
internal fun BannerActionsRowImpl(spaceBetweenItems: Dp, block: (BannerLinkActionScope.() -> Unit)?) {
51+
var visibleItems by remember { mutableIntStateOf(BANNER_MAX_LINK_ACTIONS) }
4452
val allActions by remember { derivedStateOf { block?.toList().orEmpty() } }
4553

4654
val messageProvider = LocalMessageResourceResolverProvider.current
4755

4856
Layout(
4957
content = {
5058
// Only taking the first few items to display directly
51-
for (action in allActions.take(maxVisibleItems + 1)) {
59+
for (action in allActions.take(BANNER_MAX_LINK_ACTIONS + 1)) {
5260
Link(action.text, action.onClick)
5361
}
5462

@@ -58,7 +66,8 @@ internal fun BannerActionsRow(
5866
}
5967
}
6068
},
61-
measurePolicy = BannerActionLayoutMeasurePolicy(spaceBetweenItems, maxVisibleItems) { visibleItems = it },
69+
measurePolicy =
70+
BannerActionLayoutMeasurePolicy(spaceBetweenItems, BANNER_MAX_LINK_ACTIONS) { visibleItems = it },
6271
modifier = Modifier.semantics { isTraversalGroup = true },
6372
)
6473
}

0 commit comments

Comments
 (0)