[WIP] [EPIC] Migrate plugin to Jewel #805
Conversation
WalkthroughBuild configuration: adds JetBrains Maven repositories, updates IntelliJ target to 2025.2.4, adds composeUI dependency, and bumps Compose to 1.9.0. Plugin manifest and Gradle DSL updated (plugin dependencies consolidated; ideaVersion.sinceBuild → "252"). New Gradle extension ModuleDependency.excludeCompose added. Large UI refactor: migrates many screens to Jewel-based components (Toolbar, Title, InfoCard, preview tooling) and introduces numerous reusable settings widgets (CheckboxSettingsRow, DropdownSettingsRow, RadioButtonGroup, PreviewBgSection, PreviewTheme, CodeEditor, banner system). Multiple legacy UI files and icon/vector resources removed or renamed; settings screens reorganized and localization keys expanded. Public API: rememberCodeHighlight moved into highlights-core and related API entries updated. Possibly related issues
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/action/dialog/RequiredIconPackModeDialog.kt (1)
39-46: ThefocusOnClickInsideparameter does not exist in the Jewel bridgecompose()API.This code will not compile. The
focusOnClickInsideparameter is not part of theorg.jetbrains.jewel.bridge.composefunction signature. Jewel's bridge has been moved to intellij-community and does not expose this parameter. If you need to control focus behavior on clicks, use Compose Multiplatform'sisClearFocusOnMouseDownEnabledproperty on the composition host instead, or remove the unsupported parameter.
🤖 Fix all issues with AI agents
In @gradle/libs.versions.toml:
- Line 2: The Compose version was bumped to "1.9.0" but the Tiamat artifact
"2.1.0" does not exist; either point Tiamat to the actual published artifact
"2.1.0-1.8.2" (and keep Compose at 1.8.2) or if you must keep compose = "1.9.0"
replace the Tiamat dependency with a published artifact compatible with Compose
1.9.0 (e.g., use Leviathan 3.1.0 unsuffixed) — update the version string(s)
accordingly where compose = "1.9.0" and any tiamat/leviathan version
declarations are defined.
🧹 Nitpick comments (6)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/platform/Browser.kt (1)
7-7: Consider enhancing the deprecation message for better developer guidance.The deprecation annotation could be more helpful by specifying which Jewel API should be used instead and potentially including a
ReplaceWithparameter to assist with automated migration.💡 Example enhancement
-@Deprecated("use jewel api") +@Deprecated( + message = "Use Jewel's built-in browser utilities instead", + replaceWith = ReplaceWith("jewel.browser.openInBrowser(url)", imports = ["org.jetbrains.jewel.browser.openInBrowser"]), + level = DeprecationLevel.WARNING +)Note: Adjust the
ReplaceWithsuggestion to match the actual Jewel API once identified.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
27-33: Consider extracting PreviewWrapper to eliminate duplication.The
PreviewWrapperfunction is duplicated between this file andTheme.kt(lines 135-141). Consider extracting it to a shared location to follow DRY principles.♻️ Consolidate PreviewWrapper into a single location
Since
PreviewWrapperis identical in both files, you could keep it in this file and haveTheme.ktimport it from here, or extract it to a separate shared utilities file.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/CheckboxRow.kt (1)
18-42: LGTM! Consider extracting the magic number padding.The
CheckboxRowwrapper cleanly extends Jewel's component with optional info text. The 29.dp padding at Line 37 appears to align the info text with the checkbox content, but extracting it as a named constant would improve maintainability.♻️ Extract padding constant
+private val CHECKBOX_INFO_TEXT_START_PADDING = 29.dp + @Composable fun CheckboxRow( @Nls text: String, checked: Boolean, onCheckedChange: (Boolean) -> Unit, modifier: Modifier = Modifier, infoText: String? = null, ) { Column( modifier = modifier, verticalArrangement = Arrangement.spacedBy(4.dp), ) { CheckboxRow( text = text, checked = checked, onCheckedChange = onCheckedChange, ) infoText?.let { InfoText( - modifier = Modifier.padding(start = 29.dp), + modifier = Modifier.padding(start = CHECKBOX_INFO_TEXT_START_PADDING), text = infoText, ) } } }tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kt (1)
51-74: Improve the deprecation message for clarity.The current deprecation message
"use jewel"is vague and doesn't provide actionable guidance for developers. Consider updating it to specify the replacement and migration path.💡 Suggested improvement
-@Deprecated("use jewel") +@Deprecated( + message = "Use PreviewTheme from io.github.composegears.valkyrie.uikit.tooling instead", + replaceWith = ReplaceWith( + "PreviewTheme(modifier, alignment, content)", + "io.github.composegears.valkyrie.uikit.tooling.PreviewTheme" + ) +) @Composable fun PreviewTheme(compose/core/src/commonMain/kotlin/io/github/composegears/valkyrie/compose/core/layout/Spacer.kt (1)
23-29: Code duplication: VSpacer is identical to VerticalSpacer.The new
VSpacerfunction duplicates the existingVerticalSpacerimplementation. IfVSpaceris intended as a convenience alias, consider having it delegate toVerticalSpacerto avoid duplication.♻️ Suggested refactor to eliminate duplication
@Composable inline fun ColumnScope.VSpacer( dp: Dp, modifier: Modifier = Modifier, -) { - Spacer(modifier = modifier.height(dp)) -} +) = VerticalSpacer(dp, modifier)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/PreviewBgSection.kt (1)
113-194: Consider extracting magic numbers as constants.The
AutoItemcomposable has several hardcoded values (15ffor squareSize,-2offset in loops). While the drawing logic is correct, extracting these as named constants would improve readability and maintainability.♻️ Optional: Extract magic numbers
+private const val AUTO_ITEM_SQUARE_SIZE = 15f +private const val AUTO_ITEM_LOOP_OFFSET = 2 @Composable private fun AutoItem( label: String, selected: Boolean, onSelect: () -> Unit, ) { PreviewItem( modifier = Modifier .clip(RoundedCornerShape(8.dp)) .drawBehind { // ... path drawing code ... clipPath(...) { val canvasWidth = size.width val canvasHeight = size.height - val squareSize = 15f + val squareSize = AUTO_ITEM_SQUARE_SIZE val verticalSquares = (canvasHeight / squareSize).roundToInt() val horizontalSquares = (canvasWidth / squareSize).roundToInt() // ... - for (i in -horizontalSquares / 2 - 2..horizontalSquares / 2) { - for (j in -verticalSquares / 2 - 2..verticalSquares / 2) { + for (i in -horizontalSquares / 2 - AUTO_ITEM_LOOP_OFFSET..horizontalSquares / 2) { + for (j in -verticalSquares / 2 - AUTO_ITEM_LOOP_OFFSET..verticalSquares / 2) {
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
build.gradle.ktscompose/core/src/commonMain/kotlin/io/github/composegears/valkyrie/compose/core/layout/Spacer.ktgradle/libs.versions.tomlsettings.gradle.ktstools/idea-plugin/build.gradle.ktstools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ValkyrieToolWindow.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/action/dialog/RequiredIconPackModeDialog.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/editor/TextEditorWithImageVectorPreview.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/ScrollableTabRow.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/CompositionLocal.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/platform/Browser.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/platform/DragAndDropHandler.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/platform/JComponent.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/SettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/AboutSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/GeneralSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/GeneratorSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/IndentSizeSection.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/PreviewAnnotationSection.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ImageVectorPreviewSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/PreviewBgSection.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/CheckboxRow.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/Group.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/HorizontalDivider.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kttools/idea-plugin/src/main/resources/META-INF/plugin.xmltools/idea-plugin/src/main/resources/messages/Valkyrie.properties
💤 Files with no reviewable changes (3)
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/platform/JComponent.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/ScrollableTabRow.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/CompositionLocal.kt
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 801
File: gradle/libs.versions.toml:14-14
Timestamp: 2026-01-01T18:09:41.901Z
Learning: In the Valkyrie project (ComposeGears/Valkyrie), compose-ui-tooling-preview must use version 1.10.0 (even though runtime Compose is 1.8.2) because 1.10.0+ provides unified KMP preview annotation support required for IntelliJ IDEA 2025.3+ preview functionality.
📚 Learning: 2026-01-01T18:09:41.901Z
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 801
File: gradle/libs.versions.toml:14-14
Timestamp: 2026-01-01T18:09:41.901Z
Learning: In the Valkyrie project (ComposeGears/Valkyrie), compose-ui-tooling-preview must use version 1.10.0 (even though runtime Compose is 1.8.2) because 1.10.0+ provides unified KMP preview annotation support required for IntelliJ IDEA 2025.3+ preview functionality.
Applied to files:
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/GeneralSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/PreviewAnnotationSection.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ValkyrieToolWindow.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/IndentSizeSection.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ImageVectorPreviewSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/action/dialog/RequiredIconPackModeDialog.kttools/idea-plugin/build.gradle.ktstools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/GeneratorSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/editor/TextEditorWithImageVectorPreview.ktbuild.gradle.kts
📚 Learning: 2026-01-01T18:09:32.917Z
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 801
File: gradle/libs.versions.toml:14-14
Timestamp: 2026-01-01T18:09:32.917Z
Learning: In the Valkyrie project (ComposeGears/Valkyrie), pin compose-ui-tooling-preview to version 1.10.0 in gradle/libs.versions.toml even if runtime Compose is 1.8.2. This is required because 1.10.0+ provides unified KMP preview annotation support needed for IntelliJ IDEA 2025.3+ preview functionality. Update the entry for compose-ui-tooling-preview to 1.10.0 and add a brief note in repository docs explaining the rationale.
Applied to files:
gradle/libs.versions.toml
📚 Learning: 2025-12-07T20:07:49.753Z
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 750
File: tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/task/GenerateImageVectorsTask.kt:71-85
Timestamp: 2025-12-07T20:07:49.753Z
Learning: In the Valkyrie Gradle plugin (Kotlin), the `useFlatPackage` flag in `IconPackExtension` is only applicable when nested packs are configured. For single icon packs (without nested packs), the flag is intentionally not propagated to `ImageVectorGeneratorConfig` as there is no package hierarchy to flatten.
Applied to files:
tools/idea-plugin/src/main/resources/META-INF/plugin.xmltools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ImageVectorPreviewSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/action/dialog/RequiredIconPackModeDialog.kt
📚 Learning: 2025-10-21T20:55:27.073Z
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 651
File: tools/idea-plugin/build.gradle.kts:147-175
Timestamp: 2025-10-21T20:55:27.073Z
Learning: In Gradle Kotlin DSL (.gradle.kts) scripts, the types `org.gradle.api.artifacts.ArtifactCollection` and `org.gradle.api.artifacts.component.ModuleComponentIdentifier` are implicitly available and do not require explicit import statements.
Applied to files:
settings.gradle.ktstools/idea-plugin/build.gradle.kts
🧬 Code graph analysis (9)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/PreviewAnnotationSection.kt (2)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kt (1)
PreviewTheme(51-74)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/IndentSizeSection.kt (2)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kt (1)
PreviewTheme(51-74)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ImageVectorPreviewSettingsScreen.kt (5)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/Group.kt (1)
Group(21-39)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (1)
stringResource(14-18)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/CheckboxRow.kt (1)
CheckboxRow(18-42)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/PreviewBgSection.kt (1)
PreviewBgSection(44-87)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/PreviewBgSection.kt (5)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/Group.kt (1)
Group(21-39)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (1)
stringResource(14-18)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/previewbg/PreviewBackground.kt (1)
PreviewBackground(17-47)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kt (1)
PreviewTheme(51-74)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/Group.kt (4)
compose/core/src/commonMain/kotlin/io/github/composegears/valkyrie/compose/core/layout/Spacer.kt (1)
VSpacer(23-29)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kt (1)
PreviewTheme(51-74)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/CheckboxRow.kt (1)
CheckboxRow(18-42)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/GeneratorSettingsScreen.kt (2)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kt (1)
PreviewTheme(51-74)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/editor/TextEditorWithImageVectorPreview.kt (1)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kt (2)
ValkyrieTheme(21-34)ValkyrieTheme(36-49)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/SettingsScreen.kt (3)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/HorizontalDivider.kt (1)
HorizontalDivider(9-15)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kt (1)
PreviewTheme(51-74)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/CheckboxRow.kt (2)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kt (1)
PreviewTheme(51-74)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (ubuntu-latest)
🔇 Additional comments (38)
tools/idea-plugin/src/main/resources/messages/Valkyrie.properties (1)
43-52: New localization keys align well with PR objectives.The eight new entries follow established naming conventions and provide clear, descriptive strings for the new IDE preview and background settings UI. Key structure (
settings.imagevector.preview.{ide|background}.*) logically extends the existing hierarchy.Minor observation: Line 47 is the only description string in the file using
\nfor line wrapping. While this is valid properties file syntax and may be intentional for longer descriptions, please confirm this aligns with your UI rendering expectations and verify all eight new keys are referenced in the updated settings UI code.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (2)
12-12: LGTM! Clean refactoring of bundle constant.The rename from
BUNDLE_NAMEtoBUNDLEis applied consistently across all references, and the annotation placement on line 16 now follows standard Kotlin conventions.Also applies to: 16-16, 20-20, 23-23
17-17: Verify that preview mode works correctly after simplification.The removal of
LocalInspectionModebypass logic meansstringResourcenow always delegates to the resource bundle, even in preview contexts. Please confirm that string resources load correctly in IntelliJ's preview/inspection mode.tools/idea-plugin/src/main/resources/META-INF/plugin.xml (1)
29-32: LGTM - Dependency consolidation for Jewel migration.The dependency declarations have been properly consolidated into a
<dependencies>block using the plugin-based format. The addition ofcom.intellij.modules.composealigns with the Jewel UI migration objectives.settings.gradle.kts (1)
42-44: LGTM - Standard IntelliJ Platform repositories.The added Maven repositories are standard IntelliJ Platform artifact sources required for IntelliJ IDEA 2025.2.4 and related tooling dependencies.
build.gradle.kts (3)
35-37: LGTM - IntelliJ Platform repository additions.These repositories align with the corresponding additions in
settings.gradle.ktsand are required for IntelliJ Platform 2025.2.4 dependencies.
47-47: No edition switch; this is a migration to the new IntelliJ Platform Gradle Plugin API.The change from
intellijIdeaCommunity()tointellijIdea()is not a switch to Ultimate Edition. TheintellijIdea()function is the new unified helper in the modern IntelliJ Platform Gradle Plugin that replaced both the oldintellijIdeaCommunity()andintellijIdeaUltimate()functions. It targets the unified IntelliJ IDEA distribution, and the Community Edition-specific helpers were deprecated starting with version 2025.3. This change aligns with JetBrains' current plugin development guidance and has no licensing implications—it's simply the recommended approach for new versions of the plugin.Likely an incorrect or invalid review comment.
52-52: The composeUI() setup is correctly configured.All requirements are met: intellij-platform-gradle-plugin 2.10.5 satisfies the >= 2.10.0 constraint, org.jetbrains.compose plugin is applied, and Kotlin/Compose versions are aligned. The compose-ui-tooling-preview 1.10.0 provides KMP preview support for IntelliJ IDEA 2025+. Jewel UI integration is properly in place through transitive dependencies.
tools/idea-plugin/build.gradle.kts (5)
84-84: LGTM - Plugin verifier addition.Adding
pluginVerifier()enables automated compatibility checking against target IDE versions, which is essential for maintaining plugin quality across IntelliJ IDEA releases.
114-114: Simplified IDE verification strategy.Switching from an explicit IDE list to
recommended()simplifies the verification configuration and automatically tests against JetBrains-recommended IDE versions, which is more maintainable.
63-63: Verify compose.preview replaces desktop dependencies correctly.The change from platform-specific Compose desktop dependencies to
compose.previewsimplifies the configuration. Ensure this provides the necessary preview functionality for IntelliJ IDEA 2025.2.4 and doesn't break existing preview features.What is compose.preview dependency in Jetpack Compose for IntelliJ Platform plugins and is it the correct replacement for compose.desktop dependencies in IntelliJ IDEA 2025.2.4?
17-37: Skiko exclusion is appropriate — no rendering impact.The
org.jetbrains.skikoexclusion is safe. Skiko is not used anywhere in the plugin code; it's a transitive dependency from Compose multiplatform. Since the plugin usescompose.preview(a lightweight dependency for IDE preview functionality), skiko is unnecessary and excluding it has no impact on Compose rendering functionality.The other active exclusions are also justified:
net.sourceforge.plantuml— Large dependency (~20MB) from highlights librarykotlinx-serialization-json-io— Prevents conflicts with IntelliJ's bundled libraries
97-97: sinceBuild "252" correctly targets IntelliJ IDEA 2025.2, which aligns with the platform target version 2025.2.4 specified in the rootbuild.gradle.kts(line 47:intellijIdea("2025.2.4")). The update from "243" to "252" is accurate.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/action/dialog/RequiredIconPackModeDialog.kt (1)
25-25: LGTM! Clean migration to Jewel bridge.The import change aligns with the broader migration from custom Compose panel construction to Jewel's bridge API.
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/HorizontalDivider.kt (1)
9-15: LGTM!Clean and straightforward wrapper for Jewel's horizontal divider. The implementation correctly applies
fillMaxWidth()and sets the horizontal orientation.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
12-25: LGTM!The simplified
PreviewThemeprovides a clean API for preview composables with configurable alignment and modifier support. This is a good replacement for the deprecated version with theisDarkparameter.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/PreviewAnnotationSection.kt (1)
95-95: LGTM!The visibility change from
privatetointernalfor the preview function aligns with the broader pattern across the PR, enabling better preview discovery by IntelliJ's tooling.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/CheckboxRow.kt (1)
44-55: LGTM!The preview function effectively demonstrates the
CheckboxRowcomponent with interactive state management.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/platform/DragAndDropHandler.kt (1)
24-24: This is a new file, not a migration—no action needed.
DragAndDropHandler.ktwas created as a new file during the Jewel migration (not migrated from an existing custom implementation). Jewel'sLocalComponentis designed for AWT interop and correctly provides ajava.awt.Componenttype that works withDropTarget. The code compiles successfully, confirming type compatibility.Likely an incorrect or invalid review comment.
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/GeneralSettingsScreen.kt (1)
59-59: LGTM!The import path update aligns with the migration to the new
uikit.tooling.PreviewThemeimplementation.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/IndentSizeSection.kt (1)
34-34: LGTM!The import path update and visibility change align with the broader migration pattern across preview functions.
Also applies to: 140-140
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/GeneratorSettingsScreen.kt (1)
31-31: LGTM!The import path update and visibility change are consistent with the migration pattern.
Also applies to: 101-101
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/editor/TextEditorWithImageVectorPreview.kt (1)
54-61: LGTM! Theme signature update is correct.The migration to the new
ValkyrieThemesignature (removing thecurrentComponentparameter) is correctly applied with the content passed as a trailing lambda.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/SettingsScreen.kt (2)
110-121: LGTM! Preview demonstrates the new Tabs component.The preview composable is well-structured and provides a useful development view of the Tabs component.
80-103: Jewel TabStrip API usage is correct. TheSimpleTabContentcall at line 92 properly uses thestateparameter, which receives theTabStatefrom theTabData.Defaultcontent lambda. The implementation follows the standard Jewel API pattern and all components are correctly imported fromorg.jetbrains.jewel.ui.component.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ValkyrieToolWindow.kt (2)
25-29: LGTM! Theme integration is correct.The
ValkyrieThemeusage is consistent with the new signature, and the composition structure is maintained correctly.
23-30: The removal of System.setProperty calls in ValkyrieToolWindow.kt appears safe. The Compose properties (compose.swing.render.on.graphicsandcompose.interop.blending) are only set in TextEditorWithImageVectorPreview.kt, indicating they are specific to the preview editor rendering context. ValkyrieToolWindow.kt's migration toaddComposeTab(from Jewel bridge) does not require these properties, suggesting the bridge integration handles the necessary configuration internally or the properties are only needed for the specialized preview editor use case.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/Group.kt (3)
21-39: LGTM! Group component is well-structured.The
Groupcomposable provides a clean abstraction for grouped settings with consistent padding and spacing.
41-42: LGTM! GroupSpacing provides consistent inter-group spacing.The 32.dp spacing is appropriate for separating distinct groups in a settings layout.
44-60: LGTM! Preview demonstrates the Group component effectively.The preview provides a clear example of the
Groupusage with interactive content.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/AboutSettingsScreen.kt (2)
36-71: LGTM! Clean migration to Jewel components.The refactoring simplifies the About screen by using Jewel's
ExternalLinkand typography system, making the code more maintainable and consistent with the new UI framework.
76-78: LGTM! Preview visibility update is consistent.The change to
internalvisibility aligns with the preview pattern used across other settings screens in this PR.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/PreviewBgSection.kt (4)
44-87: LGTM! Clean refactoring to use Group and SimplePreviewItem.The migration to
GroupwithstringResourcefor the header and the consolidation of individual preview items into reusableSimplePreviewItemandAutoItemcomposables improves code maintainability and reduces duplication.
89-111: Good consolidation of preview item logic.The
SimplePreviewItemcomposable cleanly encapsulates the pattern of rendering a labeled preview background option. The parameters are well-chosen for flexibility.
196-222: Clean integration with JewelTheme.The use of
JewelTheme.globalColors.outlines.focusedfor the selected border color and the consistentRoundedCornerShape(8.dp)aligns well with the Jewel UI migration.
224-233: Visibility change from private to internal is intentional for preview tooling.The change to
internalvisibility and the use ofrememberMutableStatefor interactive preview state management follows the pattern established in this PR for Jewel migration.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ImageVectorPreviewSettingsScreen.kt (2)
69-91: Preview function handles state interactively for better tooling experience.The change to
internalvisibility and the use ofrememberMutableStateenables interactive state management in previews. Thewhenexpression correctly handles the relevant action types for this screen.Minor note: The
else -> {}branch silently ignores otherSettingsActiontypes, which is appropriate for a preview function but worth noting for clarity.
38-67: Clean refactoring to Jewel UI components.The migration to
VerticallyScrollableContainer,Group,CheckboxRow, andGroupSpacingaligns with the Jewel UI patterns. The structure is clear and well-organized. The function signature has nomodifierparameter, and both call sites work seamlessly with the current implementation.
egorikftp
changed the title
egorikftp
force-pushed
the
feature/idea/jewel-migration
branch
from
c16f701 to
c5c2b04
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/SettingsScreen.kt (1)
62-70: Handle potential-1fromindexOfFirstwhen destination is null or unmatched.If
tabsDestinationisnullduring initial composition or doesn't match any tab name,indexOfFirstreturns-1, leaving no tab visually selected. Consider adding a fallback:Suggested fix
Tabs( tabNames = tabNames, - selectedTabIndex = { tabScreens.indexOfFirst { destination -> destination.name == tabsDestination?.name } }, + selectedTabIndex = { + tabScreens.indexOfFirst { destination -> destination.name == tabsDestination?.name } + .coerceAtLeast(0) + }, ) { tabIndex ->
🤖 Fix all issues with AI agents
In
@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/CheckboxRow.kt:
- Around line 17-41: The CheckboxRow composable currently calls itself causing
infinite recursion and a StackOverflowError; replace the self-call with the
actual Jewel checkbox composable (e.g., import Jewel's Checkbox as JewelCheckbox
or similar) inside CheckboxRow so it renders the real checkbox, pass through the
same parameters (text, checked, onCheckedChange) and keep the infoText branch
using InfoText with the same padding; ensure you add the appropriate import
alias for the Jewel component and remove the recursive call to CheckboxRow.
🧹 Nitpick comments (3)
build.gradle.kts (1)
46-52: Misleading comment references Android Studio instead of IntelliJ IDEA.The comment on line 46 links to the Android Studio releases list, but
intellijIdea("2025.2.4")targets IntelliJ IDEA. Consider updating the comment to reference the correct documentation.📝 Suggested fix
- // https://plugins.jetbrains.com/docs/intellij/android-studio-releases-list.html + // https://plugins.jetbrains.com/docs/intellij/tools-intellij-platform-gradle-plugin-dependencies-extension.html intellijIdea("2025.2.4")tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ImageVectorPreviewSettingsScreen.kt (1)
82-89: Consider using exhaustivewhenfor action handling.The
else -> {}branch silently ignores unhandled actions. While this is acceptable for a preview, using an exhaustivewhen(withoutelse) would catch missing handlers at compile time if new actions are added.Suggested improvement
onAction = { - when (it) { + when (it) { // Consider removing else for exhaustive matching is UpdateImageVectorPreview -> showImageVectorPreview = it.enabled is UpdateIconsInProjectView -> showIconsInProjectView = it.enabled is UpdatePreviewType -> previewType = it.previewType - else -> {} + // Add explicit handlers for other SettingsAction types if needed } },tools/idea-plugin/build.gradle.kts (1)
17-42: Consider consolidatingconfigurations.allblocks and cleaning up commented code.There are two separate
configurations.allblocks that could be merged for clarity. Additionally, the commented-out exclusions (lines 18-29) appear to be leftover from experimentation. Either remove them or add a comment explaining why they're preserved.Suggested consolidation
configurations.all { - // Exclude Compose Multiplatform dependencies not needed in IntelliJ plugin - // exclude("androidx.annotation") - // exclude("androidx.arch.core") - // exclude("androidx.compose") - // exclude("androidx.lifecycle") - // exclude("org.jetbrains.compose") - // exclude("org.jetbrains.compose.foundation") - // exclude("org.jetbrains.compose.runtime") - // exclude("org.jetbrains.compose.ui") - // exclude("org.jetbrains.jewel") - // exclude("org.jetbrains.kotlin") - // exclude("org.jetbrains.kotlinx") + // Use bundled skiko from IntelliJ to avoid version conflicts exclude("org.jetbrains.skiko") exclude(group = "net.sourceforge.plantuml") // PlantUML (~20MB) - from dev.snipme:highlights // Exclude kotlinx-serialization-json-io to prevent conflicts with IntelliJ's bundled libraries exclude(group = "org.jetbrains.kotlinx", module = "kotlinx-serialization-json-io") exclude(group = "org.jetbrains.kotlinx", module = "kotlinx-serialization-json-io-jvm") -} -configurations.all { // Exclude desktop preview exclude(group = "org.jetbrains.compose.ui", module = "ui-tooling-preview-desktop") }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
build.gradle.ktsgradle/libs.versions.tomlsettings.gradle.ktstools/idea-plugin/build.gradle.ktstools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ValkyrieToolWindow.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/action/dialog/RequiredIconPackModeDialog.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/editor/TextEditorWithImageVectorPreview.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/ScrollableTabRow.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/CompositionLocal.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/platform/Browser.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/platform/DragAndDropHandler.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/platform/JComponent.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/SettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/AboutSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/GeneralSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/GeneratorSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/IndentSizeSection.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/PreviewAnnotationSection.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ImageVectorPreviewSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/PreviewBgSection.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/CheckboxRow.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/Group.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/HorizontalDivider.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kttools/idea-plugin/src/main/resources/META-INF/plugin.xmltools/idea-plugin/src/main/resources/messages/Valkyrie.properties
💤 Files with no reviewable changes (3)
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/CompositionLocal.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/ScrollableTabRow.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/platform/JComponent.kt
🚧 Files skipped from review as they are similar to previous changes (13)
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/action/dialog/RequiredIconPackModeDialog.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/HorizontalDivider.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/PreviewAnnotationSection.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/editor/TextEditorWithImageVectorPreview.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/platform/DragAndDropHandler.kt
- tools/idea-plugin/src/main/resources/messages/Valkyrie.properties
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/Group.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/platform/Browser.kt
- gradle/libs.versions.toml
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ValkyrieToolWindow.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kt
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 801
File: gradle/libs.versions.toml:14-14
Timestamp: 2026-01-01T18:09:41.901Z
Learning: In the Valkyrie project (ComposeGears/Valkyrie), compose-ui-tooling-preview must use version 1.10.0 (even though runtime Compose is 1.8.2) because 1.10.0+ provides unified KMP preview annotation support required for IntelliJ IDEA 2025.3+ preview functionality.
📚 Learning: 2025-12-07T20:07:49.753Z
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 750
File: tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/task/GenerateImageVectorsTask.kt:71-85
Timestamp: 2025-12-07T20:07:49.753Z
Learning: In the Valkyrie Gradle plugin (Kotlin), the `useFlatPackage` flag in `IconPackExtension` is only applicable when nested packs are configured. For single icon packs (without nested packs), the flag is intentionally not propagated to `ImageVectorGeneratorConfig` as there is no package hierarchy to flatten.
Applied to files:
tools/idea-plugin/src/main/resources/META-INF/plugin.xml
📚 Learning: 2026-01-01T18:09:41.901Z
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 801
File: gradle/libs.versions.toml:14-14
Timestamp: 2026-01-01T18:09:41.901Z
Learning: In the Valkyrie project (ComposeGears/Valkyrie), compose-ui-tooling-preview must use version 1.10.0 (even though runtime Compose is 1.8.2) because 1.10.0+ provides unified KMP preview annotation support required for IntelliJ IDEA 2025.3+ preview functionality.
Applied to files:
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/GeneralSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/GeneratorSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ImageVectorPreviewSettingsScreen.kttools/idea-plugin/build.gradle.ktsbuild.gradle.ktstools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/AboutSettingsScreen.kt
📚 Learning: 2025-10-21T20:55:27.073Z
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 651
File: tools/idea-plugin/build.gradle.kts:147-175
Timestamp: 2025-10-21T20:55:27.073Z
Learning: In Gradle Kotlin DSL (.gradle.kts) scripts, the types `org.gradle.api.artifacts.ArtifactCollection` and `org.gradle.api.artifacts.component.ModuleComponentIdentifier` are implicitly available and do not require explicit import statements.
Applied to files:
tools/idea-plugin/build.gradle.kts
🧬 Code graph analysis (6)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/IndentSizeSection.kt (2)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kt (1)
PreviewTheme(51-74)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/SettingsScreen.kt (3)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/HorizontalDivider.kt (1)
HorizontalDivider(9-15)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kt (1)
PreviewTheme(51-74)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/GeneratorSettingsScreen.kt (2)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kt (1)
PreviewTheme(51-74)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/PreviewBgSection.kt (5)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/Group.kt (1)
Group(21-39)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (1)
stringResource(14-18)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/previewbg/PreviewBackground.kt (1)
PreviewBackground(17-47)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kt (1)
PreviewTheme(51-74)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/AboutSettingsScreen.kt (7)
sdk/compose/foundation/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/foundation/layout/Spacer.kt (2)
Spacer(14-17)Spacer(19-22)sdk/compose/foundation/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/foundation/layout/Row.kt (1)
CenterVerticalRow(10-20)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/intro/util/PluginVersion.kt (1)
rememberPluginVersion(9-17)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/HorizontalDivider.kt (1)
HorizontalDivider(9-15)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (1)
stringResource(14-18)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kt (1)
PreviewTheme(51-74)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/CheckboxRow.kt (2)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kt (1)
PreviewTheme(51-74)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
🔇 Additional comments (19)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/GeneralSettingsScreen.kt (1)
59-59: LGTM!The import update aligns with the Jewel migration. The old
PreviewThemeinTheme.ktis marked as deprecated with@Deprecated("use jewel"), and this correctly migrates to the new Jewel-based implementation.build.gradle.kts (1)
35-38: LGTM!The JetBrains Maven repositories are required for resolving IntelliJ platform dependencies and Jewel artifacts. These align with the repositories added in
settings.gradle.kts.settings.gradle.kts (1)
42-44: LGTM!The JetBrains repositories are correctly added to
dependencyResolutionManagement, mirroring the configuration inbuild.gradle.kts. This ensures consistent dependency resolution across the multi-project build.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/CheckboxRow.kt (1)
43-54: Preview looks correct, pending the recursion fix.Once the infinite recursion issue in the main
CheckboxRowis resolved, this preview will work correctly. The use ofrememberMutableStateandPreviewThemefollows the project's patterns.tools/idea-plugin/src/main/resources/META-INF/plugin.xml (1)
29-32: Dependency declaration correctly migrated to modern format with Compose module.The switch to the
<dependencies>block and the dependencies oncom.intellij.modules.composeandorg.jetbrains.kotlinare correct for the Jewel migration. This ensures the plugin uses the bundled Compose runtime from IntelliJ IDEA. Thecom.intellij.modules.platformmodule is not referenced anywhere in the codebase and is not required—the Compose module provides the necessary platform dependencies.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/PreviewBgSection.kt (2)
44-87: LGTM! Good consolidation of preview items.The refactor from multiple separate item composables to a unified
SimplePreviewItemreduces code duplication while maintaining the same functionality. The use ofGroupwrapper withstringResourcefor the header is consistent with the new UI kit patterns.
196-222: LGTM! JewelTheme integration looks correct.The migration from
MaterialThemetoJewelTheme.globalColors.outlines.focusedfor selection state is appropriate for the Jewel migration. The consistent use ofRoundedCornerShape(8.dp)for both border and clip aligns with the updated visual style.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ImageVectorPreviewSettingsScreen.kt (1)
43-66: LGTM! Clean migration to Jewel components.The UI structure using
VerticallyScrollableContainer,Group,CheckboxRow, andGroupSpacingaligns well with the new UI kit patterns and provides consistent styling with the Jewel framework.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/IndentSizeSection.kt (2)
34-34: LGTM! Import path and visibility updated correctly.The import path update to
io.github.composegears.valkyrie.uikit.tooling.PreviewThemeand the visibility change tointernalare consistent with the broader refactoring pattern in this PR.Also applies to: 140-140
52-92: Note: This component still usesMaterialThemestyling.The
IndentSizeSectioncomponent continues to useMaterialTheme.typographyand Material3ListItem,DropdownMenu, etc. This may be intentional for this phase of the migration, but consider whether this should be migrated to Jewel components in a follow-up.tools/idea-plugin/build.gradle.kts (1)
96-99: sinceBuild = "252" correctly targets IntelliJ 2025.2.x—no action needed.The
sinceBuild = "252"value is correct. Jewel is a transitive dependency of the IntelliJ Platform Gradle Plugin (version 2.10.5 per gradle/libs.versions.toml) and is not directly declared in dependencies. The Jewel version is automatically managed by the platform plugin itself based on compatibility. The compose-ui-tooling-preview version is correctly set to 1.10.0, which supports IntelliJ IDEA 2025.3+ preview functionality.Likely an incorrect or invalid review comment.
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/GeneratorSettingsScreen.kt (2)
31-31: LGTM - Import migrated to new Jewel-compatible PreviewTheme.The import correctly references the new
uikit.tooling.PreviewThemewhich aligns with the Jewel migration. The deprecatedfoundation.theme.PreviewThemeis being phased out as expected.
99-115: Visibility change and PreviewTheme usage look correct.The change from
privatetointernalaligns with the broader pattern across settings screens in this PR, enabling internal tooling/test access to preview functions. ThePreviewThemeusage correctly passesalignment = Alignment.TopStartmatching the new simplified API.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/AboutSettingsScreen.kt (4)
21-28: Imports correctly migrated to Jewel UI components.The import changes align with the Jewel migration objective:
HorizontalDividerandPreviewThemefrom internal uikit wrappers- Jewel components (
JewelTheme,LocalContentColor,ExternalLink,Text,typography) replace Material equivalentsThis consolidates the UI layer on JetBrains' bundled Compose/Jewel stack.
36-58: Layout and typography changes look good.The refactoring improves structure:
- Horizontal padding on the outer Column provides consistent margins
fillMaxWidth()onCenterVerticalRowensures proper layout expansionJewelTheme.typography.h3TextStyleand.smallcorrectly replace MaterialTheme typography
74-78: Preview visibility change is consistent with project patterns.The change from
privatetointernalfollows the same pattern as other settings screen previews in this PR.
62-70: ExternalLink API usage is correct.The
ExternalLinkcomponent from Jewel (bundled with IntelliJ platform) is properly used with theuriandtextparameters. Both calls in lines 62-70 are correctly configured and will handle browser navigation for the GitHub repository and issues links.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/SettingsScreen.kt (2)
110-121: LGTM!The preview properly demonstrates the
Tabscomponent with local state management and uses the newPreviewThemefrom the uikit.tooling package, consistent with the Jewel migration.
86-100: No issue withinteractionSourceusage.The single
MutableInteractionSourcepassed toTabStripis the correct pattern. Jewel'sTabStripexpects a single hoistableinteractionSourcefor the container itself, not per-tab sources. Usingremember { MutableInteractionSource() }to hoist it follows the standard Compose interaction handling pattern and allows interaction observation/coordination at the strip level.
egorikftp
force-pushed
the
feature/idea/jewel-migration
branch
from
c5c2b04 to
3b40205
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @tools/idea-plugin/build.gradle.kts:
- Around line 173-178: The custom extension function excludeCompose uses
positional exclude(...) calls which are invalid in Gradle Kotlin DSL; update the
four calls inside fun ModuleDependency.excludeCompose() to use the named
parameter form exclude(group = "org.jetbrains.compose") (and similarly for
foundation, runtime, ui) so the code compiles with the ModuleDependency.exclude
signature.
In
@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/CheckboxRow.kt:
- Around line 29-33: The local CheckboxRow function is recursively calling
itself causing infinite recursion; fix it by either importing and calling
Jewel's CheckboxRow (add an alias import like
org.jetbrains.jewel.ui.component.CheckboxRow as JewelCheckboxRow and replace the
inner call with JewelCheckboxRow(text = text, checked = checked, onCheckedChange
= onCheckedChange)) or rename this wrapper (e.g., CheckboxRowWithInfo) and
ensure the inner call targets the Jewel component; update any call-sites
accordingly so the wrapper does not shadow
org.jetbrains.jewel.ui.component.CheckboxRow.
🧹 Nitpick comments (2)
tools/idea-plugin/build.gradle.kts (1)
19-39: Consider removing commented-out exclusion rules.The commented-out exclusions add noise and reduce readability. If these were tried and rejected, consider documenting the reasoning in a brief comment instead of keeping all the dead code. If they're placeholders for future work, a TODO comment would be clearer.
♻️ Suggested cleanup
configurations.all { - // Exclude Compose Multiplatform dependencies not needed in IntelliJ plugin - // exclude("androidx.annotation") - // exclude("androidx.arch.core") - // exclude("androidx.compose") - // exclude("androidx.lifecycle") - // exclude("org.jetbrains.compose") - // exclude("org.jetbrains.compose.foundation") - // exclude("org.jetbrains.compose.runtime") - // exclude("org.jetbrains.compose.ui") - // exclude("org.jetbrains.jewel") - // exclude("org.jetbrains.kotlin") - // exclude("org.jetbrains.kotlinx") + // Use bundled Compose from IntelliJ; exclude skiko to prevent version conflicts exclude("org.jetbrains.skiko") - exclude(group = "net.sourceforge.plantuml") // PlantUML (~20MB) - from dev.snipme:highlights + // PlantUML (~20MB) - transitive from dev.snipme:highlights + exclude(group = "net.sourceforge.plantuml") // Exclude kotlinx-serialization-json-io to prevent conflicts with IntelliJ's bundled libraries exclude(group = "org.jetbrains.kotlinx", module = "kotlinx-serialization-json-io") exclude(group = "org.jetbrains.kotlinx", module = "kotlinx-serialization-json-io-jvm") }tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/platform/Browser.kt (1)
7-7: Consider a more actionable deprecation message.The message
"use jewel api"is vague. SinceExternalLinkfrom Jewel handles URL opening directly (as used inAboutSettingsScreen.kt), a more specific message would help developers migrate.Suggested improvement
-@Deprecated("use jewel api") +@Deprecated("Use org.jetbrains.jewel.ui.component.ExternalLink instead")
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
build.gradle.ktsgradle/libs.versions.tomlsettings.gradle.ktstools/idea-plugin/build.gradle.ktstools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ValkyrieToolWindow.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/action/dialog/RequiredIconPackModeDialog.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/editor/TextEditorWithImageVectorPreview.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/ScrollableTabRow.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/CompositionLocal.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/platform/Browser.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/platform/DragAndDropHandler.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/platform/JComponent.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/SettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/AboutSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/GeneralSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/GeneratorSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/IndentSizeSection.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/PreviewAnnotationSection.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ImageVectorPreviewSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/PreviewBgSection.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/CheckboxRow.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/Group.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/HorizontalDivider.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kttools/idea-plugin/src/main/resources/META-INF/plugin.xmltools/idea-plugin/src/main/resources/messages/Valkyrie.properties
💤 Files with no reviewable changes (3)
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/platform/JComponent.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/ScrollableTabRow.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/CompositionLocal.kt
🚧 Files skipped from review as they are similar to previous changes (12)
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/GeneralSettingsScreen.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/PreviewAnnotationSection.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/editor/TextEditorWithImageVectorPreview.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/Group.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/platform/DragAndDropHandler.kt
- settings.gradle.kts
- gradle/libs.versions.toml
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ValkyrieToolWindow.kt
- build.gradle.kts
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kt
- tools/idea-plugin/src/main/resources/messages/Valkyrie.properties
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 801
File: gradle/libs.versions.toml:14-14
Timestamp: 2026-01-01T18:09:41.901Z
Learning: In the Valkyrie project (ComposeGears/Valkyrie), compose-ui-tooling-preview must use version 1.10.0 (even though runtime Compose is 1.8.2) because 1.10.0+ provides unified KMP preview annotation support required for IntelliJ IDEA 2025.3+ preview functionality.
📚 Learning: 2026-01-01T18:09:41.901Z
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 801
File: gradle/libs.versions.toml:14-14
Timestamp: 2026-01-01T18:09:41.901Z
Learning: In the Valkyrie project (ComposeGears/Valkyrie), compose-ui-tooling-preview must use version 1.10.0 (even though runtime Compose is 1.8.2) because 1.10.0+ provides unified KMP preview annotation support required for IntelliJ IDEA 2025.3+ preview functionality.
Applied to files:
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ImageVectorPreviewSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/AboutSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/GeneratorSettingsScreen.kttools/idea-plugin/build.gradle.kts
📚 Learning: 2025-12-07T20:07:49.753Z
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 750
File: tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/task/GenerateImageVectorsTask.kt:71-85
Timestamp: 2025-12-07T20:07:49.753Z
Learning: In the Valkyrie Gradle plugin (Kotlin), the `useFlatPackage` flag in `IconPackExtension` is only applicable when nested packs are configured. For single icon packs (without nested packs), the flag is intentionally not propagated to `ImageVectorGeneratorConfig` as there is no package hierarchy to flatten.
Applied to files:
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ImageVectorPreviewSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/action/dialog/RequiredIconPackModeDialog.kttools/idea-plugin/src/main/resources/META-INF/plugin.xml
📚 Learning: 2025-10-21T20:55:27.073Z
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 651
File: tools/idea-plugin/build.gradle.kts:147-175
Timestamp: 2025-10-21T20:55:27.073Z
Learning: In Gradle Kotlin DSL (.gradle.kts) scripts, the types `org.gradle.api.artifacts.ArtifactCollection` and `org.gradle.api.artifacts.component.ModuleComponentIdentifier` are implicitly available and do not require explicit import statements.
Applied to files:
tools/idea-plugin/build.gradle.kts
🧬 Code graph analysis (6)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/SettingsScreen.kt (3)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/HorizontalDivider.kt (1)
HorizontalDivider(9-15)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kt (1)
PreviewTheme(51-74)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/AboutSettingsScreen.kt (4)
sdk/compose/foundation/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/foundation/layout/Row.kt (1)
CenterVerticalRow(10-20)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/HorizontalDivider.kt (1)
HorizontalDivider(9-15)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (1)
stringResource(14-18)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/GeneratorSettingsScreen.kt (2)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kt (1)
PreviewTheme(51-74)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/PreviewBgSection.kt (3)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (1)
stringResource(14-18)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/previewbg/PreviewBackground.kt (1)
PreviewBackground(17-47)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/CheckboxRow.kt (2)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kt (1)
PreviewTheme(51-74)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/IndentSizeSection.kt (2)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kt (1)
PreviewTheme(51-74)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (ubuntu-latest)
🔇 Additional comments (24)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/IndentSizeSection.kt (1)
34-34: LGTM! Import and visibility changes align with Jewel migration.The import correctly points to the new
PreviewThemeinuikit.tooling, replacing the deprecated version inui.foundation.theme. The visibility change tointernalis consistent with the broader refactor across settings tab previews.Also applies to: 140-145
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/GeneratorSettingsScreen.kt (1)
31-31: LGTM! Clean migration to new PreviewTheme.The import and visibility changes correctly migrate from the deprecated
PreviewThemeto the new Jewel-based implementation. The explicitalignment = Alignment.TopStartparameter usage is valid with the new API signature.Also applies to: 101-115
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/HorizontalDivider.kt (1)
9-15: LGTM!Clean utility wrapper around Jewel's
Divider. The modifier chain correctly appliesfillMaxWidth()after the caller-provided modifier, allowing flexibility for padding or other customizations.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/SettingsScreen.kt (3)
62-70: LGTM!The tab selection logic correctly maps destinations to indices and uses the lambda pattern to defer computation. The navigation replacement logic properly checks for actual destination changes before calling
replace.
80-103: LGTM!Good encapsulation of the Jewel
TabStripwith proper use ofTabData.Default. TheHorizontalDividerbelow the tabs provides clear visual separation.Minor note:
selectedTabIndex()is invoked once per tab inmapIndexed. For just 4 tabs this is negligible, but if the tab count grows significantly, consider caching the result before the loop.
110-121: LGTM!The preview is well-structured with
internalvisibility and correctly uses the newPreviewThemefromuikit.tooling. The interactive state viarememberMutableStateallows testing tab switching behavior.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/action/dialog/RequiredIconPackModeDialog.kt (2)
25-25: LGTM!Correct import for Jewel bridge integration. This replaces the custom
buildComposePanelwith the official Jewel API for embedding Compose content in IntelliJ components.
39-46: LGTM!The migration from
buildComposePanelto Jewel'scompose()bridge function is correct. ThefocusOnClickInside = trueparameter is appropriate for dialog panels, enabling proper focus handling when the user clicks inside the Compose content. This change addresses the core PR objective of using the bundled Compose runtime via Jewel to avoid skiko version conflicts.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/CheckboxRow.kt (1)
43-54: Preview setup looks correct.The preview follows project conventions with
internalvisibility and proper state management viarememberMutableState. It will function correctly once the recursive call issue is resolved.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (1)
12-23: LGTM!The constant rename from
BUNDLE_NAMEtoBUNDLEis consistently applied across all usages including the@PropertyKeyannotations andDynamicBundleinstantiation.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/PreviewBgSection.kt (4)
44-87: Clean migration to Jewel components.The refactoring from Column/MaterialTheme-based layout to the
Groupwrapper withstringResourceheaders and the newSimplePreviewItempattern improves consistency and aligns with the Jewel migration objectives.
89-111: Good extraction of reusable pattern.
SimplePreviewItemcleanly consolidates the common rendering logic for Black, White, and Pixel options, reducing code duplication.
196-222: Proper Jewel theming integration.The
PreviewItemcorrectly usesJewelTheme.globalColors.outlines.focusedfor the selection indicator, and theRoundedCornerShape(8.dp)is applied consistently for both border and clip.
224-233: LGTM!Preview follows project conventions with
internalvisibility and local mutable state.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ImageVectorPreviewSettingsScreen.kt (3)
21-26: LGTM!The updated imports correctly reflect the migration to the new UI kit components (
CheckboxRow,Group,GroupSpacing) and Jewel'sVerticallyScrollableContainer.
38-67: Well-structured settings UI.The layout uses
VerticallyScrollableContainer,Group, andCheckboxRowcomponents consistently, with proper i18n viastringResource. TheGroupSpacingelements provide clear visual separation.
69-91: LGTM!The preview correctly manages local state for interactive demonstration. The
else -> {}branch is acceptable here since the preview only needs to handle the actions relevant to this screen.tools/idea-plugin/build.gradle.kts (2)
97-121: Configuration updates align with Jewel migration requirements.
sinceBuild = "252"correctly targets IntelliJ 2025.2.x which supports Jewel v0.31 with Compose 1.9.0 per the compatibility mapping.- Adding
pluginVerifier()and usingrecommended()for IDE verification is good practice for catching compatibility issues early.
54-57: Correct exclusion pattern for bundled Compose usage.Excluding transitive Kotlin stdlib and Compose dependencies from
sdk.compose.foundationprevents version conflicts with the IDE's bundled libraries, which is essential for the Jewel migration.tools/idea-plugin/src/main/resources/META-INF/plugin.xml (1)
29-32: Dependency declaration correctly uses IntelliJ's bundled Compose module.The switch to the modern
<dependencies>block format with<plugin>entries aligns with IntelliJ Platform 2025.2+ conventions. Addingcom.intellij.modules.composeas a plugin dependency is correct for accessing the IDE's bundled Compose, which is essential for the Jewel migration. ThesinceBuild="252"setting in build.gradle.kts confirms compatibility with IntelliJ 2025.2.x, and the absence of bundled module declarations in Gradle appropriately relies on the platform's native Compose support.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/AboutSettingsScreen.kt (4)
21-28: Clean migration to Jewel components.The imports correctly bring in Jewel's theming and UI components. The switch from custom implementations to
ExternalLink,JewelTheme, andTextfrom Jewel aligns well with the migration objectives.
48-56: Typography migration looks correct.Using
JewelTheme.typography.h3TextStyleandJewelTheme.typography.smallproperly integrates with Jewel's theming system. TheLocalContentColor.current.dim()pattern is retained for consistent secondary text styling.
74-78: Preview visibility change is consistent with PR-wide refactoring.Changing preview functions to
internalvisibility aligns with the pattern applied across other files in this PR, as noted in the summary. This maintains encapsulation while enabling test access if needed.
62-70: ExternalLink usage simplifies navigation logic.Replacing the custom
ClickableUrlandrememberBrowser()pattern with Jewel'sExternalLinkis cleaner and aligns with the migration goals. The component handles browser opening internally. The code usesuriandtextparameters consistently across both usages, which appear correct for the Jewel component.
...plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/CheckboxSettingsRow.kt
Show resolved
Hide resolved
egorikftp
force-pushed
the
feature/idea/jewel-migration
branch
from
3b40205 to
308ee55
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/CheckboxRow.kt:
- Around line 17-41: The top-level CheckboxRow is recursively calling itself;
replace the inner recursive call with Jewel's CheckboxRow by adding the import
for org.jetbrains.jewel.components.CheckboxRow (or fully qualify it) and invoke
that component instead of the local CheckboxRow; keep the same parameters (text,
checked, onCheckedChange) and ensure the Jewel import is added so the compiler
resolves the correct CheckboxRow symbol.
🧹 Nitpick comments (5)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/platform/Browser.kt (1)
7-7: Consider enhancing the deprecation annotation withReplaceWith.The deprecation message "use jewel api" is vague. Once the replacement Jewel API is finalized, consider adding a
ReplaceWithclause and a more specific message to guide migration:@Deprecated( message = "Use Jewel's browser API instead", replaceWith = ReplaceWith("...", "org.jetbrains.jewel....") )This enables IDE-assisted migration. Given this is a WIP PR, this can be addressed once the replacement API is determined.
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
27-33: Consider consolidating duplicate PreviewWrapper.
PreviewWrapperis duplicated inTheme.kt(lines 135-141). Once the deprecatedPreviewThemeinTheme.ktis removed, consider extractingPreviewWrapperto a shared location or removing the duplicate.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/PreviewAnnotationSection.kt (1)
23-23: Import uses deprecatedPreviewTheme.This imports the deprecated
PreviewThemefromio.github.composegears.valkyrie.ui.foundation.theme, but the PR introduces a newPreviewThemeatio.github.composegears.valkyrie.uikit.tooling.PreviewTheme(as used inPreviewBgSection.ktandImageVectorPreviewSettingsScreen.kt). Consider updating the import to use the new non-deprecated version for consistency with the Jewel migration.Suggested fix
-import io.github.composegears.valkyrie.ui.foundation.theme.PreviewTheme +import io.github.composegears.valkyrie.uikit.tooling.PreviewThemetools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kt (1)
51-74: Consider enhancing@DeprecatedwithReplaceWithfor better IDE support.The deprecation message
"use jewel"is vague. Adding aReplaceWithclause would provide IDE-assisted migration to the newPreviewThemeinio.github.composegears.valkyrie.uikit.tooling.Suggested improvement
-@Deprecated("use jewel") +@Deprecated( + message = "Use PreviewTheme from uikit.tooling package instead", + replaceWith = ReplaceWith( + "PreviewTheme(modifier, alignment, content)", + "io.github.composegears.valkyrie.uikit.tooling.PreviewTheme" + ) +)Note: The new
PreviewThemedoesn't have theisDarkparameter, so callers using that parameter will need manual adjustment.tools/idea-plugin/build.gradle.kts (1)
18-38: Remove or document the commented-out exclusions.The commented-out exclusions (lines 19-30) appear to be debugging artifacts or notes. If they're intentionally kept for reference, consider adding a comment explaining why they're preserved. Otherwise, remove them to keep the build file clean.
Suggested cleanup
configurations.all { - // Exclude Compose Multiplatform dependencies not needed in IntelliJ plugin - // exclude("androidx.annotation") - // exclude("androidx.arch.core") - // exclude("androidx.compose") - // exclude("androidx.lifecycle") - // exclude("org.jetbrains.compose") - // exclude("org.jetbrains.compose.foundation") - // exclude("org.jetbrains.compose.runtime") - // exclude("org.jetbrains.compose.ui") - // exclude("org.jetbrains.jewel") - // exclude("org.jetbrains.kotlin") - // exclude("org.jetbrains.kotlinx") exclude("org.jetbrains.skiko") exclude(group = "net.sourceforge.plantuml") // PlantUML (~20MB) - from dev.snipme:highlights
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
build.gradle.ktsgradle/libs.versions.tomlsettings.gradle.ktstools/idea-plugin/build.gradle.ktstools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ValkyrieToolWindow.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/action/dialog/RequiredIconPackModeDialog.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/editor/TextEditorWithImageVectorPreview.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/ScrollableTabRow.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/CompositionLocal.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/platform/Browser.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/platform/DragAndDropHandler.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/platform/JComponent.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/SettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/AboutSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/GeneralSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/GeneratorSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/IndentSizeSection.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/PreviewAnnotationSection.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ImageVectorPreviewSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/PreviewBgSection.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/CheckboxRow.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/Group.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/HorizontalDivider.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kttools/idea-plugin/src/main/resources/META-INF/plugin.xmltools/idea-plugin/src/main/resources/messages/Valkyrie.properties
💤 Files with no reviewable changes (3)
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/CompositionLocal.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/platform/JComponent.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/ScrollableTabRow.kt
🚧 Files skipped from review as they are similar to previous changes (10)
- tools/idea-plugin/src/main/resources/messages/Valkyrie.properties
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/HorizontalDivider.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/IndentSizeSection.kt
- build.gradle.kts
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/SettingsScreen.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/GeneratorSettingsScreen.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/action/dialog/RequiredIconPackModeDialog.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ValkyrieToolWindow.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/Group.kt
- tools/idea-plugin/src/main/resources/META-INF/plugin.xml
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 801
File: gradle/libs.versions.toml:14-14
Timestamp: 2026-01-01T18:09:41.901Z
Learning: In the Valkyrie project (ComposeGears/Valkyrie), compose-ui-tooling-preview must use version 1.10.0 (even though runtime Compose is 1.8.2) because 1.10.0+ provides unified KMP preview annotation support required for IntelliJ IDEA 2025.3+ preview functionality.
📚 Learning: 2026-01-01T18:09:41.901Z
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 801
File: gradle/libs.versions.toml:14-14
Timestamp: 2026-01-01T18:09:41.901Z
Learning: In the Valkyrie project (ComposeGears/Valkyrie), compose-ui-tooling-preview must use version 1.10.0 (even though runtime Compose is 1.8.2) because 1.10.0+ provides unified KMP preview annotation support required for IntelliJ IDEA 2025.3+ preview functionality.
Applied to files:
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/editor/TextEditorWithImageVectorPreview.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/PreviewAnnotationSection.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ImageVectorPreviewSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/AboutSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/GeneralSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kttools/idea-plugin/build.gradle.kts
📚 Learning: 2025-12-07T20:07:49.753Z
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 750
File: tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/task/GenerateImageVectorsTask.kt:71-85
Timestamp: 2025-12-07T20:07:49.753Z
Learning: In the Valkyrie Gradle plugin (Kotlin), the `useFlatPackage` flag in `IconPackExtension` is only applicable when nested packs are configured. For single icon packs (without nested packs), the flag is intentionally not propagated to `ImageVectorGeneratorConfig` as there is no package hierarchy to flatten.
Applied to files:
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/editor/TextEditorWithImageVectorPreview.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ImageVectorPreviewSettingsScreen.kt
📚 Learning: 2026-01-01T18:09:32.917Z
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 801
File: gradle/libs.versions.toml:14-14
Timestamp: 2026-01-01T18:09:32.917Z
Learning: In the Valkyrie project (ComposeGears/Valkyrie), pin compose-ui-tooling-preview to version 1.10.0 in gradle/libs.versions.toml even if runtime Compose is 1.8.2. This is required because 1.10.0+ provides unified KMP preview annotation support needed for IntelliJ IDEA 2025.3+ preview functionality. Update the entry for compose-ui-tooling-preview to 1.10.0 and add a brief note in repository docs explaining the rationale.
Applied to files:
gradle/libs.versions.toml
📚 Learning: 2025-10-21T20:55:27.073Z
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 651
File: tools/idea-plugin/build.gradle.kts:147-175
Timestamp: 2025-10-21T20:55:27.073Z
Learning: In Gradle Kotlin DSL (.gradle.kts) scripts, the types `org.gradle.api.artifacts.ArtifactCollection` and `org.gradle.api.artifacts.component.ModuleComponentIdentifier` are implicitly available and do not require explicit import statements.
Applied to files:
tools/idea-plugin/build.gradle.kts
🧬 Code graph analysis (7)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/editor/TextEditorWithImageVectorPreview.kt (1)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kt (2)
ValkyrieTheme(21-34)ValkyrieTheme(36-49)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/PreviewAnnotationSection.kt (2)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kt (1)
PreviewTheme(51-74)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ImageVectorPreviewSettingsScreen.kt (5)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/Group.kt (2)
Group(21-39)GroupSpacing(41-43)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (1)
stringResource(14-18)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/CheckboxRow.kt (1)
CheckboxRow(17-41)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/PreviewBgSection.kt (1)
PreviewBgSection(44-87)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/AboutSettingsScreen.kt (7)
sdk/compose/foundation/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/foundation/layout/Spacer.kt (2)
Spacer(14-17)Spacer(19-22)sdk/compose/foundation/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/foundation/layout/Row.kt (1)
CenterVerticalRow(10-20)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/intro/util/PluginVersion.kt (1)
rememberPluginVersion(9-17)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/HorizontalDivider.kt (1)
HorizontalDivider(9-15)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (1)
stringResource(14-18)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kt (1)
PreviewTheme(51-74)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kt (1)
PreviewWrapper(136-142)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/PreviewBgSection.kt (4)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/Group.kt (1)
Group(21-39)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (1)
stringResource(14-18)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/previewbg/PreviewBackground.kt (1)
PreviewBackground(17-47)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/CheckboxRow.kt (2)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kt (1)
PreviewTheme(51-74)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
🔇 Additional comments (24)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/platform/DragAndDropHandler.kt (2)
46-70: Implementation is sound with appropriate null-safety handling.The
rememberMultiSelectDragAndDropHandlercorrectly:
- Returns early for inspection/preview mode (line 48-50)
- Uses
rememberUpdatedStateto capture the latest callback (line 53)- Properly cleans up the listener in
onDispose(line 64)Note:
LocalComponent.currentfrom Jewel returnsComponent?(nullable). WhileDropTargetaccepts a nullable component, iflocalComponentisnullat runtime, drag-and-drop silently won't function. This is acceptable behavior since no component means no drop target surface, but worth being aware of during testing.
24-24: LGTM! Import correctly updated to use Jewel'sLocalComponent.This aligns with the Jewel migration objective. Jewel's
LocalComponentprovides the AWTComponenthosting the Compose content, which is the correct type for theDropTargetconstructor. The composition hierarchy set up by Jewel'saddComposeTabbridge component automatically provides thisLocalComponentthroughout the application.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (2)
11-18: LGTM!The constant rename to
BUNDLEis cleaner, and the@PropertyKeyannotation is correctly updated to maintain IDE resource key completion support.
20-24: No breaking changes detected from the ValkyrieStrings API removal.Verification confirms that the removed
ValkyrieStringspublic API has no usages in the codebase. ThestringResourcecomposable replacement is actively used throughout the plugin in multiple files (UniversalPicker, GenericPickerScreen, EditorSelectScreen, IntroScreen, etc.), confirming a clean migration to the new API. The changes are safe to merge.gradle/libs.versions.toml (3)
2-2: LGTM! Compose version aligns with Jewel v0.31 compatibility.The update to Compose 1.9.0 is consistent with the IntelliJ 2025.2.4 → Jewel v0.31 → Compose 1.9.0 compatibility mapping documented in the PR objectives.
14-14: Intentional version mismatch for KMP preview support.The
compose-ui-tooling-previewat 1.10.0 while runtime Compose is 1.9.0 is correct. Based on learnings, version 1.10.0+ provides unified KMP preview annotation support required for IntelliJ IDEA 2025.3+ preview functionality.
8-8: Both library versions are compatible with Compose 1.9.0 and available on Maven Central.Leviathan 3.1.0 has no direct Compose dependency (only kotlin-stdlib), and tiamat 2.1.0 explicitly declares a dependency on
androidx.compose.ui:ui-backhandler:1.9.0, confirming compatibility with the current Compose version.settings.gradle.kts (1)
42-44: LGTM! Required repositories for Jewel and IntelliJ platform dependencies.The JetBrains repositories are necessary for resolving Jewel components and IntelliJ platform artifacts. The ordering (releases before snapshots) is appropriate.
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/CheckboxRow.kt (1)
43-54: Preview implementation looks correct.The preview properly uses
PreviewThemeand demonstrates the component with interactive state. Once the recursion bug is fixed, this will work as expected.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
12-25: LGTM! Simplified PreviewTheme for Jewel migration.The new
PreviewThemecorrectly removes theisDarkparameter since Jewel obtains theming from the IDE. TheLocalInspectionModeprovider ensures preview-specific behavior works correctly.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/GeneralSettingsScreen.kt (1)
59-59: LGTM! Import updated to use new PreviewTheme location.The import correctly references the new
PreviewThemefromuikit.toolingpackage, migrating away from the deprecated version inui.foundation.theme.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/PreviewAnnotationSection.kt (1)
97-106: LGTM on visibility change.The visibility change from
privatetointernalis consistent with the broader refactor across preview helpers in this PR.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/editor/TextEditorWithImageVectorPreview.kt (1)
53-63: LGTM onValkyrieThemesignature update.The change correctly adapts to the simplified
ValkyrieTheme(project)signature, removing the now-unnecessarycurrentComponentparameter.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/PreviewBgSection.kt (4)
44-87: Good refactor usingGroupandstringResource.The migration to
Groupwrapper with localized strings viastringResourceis clean and consistent with the PR's UI component standardization.
89-111: Nice extraction ofSimplePreviewItem.This helper consolidates the previously duplicated rendering logic for the Black, White, and PixelGrid items, improving maintainability.
196-222: Correct integration withJewelThemefor selection styling.Using
JewelTheme.globalColors.outlines.focusedfor the selection border aligns with the Jewel design system.
224-233: LGTM on preview function refactor.The use of
rememberMutableStatefor interactive preview state and the newPreviewThemewrapper is consistent with the migration pattern.tools/idea-plugin/build.gradle.kts (2)
173-178: Good utility for excluding Compose Multiplatform dependencies.The
excludeCompose()extension cleanly encapsulates the exclusions needed when relying on IntelliJ's bundled Compose via Jewel.
100-104:sinceBuildaligns with Jewel compatibility requirements.Setting
sinceBuild = "252"targets IntelliJ 2025.2.x, which is correct per the PR objectives for Jewel v0.28+ compatibility.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ImageVectorPreviewSettingsScreen.kt (2)
38-67: Clean migration to Jewel UI components.The refactored UI using
VerticallyScrollableContainer,Group,CheckboxRow, andGroupSpacingaligns well with the Jewel migration and improves consistency across settings screens.
69-91: LGTM on interactive preview implementation.The preview function correctly uses
rememberMutableStatefor local state management and properly handles the relevantSettingsActionvariants. Theelsebranch appropriately ignores actions not relevant to this preview context.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/AboutSettingsScreen.kt (3)
34-60: LGTM! Clean migration to Jewel theming and simplified layout.The layout refactoring is well-structured:
- Moving horizontal padding to the outer
Columnreduces redundancy.- Typography migration to
JewelTheme.typography.h3TextStyleandsmallaligns with Jewel conventions.HorizontalDividerusage is now cleaner since the uikit version handlesfillMaxWidth()internally.
74-78: Preview visibility change tointernalis intentional.The visibility change from
privatetointernalaligns with the broader migration pattern mentioned in the PR. Using the newPreviewThemefromuikit.toolingpackage (instead of the deprecated one inTheme.kt) is the correct approach for the Jewel migration.
62-70: TheExternalLinkusage is correct. The Jewel API confirms thaturiandtextare the appropriate parameter names for the component, matching the documented API pattern.
...plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/CheckboxSettingsRow.kt
Show resolved
Hide resolved
egorikftp
force-pushed
the
feature/idea/jewel-migration
branch
from
9df2012 to
f828e7f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/DropdownSettingsRow.kt`:
- Around line 82-84: Remove the unnecessary ExperimentalFoundationApi opt-in
annotation applied to the preview composable: delete the
`@OptIn`(ExperimentalFoundationApi::class) line that precedes the
`@Preview/`@Composable preview function (the Preview for DropdownSettingsRow),
leaving the `@Preview` and `@Composable` annotations intact; verify that only Jewel
APIs (e.g., Tooltip and items() from org.jetbrains.jewel.ui.component) are used
in the preview and do not require ExperimentalFoundationApi so no other changes
are needed.
🧹 Nitpick comments (12)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/ui/PreviewAnnotationSection.kt (1)
86-104: Consider adding Icon/Icons imports to the code hints for completeness.The preview hint examples omit the
IconandIconsimports, which could leave users wondering about the full setup. Since the goal is to show the annotation difference, this is acceptable, but adding brief comments or the missing imports would make the hints more self-contained.📝 Optional: Add missing imports for clarity
private val AndroidXPreviewHint = """ import androidx.compose.ui.tooling.preview.Preview + import androidx.compose.material.Icon + import androidx.compose.material.icons.Icons + import androidx.compose.material.icons.filled.MyIcon `@Preview` `@Composable` fun MyIconPreview() { Icon(imageVector = Icons.Filled.MyIcon) } """.trimIndent()tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/RadioButtonTooltipRow.kt (1)
43-54: Consider previewing both selected states.The preview only shows the unselected state. For components with binary selection states, previewing both can help visualize the full component behavior during development.
🔧 Optional enhancement to preview both states
`@Preview` `@Composable` -private fun RadioButtonTooltipRowPreview() = PreviewTheme { - RadioButtonTooltipRow( - text = "Radio option", - selected = false, - onClick = {}, - tooltipContent = { - Text(text = "Tooltip content") - }, - ) +private fun RadioButtonTooltipRowPreview() = PreviewTheme { + Column { + RadioButtonTooltipRow( + text = "Radio option (unselected)", + selected = false, + onClick = {}, + tooltipContent = { + Text(text = "Tooltip content") + }, + ) + RadioButtonTooltipRow( + text = "Radio option (selected)", + selected = true, + onClick = {}, + tooltipContent = { + Text(text = "Tooltip content") + }, + ) + } }Note: You'll need to import
androidx.compose.foundation.layout.Column.sdk/compose/highlights-core/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/highlights/core/Common.kt (1)
17-31: Potential unnecessary recompositions due to vararg array as remember key.The
emphasisLocationvararg is passed directly toremember()as a key. Since vararg creates a new array on each call, this may cause the remember block to re-execute on every recomposition even when the content hasn't changed.Consider using
contentEqualsvia a custom key or converting to a stable type:♻️ Proposed fix using contentDeepHashCode for stable key
`@Composable` fun rememberCodeHighlight( codeBlock: String, isLight: Boolean, vararg emphasisLocation: PhraseLocation, ): Highlights { - return remember(isLight, codeBlock, emphasisLocation) { + return remember(isLight, codeBlock, emphasisLocation.contentDeepHashCode()) { Highlights.Builder() .code(codeBlock) .language(SyntaxLanguage.KOTLIN) .theme(SyntaxThemes.darcula(darkMode = !isLight)) .emphasis(*emphasisLocation) .build() } }Alternatively, consider accepting
Array<PhraseLocation>instead of vararg to make the array identity explicit at call sites.build-logic/convention/src/main/kotlin/io/github/composegears/valkyrie/extension/ModuleDependency.kt (1)
6-11: Implementation looks correct for the Jewel migration.This extension correctly excludes the core JetBrains Compose modules to avoid skiko/Compose version conflicts with IntelliJ's bundled Compose. The four excluded groups (
compose,foundation,runtime,ui) are comprehensive and target the primary culprits for transitive skiko dependencies that cause crashes in issue#680.Consider adding a brief KDoc comment explaining the purpose:
📝 Suggested documentation
+/** + * Excludes JetBrains Compose transitive dependencies to avoid conflicts + * with IntelliJ's bundled Compose when using Jewel. + */ fun ModuleDependency.excludeCompose() { exclude("org.jetbrains.compose") exclude("org.jetbrains.compose.foundation") exclude("org.jetbrains.compose.runtime") exclude("org.jetbrains.compose.ui") }tools/idea-plugin/src/main/resources/messages/Valkyrie.properties (1)
89-91: Consider more descriptive accessibility labels.The current accessibility labels are quite terse (e.g., "Help", "Warning"). For better screen reader support, consider making them more descriptive and action-oriented:
accessibility.help→ "Open help documentation" or "Help button"accessibility.logo→ "Valkyrie plugin logo" or "Open plugin homepage"accessibility.warning→ "Warning icon" or "Warning message"However, if these labels are augmented with context at runtime or used in contexts where surrounding elements provide clarity, the current approach may be acceptable.
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/RadioButtonGroup.kt (1)
29-29: Consider making the colon suffix configurable.The hardcoded colon in
"$text:"may not be appropriate for all locales or use cases. If this is intentional per design guidelines, this is fine; otherwise, consider making it configurable or handling it in the localization strings.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/general/GeneralSettingsScreen.kt (1)
92-98: Consider consolidatingWebImportwithUnspecifiedor adding a distinct label.
WebImportmode shares the same "unspecified" label asUnspecified. IfWebImportis a valid user-selectable mode, it might deserve its own localized string for clarity.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/CheckboxSettingsRow.kt (1)
37-37: Magic number for InfoText padding.The
29.dpstart padding is presumably calculated to align the info text with the checkbox label. Consider extracting this as a named constant or adding a brief comment explaining the alignment calculation for maintainability.♻️ Suggested improvement
+// Aligns InfoText with checkbox label (checkbox icon width + spacing) +private val InfoTextStartPadding = 29.dp + `@Composable` fun CheckboxSettingsRow( // ... ) { // ... infoText?.let { InfoText( - modifier = Modifier.padding(start = 29.dp), + modifier = Modifier.padding(start = InfoTextStartPadding), text = infoText, ) }tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ui/PreviewBgSection.kt (1)
216-218: Consider extracting the shape to a constant.
RoundedCornerShape(8.dp)is used in multiple places (lines 121, 216, 218). Extracting it to aprivate valwould reduce duplication and ensure consistent styling.♻️ Suggested refactor
+private val PreviewItemShape = RoundedCornerShape(8.dp) + `@Composable` private fun PreviewItem( selected: Boolean, onSelect: () -> Unit, modifier: Modifier = Modifier, content: `@Composable` BoxScope.() -> Unit, ) { // ... Box( modifier = modifier .size(72.dp) .border( width = 2.dp, color = color, - shape = RoundedCornerShape(8.dp), + shape = PreviewItemShape, ) - .clip(shape = RoundedCornerShape(8.dp)) + .clip(shape = PreviewItemShape) .clickable(onClick = onSelect), content = content, ) }Then update the
AutoItemmodifier similarly.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/DropdownSettingsRow.kt (1)
56-68: Empty click handlers may confuse readers—consider adding a brief comment.The
onClickandsecondaryOnClickhandlers are empty because theOutlinedSplitButtonis used purely for its dropdown menu functionality. A brief comment explaining this intent would improve maintainability.💡 Suggested clarification
OutlinedSplitButton( modifier = Modifier.onGloballyPositioned { coordinates -> val splitButtonXInRoot = coordinates.positionInRoot().x val xInThisColumn = splitButtonXInRoot - offsetInRoot.x offset = with(density) { xInThisColumn.toDp() } }, - onClick = {}, - secondaryOnClick = {}, + // Button clicks are no-op; interaction is via the dropdown menu + onClick = {}, + secondaryOnClick = {}, content = { Text(text = buttonText) }, menuContent = menuContent, )tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/SettingsScreen.kt (1)
112-123: Preview uses hardcoded strings—consider usingmessage()for consistency.The preview uses hardcoded tab names. While acceptable for previews, using
message()would ensure the preview reflects actual localized strings and helps catch missing resource keys early.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/GeneratorSettingsScreen.kt (1)
63-148: Well-structured UI with grouped settings sections.The migration to
VerticallyScrollableContainerwithGroupcomponents provides a clean, organized layout. The code highlight tooltips using!JewelTheme.isDarkcorrectly detect the light theme.Minor observation: The
GroupSpacing()at line 147 adds spacing after the last group but before the Column closes. This may result in unnecessary bottom padding. Consider removing it if the extra spacing is unintended.♻️ Optional: Remove trailing GroupSpacing
) } - GroupSpacing() } }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
build-logic/convention/src/main/kotlin/io/github/composegears/valkyrie/extension/ModuleDependency.ktsdk/compose/codeviewer/api/codeviewer.apisdk/compose/codeviewer/api/codeviewer.klib.apisdk/compose/codeviewer/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/codeviewer/CodeViewer.ktsdk/compose/codeviewer/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/codeviewer/Common.ktsdk/compose/highlights-core/api/highlights-core.apisdk/compose/highlights-core/api/highlights-core.klib.apisdk/compose/highlights-core/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/highlights/core/Common.kttools/idea-plugin/build.gradle.ktstools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/icons/PlayForward.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/iconpack/creation/common/util/IconPackTooltipContent.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/imagevectortoxml/conversion/ui/XmlCodeViewer.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/SettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/AboutSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/GeneralSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/about/AboutSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/general/GeneralSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/GeneratorSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/IndentSizeSection.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/OutputFormatSection.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/PreviewAnnotationSection.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/ui/PreviewAnnotationSection.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/ui/SelectableCard.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/ui/SwitchOption.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ImageVectorPreviewSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ui/PreviewBgSection.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/CheckboxSettingsRow.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/DropdownSettingsRow.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/Group.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/InfoSettingsRow.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/RadioButtonGroup.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/RadioButtonTooltipRow.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/highlight/KtCodeViewer.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kttools/idea-plugin/src/main/resources/messages/Valkyrie.properties
💤 Files with no reviewable changes (11)
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/ui/SwitchOption.kt
- sdk/compose/codeviewer/api/codeviewer.api
- sdk/compose/codeviewer/api/codeviewer.klib.api
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/ui/SelectableCard.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/AboutSettingsScreen.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/PreviewAnnotationSection.kt
- sdk/compose/codeviewer/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/codeviewer/Common.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/IndentSizeSection.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/icons/PlayForward.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/OutputFormatSection.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/GeneralSettingsScreen.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- tools/idea-plugin/build.gradle.kts
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ImageVectorPreviewSettingsScreen.kt
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 801
File: gradle/libs.versions.toml:14-14
Timestamp: 2026-01-01T18:09:41.901Z
Learning: In the Valkyrie project (ComposeGears/Valkyrie), compose-ui-tooling-preview must use version 1.10.0 (even though runtime Compose is 1.8.2) because 1.10.0+ provides unified KMP preview annotation support required for IntelliJ IDEA 2025.3+ preview functionality.
📚 Learning: 2026-01-13T10:00:32.135Z
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 807
File: tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/imagevectortoxml/conversion/ui/XmlCodeViewer.kt:19-22
Timestamp: 2026-01-13T10:00:32.135Z
Learning: The Highlights library used in the codebase does not support XML syntax highlighting. When displaying XML content (e.g., in XmlCodeViewer), Kotlin syntax highlighting is used as a fallback workaround.
Applied to files:
sdk/compose/highlights-core/api/highlights-core.klib.apitools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/imagevectortoxml/conversion/ui/XmlCodeViewer.ktsdk/compose/highlights-core/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/highlights/core/Common.ktsdk/compose/highlights-core/api/highlights-core.apitools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/highlight/KtCodeViewer.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/iconpack/creation/common/util/IconPackTooltipContent.ktsdk/compose/codeviewer/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/codeviewer/CodeViewer.kt
📚 Learning: 2026-01-01T18:09:41.901Z
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 801
File: gradle/libs.versions.toml:14-14
Timestamp: 2026-01-01T18:09:41.901Z
Learning: In the Valkyrie project (ComposeGears/Valkyrie), compose-ui-tooling-preview must use version 1.10.0 (even though runtime Compose is 1.8.2) because 1.10.0+ provides unified KMP preview annotation support required for IntelliJ IDEA 2025.3+ preview functionality.
Applied to files:
sdk/compose/highlights-core/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/highlights/core/Common.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/highlight/KtCodeViewer.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/ui/PreviewAnnotationSection.ktbuild-logic/convention/src/main/kotlin/io/github/composegears/valkyrie/extension/ModuleDependency.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ui/PreviewBgSection.kt
📚 Learning: 2025-10-21T20:55:27.073Z
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 651
File: tools/idea-plugin/build.gradle.kts:147-175
Timestamp: 2025-10-21T20:55:27.073Z
Learning: In Gradle Kotlin DSL (.gradle.kts) scripts, the types `org.gradle.api.artifacts.ArtifactCollection` and `org.gradle.api.artifacts.component.ModuleComponentIdentifier` are implicitly available and do not require explicit import statements.
Applied to files:
build-logic/convention/src/main/kotlin/io/github/composegears/valkyrie/extension/ModuleDependency.kt
📚 Learning: 2025-12-07T20:07:49.753Z
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 750
File: tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/task/GenerateImageVectorsTask.kt:71-85
Timestamp: 2025-12-07T20:07:49.753Z
Learning: In the Valkyrie Gradle plugin (Kotlin), the `useFlatPackage` flag in `IconPackExtension` is only applicable when nested packs are configured. For single icon packs (without nested packs), the flag is intentionally not propagated to `ImageVectorGeneratorConfig` as there is no package hierarchy to flatten.
Applied to files:
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/iconpack/creation/common/util/IconPackTooltipContent.kt
🧬 Code graph analysis (9)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/general/GeneralSettingsScreen.kt (4)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/Group.kt (2)
Group(24-47)GroupSpacing(49-51)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (2)
stringResource(14-18)message(22-23)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/InfoSettingsRow.kt (1)
InfoSettingsRow(17-37)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/platform/CurrentProject.kt (1)
rememberCurrentProject(7-12)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/about/AboutSettingsScreen.kt (4)
sdk/compose/foundation/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/foundation/layout/Row.kt (1)
CenterVerticalRow(10-20)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (1)
stringResource(14-18)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/intro/util/PluginVersion.kt (1)
rememberPluginVersion(9-17)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/Group.kt (1)
GroupSpacing(49-51)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/highlight/KtCodeViewer.kt (2)
sdk/compose/highlights-core/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/highlights/core/Common.kt (1)
rememberCodeHighlight(17-31)sdk/compose/codeviewer/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/codeviewer/KotlinCodeViewer.kt (1)
KotlinCodeViewer(17-46)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/Group.kt (3)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kt (1)
PreviewTheme(51-74)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/CheckboxSettingsRow.kt (1)
CheckboxSettingsRow(18-42)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ui/PreviewBgSection.kt (3)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/Group.kt (1)
Group(24-47)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (1)
stringResource(14-18)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/previewbg/PreviewBackground.kt (1)
PreviewBackground(17-47)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/InfoSettingsRow.kt (1)
sdk/compose/foundation/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/foundation/layout/Row.kt (1)
CenterVerticalRow(10-20)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/SettingsScreen.kt (4)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (2)
message(22-23)stringResource(14-18)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/TopAppBar.kt (1)
AppBarTitle(50-61)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/HorizontalDivider.kt (1)
HorizontalDivider(9-15)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/RadioButtonTooltipRow.kt (2)
sdk/compose/foundation/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/foundation/layout/Row.kt (1)
CenterVerticalRow(10-20)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (1)
stringResource(14-18)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/GeneratorSettingsScreen.kt (9)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/Group.kt (2)
Group(24-47)GroupSpacing(49-51)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (1)
stringResource(14-18)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/CheckboxSettingsRow.kt (1)
CheckboxSettingsRow(18-42)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/DropdownSettingsRow.kt (1)
DropdownSettingsRow(33-80)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/RadioButtonGroup.kt (1)
RadioButtonGroup(21-38)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/highlight/KtCodeViewer.kt (1)
KtCodeViewer(14-24)sdk/compose/highlights-core/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/highlights/core/Common.kt (1)
rememberCodeHighlight(17-31)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/ui/PreviewAnnotationSection.kt (1)
PreviewAnnotationSection(30-84)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (ubuntu-latest)
🔇 Additional comments (31)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/ui/PreviewAnnotationSection.kt (2)
30-84: LGTM!The composable is well-structured with clean separation of concerns. The animation combinations (
expandVertically + fadeIn/shrinkVertically + fadeOut) are appropriate for revealing the nested options. The theme-aware code highlighting viaJewelTheme.isDarkcorrectly integrates with the Jewel theming system.
106-118: LGTM!The preview function properly demonstrates the component with interactive state management using
rememberMutableState.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/RadioButtonTooltipRow.kt (2)
1-17: Imports are well-organized and align with the Jewel migration.The imports correctly use Jewel UI components (
Icon,RadioButtonRow,Text,Tooltip) instead of custom implementations, which is consistent with the migration objectives to use the bundled Compose via Jewel.
18-41: Well-structured composable following Compose conventions.The component correctly:
- Orders parameters per Compose guidelines (required params → optional
Modifier→ trailing composable lambda).- Uses Jewel's
RadioButtonRowandTooltipcomponents for the migration.- Handles accessibility with a localized string resource for the icon's content description.
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/highlight/KtCodeViewer.kt (2)
14-24: LGTM! Clean Jewel-based code viewer implementation.The component correctly uses
JewelTheme.typography.editorTextStylefor IDE-consistent rendering, which aligns well with the Jewel migration objective.
26-46: Preview implementation looks good.The sample code and emphasis location correctly highlight the package name. The hardcoded
isLight = falseis appropriate for preview purposes.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/iconpack/creation/common/util/IconPackTooltipContent.kt (1)
8-8: Import path update aligns with module consolidation.The change correctly reflects the relocation of
rememberCodeHighlightto thehighlights-coremodule.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/imagevectortoxml/conversion/ui/XmlCodeViewer.kt (1)
11-11: Import path update consistent with module consolidation.The change aligns with the relocation of
rememberCodeHighlighttohighlights-core. Based on learnings, using Kotlin highlighting for XML content is an intentional workaround since the Highlights library doesn't support XML syntax highlighting.sdk/compose/codeviewer/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/codeviewer/CodeViewer.kt (2)
12-12: Import addition supports the preview function.The import enables
CodeTextViewPreviewto userememberCodeHighlightfrom the consolidatedhighlights-coremodule.
14-24: Good separation between Material-based and Jewel-based code viewers.This
CodeVieweruses Material's theming for general Compose usage, whileKtCodeViewerin the idea-plugin module uses Jewel's theming for IDE integration. The separation appropriately addresses different runtime contexts.sdk/compose/highlights-core/api/highlights-core.api (1)
4-4: LGTM!The new
rememberCodeHighlightcomposable function is properly exposed in the public API surface. The JVM signature correctly reflects a composable function with vararg parameter, aligning with the module restructuring to centralize code highlighting utilities inhighlights-core.sdk/compose/highlights-core/api/highlights-core.klib.api (1)
11-11: LGTM!The Kotlin multiplatform ABI dump correctly exposes
rememberCodeHighlightfor the wasmJs target. The signature is consistent with the JVM API declaration, ensuring proper cross-platform API surface alignment.tools/idea-plugin/src/main/resources/messages/Valkyrie.properties (1)
26-80: LGTM! Well-structured localization for the new Jewel-based settings UI.The new settings properties are well-organized, follow consistent naming conventions, and provide clear, user-friendly text. Notable highlights:
- The hierarchical key structure (
settings.<section>.<subsection>.<property>) is clean and maintainable.- The wording change on line 57 from "Handle" to "Use" is clearer and more direct.
- The reset dialog message (line 47) appropriately warns users about the consequences.
- MessageFormat placeholder {0} on line 38 is correctly formatted.
- Multi-line descriptions use proper escape sequences (\n).
These additions align well with the PR's objective to migrate to Jewel and provide comprehensive localization for the updated UI.
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/about/AboutSettingsScreen.kt (1)
31-73: LGTM! Clean implementation of the About settings screen.The screen properly leverages Jewel components (
VerticallyScrollableContainer,ExternalLink,Text) and follows the established UI patterns in this codebase. Good use of localized strings and the plugin version helper.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/RadioButtonGroup.kt (1)
21-38: LGTM! Well-structured reusable component.The
RadioButtonGroupprovides a clean API for grouped radio buttons with appropriate defaults. The layout with indented content (16.dp start padding) follows standard form conventions.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/general/GeneralSettingsScreen.kt (2)
47-66: LGTM! Clean navigation destination setup.The screen correctly wires the
SettingsViewModelfrom the parentnavControllerand properly handles mode changes and settings reset with navigation back to the intro screen.
68-76: @OptIn(ExperimentalJewelApi::class) correctly enables LocalComponent access for IntelliJ dialogs.The annotation is required to use
LocalComponent.current, which provides the parentComponentforMessageDialogBuilder.ask(component). This is the correct and recommended approach for displaying native dialogs from Compose UI in IntelliJ plugins.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/CheckboxSettingsRow.kt (1)
18-42: Previous infinite recursion issue is resolved.The component was renamed from
CheckboxRowtoCheckboxSettingsRow, which correctly resolves the naming collision with Jewel'sCheckboxRow. The inner call on line 30 now correctly invokesorg.jetbrains.jewel.ui.component.CheckboxRowas intended.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/Group.kt (3)
24-47: LGTM! Well-designed Group component with flexible API.The component properly wraps Jewel's
GroupHeaderand provides sensible defaults for padding and spacing. ThestartComponentslot allows for icons (like the warning icon used inGeneralSettingsScreen).
39-45: Consider the spacing accumulation.The
Spacer(8.dp)on line 43 is placed inside theColumnwithverticalArrangement = Arrangement.spacedBy(itemSpacing). This means the first content item will have both the 8.dp spacer height plus an additionalitemSpacinggap between the spacer and the first item. If this is intentional for visual balance after the header, it's fine; otherwise, consider placing the spacer outside the spaced arrangement or adjusting the logic.
49-51: Good use of suppression annotation.The
@Suppress("ktlint:compose:modifier-missing-check")is appropriate here sinceGroupSpacing()is aColumnScopeextension that returns a fixed-size spacer, where a modifier parameter would be unnecessary.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/InfoSettingsRow.kt (1)
17-37: LGTM!Clean and well-structured settings row component. Good use of
@Nlsannotations for IntelliJ's localization framework and Jewel UI components. The layout with weighted text column and optional trailing content is appropriate for settings UI patterns.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (1)
14-18: LGTM!Good API improvement. Adding
vararg paramssupport enables parameterized localized strings, which is essential for proper i18n. The simplified delegation toValkyrieBundle.messageis cleaner.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ui/PreviewBgSection.kt (1)
44-86: LGTM!Good refactoring to use the
Groupwrapper andstringResourcefor localization. TheSimplePreviewItemabstraction effectively reduces duplication compared to the previous per-option item components.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/DropdownSettingsRow.kt (1)
33-80: LGTM!Well-designed component with clever use of
onGloballyPositionedto align the info text with the button. The position calculation correctly handles the offset relative to the column.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/SettingsScreen.kt (2)
82-105: LGTM!Good encapsulation of the Jewel
TabStripinto a reusableTabscomposable. TheselectedTabIndexlambda pattern correctly defers the index read until needed, and theinteractionSourceis properly remembered.
42-51: Tab names are initialized once—acceptable for IntelliJ plugins.Using
message()insideremembercaptures the localized strings at composition time. This is fine for IntelliJ plugins where locale changes typically require a restart. If dynamic locale switching were needed, the tabs would need to be recomposed.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/GeneratorSettingsScreen.kt (4)
1-44: Imports are well-organized for the Jewel migration.The imports correctly bring in the Jewel components (
JewelTheme,Text,VerticallyScrollableContainer,items) and the new UI kit components. All imports appear to be utilized in the code.
46-54: Navigation destination setup looks correct.The ViewModel is properly scoped to the
navController(), ensuring proper lifecycle management within the navigation hierarchy.
152-172: Code hint constants are well-defined.The
BackingPropertyHintandLazyDelegatePropertyHintaccurately represent the two output format options. UsingtrimIndent()ensures clean formatting for the tooltip display.
174-212: Interactive preview implementation enables effective UI testing.The preview correctly uses
rememberMutableStateto create local state for each setting, allowing interactive testing of the UI components. TheonActionlambda appropriately handles the relevantSettingsActiontypes for preview purposes.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
...plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/DropdownSettingsRow.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/RadioButtonTooltipRow.kt`:
- Line 18: Remove the unnecessary `@OptIn`(ExperimentalFoundationApi::class)
annotation from the top of RadioButtonTooltipRow.kt; the function and its usages
(CenterVerticalRow, Spacer, RadioButtonRow, Tooltip, Icon, Text) only use stable
Compose and external Jewel APIs, so simply delete the
`@OptIn`(ExperimentalFoundationApi::class) line (and any related import of
ExperimentalFoundationApi) to clean up the file.
♻️ Duplicate comments (1)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/DropdownSettingsRow.kt (1)
82-84: Remove unnecessaryExperimentalFoundationApiopt-in from the preview function.The preview uses
items()andTooltipfromorg.jetbrains.jewel.ui.component, not fromandroidx.compose.foundation. This opt-in annotation appears unnecessary.♻️ Proposed fix
-@OptIn(ExperimentalFoundationApi::class) `@Preview` `@Composable` private fun DropdownSettingsRowPreview() = PreviewTheme {
🧹 Nitpick comments (5)
tools/idea-plugin/build.gradle.kts (1)
19-39: Use named parameter for consistency with line 34-38.Line 32 uses positional syntax
exclude("org.jetbrains.skiko")while lines 34-38 correctly useexclude(group = "..."). For consistency and clarity, use the named parameter form.Suggested fix
- exclude("org.jetbrains.skiko") + exclude(group = "org.jetbrains.skiko")tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/Group.kt (1)
39-45: Consider making inner content padding configurable or documenting the cumulative padding.The inner
Columnhas a hardcodedstart = 16.dppadding (line 40), which combines withpaddingValues(defaulthorizontal = 16.dp). This results in 32dp total start padding for content items. If this is intentional for visual hierarchy, consider documenting it; otherwise, consider deriving it frompaddingValuesor making it configurable.Also, the
Spacer(8.dp)on line 43 is placed inside aColumnwithArrangement.spacedBy(itemSpacing), so the first content item will have8.dp + itemSpacinggap from the header. Verify this is the intended behavior.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/general/GeneralSettingsScreen.kt (1)
92-98: Consider adding a dedicated localization key for WebImport mode.
WebImportis mapped to the same "Unspecified" string as the actualUnspecifiedmode. If WebImport should display a distinct label, consider adding asettings.general.mode.webimportkey.Suggested change
val name = when (currentMode) { Simple -> stringResource("settings.general.mode.simple") IconPack -> stringResource("settings.general.mode.iconpack") Editor -> stringResource("settings.general.mode.editor") ImageVectorToXml -> stringResource("settings.general.mode.imagevector.xml") - Unspecified, WebImport -> stringResource("settings.general.mode.unspecified") + WebImport -> stringResource("settings.general.mode.webimport") + Unspecified -> stringResource("settings.general.mode.unspecified") }tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/SettingsScreen.kt (1)
82-105: Consider memoizing the tabs list to avoid recreatingTabDataobjects on every recomposition.The
tabsparameter passed toTabStripis recreated viamapIndexedon every recomposition. While the overhead is minimal for 4 tabs, you could optimize by usingderivedStateOfor ensuring the list is only rebuilt whenselectedTabIndex()actually changes.Suggested optimization
`@Composable` private fun Tabs( tabNames: List<String>, selectedTabIndex: () -> Int, onTabClick: (Int) -> Unit, ) { + val currentSelectedIndex = selectedTabIndex() Column { TabStrip( tabs = tabNames.mapIndexed { index, string -> TabData.Default( - selected = index == selectedTabIndex(), + selected = index == currentSelectedIndex, content = { SimpleTabContent(label = string, state = it) }, onClick = { onTabClick(index) }, closable = false, ) }, style = JewelTheme.defaultTabStyle, interactionSource = remember { MutableInteractionSource() }, ) HorizontalDivider() } }tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ui/PreviewBgSection.kt (1)
196-222: Consider extracting the repeated shape constant.
RoundedCornerShape(8.dp)is instantiated multiple times (lines 121, 216, 218). While this isn't a correctness issue, extracting it to a private constant improves consistency and slightly reduces allocations.♻️ Suggested refactor
+private val PreviewItemShape = RoundedCornerShape(8.dp) + `@Composable` private fun PreviewItem( selected: Boolean, onSelect: () -> Unit, modifier: Modifier = Modifier, content: `@Composable` BoxScope.() -> Unit, ) { val color by animateColorAsState( when { selected -> JewelTheme.globalColors.outlines.focused else -> Color.Transparent }, ) Box( modifier = modifier .size(72.dp) .border( width = 2.dp, color = color, - shape = RoundedCornerShape(8.dp), + shape = PreviewItemShape, ) - .clip(shape = RoundedCornerShape(8.dp)) + .clip(shape = PreviewItemShape) .clickable(onClick = onSelect), content = content, ) }Also update line 121 in
AutoItem:- .clip(RoundedCornerShape(8.dp)) + .clip(PreviewItemShape)
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
build-logic/convention/src/main/kotlin/io/github/composegears/valkyrie/extension/ModuleDependency.ktsdk/compose/codeviewer/api/codeviewer.apisdk/compose/codeviewer/api/codeviewer.klib.apisdk/compose/codeviewer/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/codeviewer/CodeViewer.ktsdk/compose/codeviewer/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/codeviewer/Common.ktsdk/compose/highlights-core/api/highlights-core.apisdk/compose/highlights-core/api/highlights-core.klib.apisdk/compose/highlights-core/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/highlights/core/Common.kttools/idea-plugin/build.gradle.ktstools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/icons/PlayForward.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/iconpack/creation/common/util/IconPackTooltipContent.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/imagevectortoxml/conversion/ui/XmlCodeViewer.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/SettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/AboutSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/GeneralSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/about/AboutSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/general/GeneralSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/GeneratorSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/IndentSizeSection.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/OutputFormatSection.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/PreviewAnnotationSection.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/ui/PreviewAnnotationSection.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/ui/SelectableCard.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/ui/SwitchOption.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ImageVectorPreviewSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ui/PreviewBgSection.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/CheckboxSettingsRow.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/DropdownSettingsRow.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/Group.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/InfoSettingsRow.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/RadioButtonGroup.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/RadioButtonTooltipRow.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/highlight/KtCodeViewer.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kttools/idea-plugin/src/main/resources/messages/Valkyrie.properties
💤 Files with no reviewable changes (11)
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/GeneralSettingsScreen.kt
- sdk/compose/codeviewer/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/codeviewer/Common.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/ui/SwitchOption.kt
- sdk/compose/codeviewer/api/codeviewer.api
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/IndentSizeSection.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/PreviewAnnotationSection.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/OutputFormatSection.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/icons/PlayForward.kt
- sdk/compose/codeviewer/api/codeviewer.klib.api
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/AboutSettingsScreen.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/ui/SelectableCard.kt
🚧 Files skipped from review as they are similar to previous changes (11)
- sdk/compose/highlights-core/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/highlights/core/Common.kt
- build-logic/convention/src/main/kotlin/io/github/composegears/valkyrie/extension/ModuleDependency.kt
- sdk/compose/highlights-core/api/highlights-core.klib.api
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/highlight/KtCodeViewer.kt
- sdk/compose/highlights-core/api/highlights-core.api
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/InfoSettingsRow.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/iconpack/creation/common/util/IconPackTooltipContent.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/RadioButtonGroup.kt
- sdk/compose/codeviewer/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/codeviewer/CodeViewer.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/about/AboutSettingsScreen.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/imagevectortoxml/conversion/ui/XmlCodeViewer.kt
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 801
File: gradle/libs.versions.toml:14-14
Timestamp: 2026-01-01T18:09:41.901Z
Learning: In the Valkyrie project (ComposeGears/Valkyrie), compose-ui-tooling-preview must use version 1.10.0 (even though runtime Compose is 1.8.2) because 1.10.0+ provides unified KMP preview annotation support required for IntelliJ IDEA 2025.3+ preview functionality.
📚 Learning: 2026-01-01T18:09:41.901Z
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 801
File: gradle/libs.versions.toml:14-14
Timestamp: 2026-01-01T18:09:41.901Z
Learning: In the Valkyrie project (ComposeGears/Valkyrie), compose-ui-tooling-preview must use version 1.10.0 (even though runtime Compose is 1.8.2) because 1.10.0+ provides unified KMP preview annotation support required for IntelliJ IDEA 2025.3+ preview functionality.
Applied to files:
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ui/PreviewBgSection.kttools/idea-plugin/build.gradle.ktstools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/DropdownSettingsRow.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ImageVectorPreviewSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/ui/PreviewAnnotationSection.kt
📚 Learning: 2025-12-07T20:07:49.753Z
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 750
File: tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/task/GenerateImageVectorsTask.kt:71-85
Timestamp: 2025-12-07T20:07:49.753Z
Learning: In the Valkyrie Gradle plugin (Kotlin), the `useFlatPackage` flag in `IconPackExtension` is only applicable when nested packs are configured. For single icon packs (without nested packs), the flag is intentionally not propagated to `ImageVectorGeneratorConfig` as there is no package hierarchy to flatten.
Applied to files:
tools/idea-plugin/build.gradle.kts
📚 Learning: 2025-10-21T20:55:27.073Z
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 651
File: tools/idea-plugin/build.gradle.kts:147-175
Timestamp: 2025-10-21T20:55:27.073Z
Learning: In Gradle Kotlin DSL (.gradle.kts) scripts, the types `org.gradle.api.artifacts.ArtifactCollection` and `org.gradle.api.artifacts.component.ModuleComponentIdentifier` are implicitly available and do not require explicit import statements.
Applied to files:
tools/idea-plugin/build.gradle.kts
🧬 Code graph analysis (8)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ui/PreviewBgSection.kt (3)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/Group.kt (1)
Group(24-47)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (1)
stringResource(14-18)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/previewbg/PreviewBackground.kt (1)
PreviewBackground(17-47)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/SettingsScreen.kt (5)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (2)
message(22-23)stringResource(14-18)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/TopAppBar.kt (1)
AppBarTitle(50-61)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/HorizontalDivider.kt (1)
HorizontalDivider(9-15)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kt (1)
PreviewTheme(51-74)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
tools/idea-plugin/build.gradle.kts (1)
build-logic/convention/src/main/kotlin/io/github/composegears/valkyrie/extension/ModuleDependency.kt (1)
excludeCompose(6-11)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/DropdownSettingsRow.kt (2)
sdk/compose/foundation/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/foundation/layout/Row.kt (1)
CenterVerticalRow(10-20)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (1)
stringResource(14-18)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/Group.kt (3)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kt (1)
PreviewTheme(51-74)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/CheckboxSettingsRow.kt (1)
CheckboxSettingsRow(18-42)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ImageVectorPreviewSettingsScreen.kt (6)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/Group.kt (2)
Group(24-47)GroupSpacing(49-51)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (1)
stringResource(14-18)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/CheckboxSettingsRow.kt (1)
CheckboxSettingsRow(18-42)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ui/PreviewBgSection.kt (1)
PreviewBgSection(44-87)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)sdk/compose/foundation/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/foundation/RememberMutableState.kt (3)
rememberMutableState(11-15)rememberMutableState(17-22)rememberMutableState(24-30)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/ui/PreviewAnnotationSection.kt (6)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/CheckboxSettingsRow.kt (1)
CheckboxSettingsRow(18-42)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (1)
stringResource(14-18)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/RadioButtonGroup.kt (1)
RadioButtonGroup(21-38)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/RadioButtonTooltipRow.kt (1)
RadioButtonTooltipRow(18-41)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/highlight/KtCodeViewer.kt (1)
KtCodeViewer(14-24)sdk/compose/highlights-core/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/highlights/core/Common.kt (1)
rememberCodeHighlight(17-31)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/RadioButtonTooltipRow.kt (2)
sdk/compose/foundation/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/foundation/layout/Row.kt (1)
CenterVerticalRow(10-20)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (1)
stringResource(14-18)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (ubuntu-latest)
🔇 Additional comments (36)
tools/idea-plugin/build.gradle.kts (6)
1-4: LGTM!The imports are correctly structured. The custom
excludeComposeextension from build-logic and the IntelliJ platform'sexcludeKotlinStdlibare properly imported for use in dependency configuration.
55-58: LGTM!Correct application of
excludeKotlinStdlib()andexcludeCompose()to the foundation dependency. This prevents conflicts with IntelliJ's bundled Compose/Kotlin libraries, aligning with the Jewel migration objective.
68-69: LGTM!Adding
compose.previewalongside the existingcompileOnlyforcompose.ui.tooling.preview(line 86) is appropriate—the former provides runtime support while the latter provides compile-time annotations. Based on learnings, the separate 1.10.0 version for the preview annotation is required for IntelliJ 2025.3+ unified KMP preview support.
88-91: LGTM!Adding
pluginVerifier()enables the plugin verification infrastructure, which works with thepluginVerificationblock configuration. Good for ensuring compatibility with target IDE versions.
102-105: Verify thesinceBuildbump drops IntelliJ 2024.x support.Changing
sinceBuildfrom"243"(IntelliJ 2024.3) to"252"(IntelliJ 2025.2) is a significant compatibility change. Per the PR objectives, Jewel v0.31 requires IntelliJ 2025.2.4+, so this is necessary for the migration.Ensure this breaking change is documented in release notes so users on older IDE versions are aware they need to upgrade.
119-121: LGTM!Using
recommended()for IDE verification targets is a good simplification. It automatically selects appropriate IDE versions based onsinceBuild/untilBuild, reducing maintenance burden compared to manually specifying versions.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/RadioButtonTooltipRow.kt (1)
43-54: LGTM!The preview properly demonstrates the component with sample data and tooltip content.
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/DropdownSettingsRow.kt (2)
56-68: Verify intentional no-op click handlers onOutlinedSplitButton.Both
onClickandsecondaryOnClickare empty lambdas. If the primary button click should trigger the same action as selecting from the menu (e.g., open the dropdown), consider wiring it up. If the no-op is intentional (e.g., the button only displays the current selection), a brief comment would clarify the intent.
85-115: LGTM!The preview demonstrates the dropdown row with menu content and selection handling. Consider replacing
"kek"(line 102) with a more descriptive placeholder like"Option tooltip"for clarity.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/CheckboxSettingsRow.kt (2)
18-42: LGTM!The infinite recursion issue has been correctly resolved by importing
CheckboxRowfrom Jewel. The component properly composes the checkbox with optional info text.Consider extracting
29.dp(line 37) to a named constant for clarity (e.g.,private val CheckboxTextIndent = 29.dp), as it represents the visual alignment with the checkbox label.
44-55: LGTM!The preview correctly demonstrates the checkbox row with toggleable state.
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/Group.kt (2)
49-51: LGTM!Good utility extension for consistent inter-group spacing. The
@Suppressannotation is appropriate for this spacer helper.
53-75: LGTM!The preview effectively demonstrates the
Groupcomponent with a header icon and nestedCheckboxSettingsRow.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/ui/PreviewAnnotationSection.kt (3)
30-84: LGTM!Clean implementation of the preview annotation settings section. The animated visibility, radio button groups with tooltip hints, and code highlighting are well composed.
86-104: LGTM!The preview hint constants accurately demonstrate the import differences between AndroidX and JetBrains preview annotations.
106-118: LGTM!The preview comprehensively demonstrates the section with interactive state for both the checkbox and radio selection.
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (2)
14-18: LGTM!The
stringResourcefunction now properly supports formatted message parameters via vararg, which enables parameterized localization strings like"settings.general.current.mode"that use{0}placeholders.
20-24: LGTM!Making
ValkyrieBundlepublic is appropriate since it's now directly accessed viaValkyrieBundle.message()in non-Composable contexts (e.g.,MessageDialogBuilderinGeneralSettingsScreen.kt).tools/idea-plugin/src/main/resources/messages/Valkyrie.properties (1)
26-91: LGTM!The localization keys are well-structured with a consistent naming convention (
settings.tab.*,settings.general.*, etc.). The parameterized strings use proper{0}placeholders compatible withDynamicBundle.getMessage().tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/general/GeneralSettingsScreen.kt (2)
151-155: LGTM!The navigation helper correctly accesses the parent nav controller and replaces the entire stack with a fade transition to return to the intro screen.
102-107: Consider disabling the Link visually instead of silently ignoring clicks when mode is Unspecified.The
LinkonClick currently does nothing whenmode == Unspecified. Verify whether Jewel'sLinkcomponent supports anenabledparameter. If it does, use it instead of the conditional click handler for clearer UX:Suggested change (if enabled parameter is supported)
-Link( - text = stringResource("settings.general.current.mode.action"), - onClick = { if (mode != Unspecified) onChangeMode() }, -) +Link( + text = stringResource("settings.general.current.mode.action"), + enabled = mode != Unspecified, + onClick = onChangeMode, +)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/GeneratorSettingsScreen.kt (5)
46-54: LGTM!Clean navigation destination wiring with the ViewModel obtained from the parent navController's viewModelStoreOwner, ensuring settings state is shared across tabs.
90-121: LGTM!The
RadioButtonGroupwith tooltip code viewers provides excellent UX by showing users exactly what output format they'll get. The dark mode awareness via!JewelTheme.isDarkensures proper syntax highlighting in both themes.
152-172: LGTM!The code hint strings are well-formatted and clearly illustrate the difference between backing property and lazy delegate patterns.
174-212: LGTM!Comprehensive preview function that exercises all settings actions with local state, making it easy to verify the UI behavior in the IDE preview.
56-56: Remove the unnecessary@OptIn(ExperimentalFoundationApi::class)annotation.This file imports and applies the annotation but does not use any experimental Foundation API from
androidx.compose.foundation. All UI components come fromorg.jetbrains.jewelinstead.Likely an incorrect or invalid review comment.
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/SettingsScreen.kt (2)
39-80: LGTM!Clean migration to Jewel's tab components. The
rememberblocks correctly cache tab data, and themessage()function is appropriately used in the non-composable context withinremember.
112-123: LGTM!The preview demonstrates the
Tabscomponent in isolation with interactive state, which is useful for verifying tab styling and behavior.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ui/PreviewBgSection.kt (4)
44-86: Clean migration to Group-based layout with i18n support.The refactoring from a Column-based layout to the
Groupwrapper withstringResourcefor the header is well done. The consolidated approach withSimplePreviewItemreduces code duplication.
89-111: Good consolidation of preview item rendering.
SimplePreviewItemeffectively generalizes the previous individualBlackItem,WhiteItem, andPixelItemcomponents into a single parameterized composable, reducing duplication while maintaining clarity.
224-233: LGTM on preview setup.The preview correctly uses
PreviewThemewithrememberMutableStatefor interactive state management, following the established pattern in the codebase.
203-208: Clarify version compatibility requirements.The review references Jewel versions 0.28–0.33 compatibility, but these constraints are not documented in the repository's build configuration. Jewel is bundled with IntelliJ IDEA (plugin targets IntelliJ 252+) rather than managed as an explicit dependency. Either add version constraints to the build configuration or provide documentation explaining the intended Jewel version compatibility for this API usage.
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ImageVectorPreviewSettingsScreen.kt (4)
29-37: Proper ViewModel scoping with navController.Using
viewModelStoreOwner = navController()correctly scopes the ViewModel to the navigation graph, ensuring the settings state is shared appropriately across tabs.
44-67: Well-structured settings UI with Jewel components.The migration to
VerticallyScrollableContainer,Group,CheckboxSettingsRow, andGroupSpacingcreates a consistent and clean settings interface that aligns with JetBrains IDE conventions.
82-89: Silent else branch is acceptable for preview context.The
else -> {}branch handles otherSettingsActiontypes that this screen doesn't care about. This is acceptable for preview purposes where only a subset of actions are relevant. For production code, exhaustivewhenis preferred, but the preview function is isolated.
70-91: LGTM on preview with local state management.The preview uses
rememberMutableStatefor interactive local state, which allows proper preview interaction without depending on actual ViewModel persistence. This pattern aligns with other previews in the codebase.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
...ugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/RadioButtonTooltipRow.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/highlight/KtCodeViewer.kt`:
- Around line 29-43: The code passes an array to the vararg parameter
`emphasisLocation` of `rememberCodeHighlight` using named-argument syntax, which
won't compile; fix by either providing `PhraseLocation(...)` values directly as
vararg positional arguments after the named parameters (e.g., call
rememberCodeHighlight(..., PhraseLocation(...), PhraseLocation(...))) or
construct an array and use the spread operator when calling the vararg (e.g.,
use *arrayOf(PhraseLocation(...))) and ensure you reference the
`emphasisLocation` vararg correctly; see `rememberCodeHighlight`,
`emphasisLocation`, and `PhraseLocation` in your diff to locate the call to
update.
♻️ Duplicate comments (1)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/DropdownSettingsRow.kt (1)
82-84: ExperimentalFoundationApi opt-in is necessary.Per previous discussion, Jewel's
Tooltipcomponent requires this opt-in due to its internal dependency on experimental foundation APIs.
🧹 Nitpick comments (7)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ui/PreviewBgSection.kt (1)
196-222: Consider extracting repeated shape constant.The
RoundedCornerShape(8.dp)is used in multiple places (lines 121, 216, 218). Consider extracting it as aprivate valconstant for consistency and easier future adjustments.♻️ Suggested refactor
+private val PreviewItemShape = RoundedCornerShape(8.dp) + `@Composable` private fun PreviewItem( selected: Boolean, onSelect: () -> Unit, modifier: Modifier = Modifier, content: `@Composable` BoxScope.() -> Unit, ) { // ... Box( modifier = modifier .size(72.dp) .border( width = 2.dp, color = color, - shape = RoundedCornerShape(8.dp), + shape = PreviewItemShape, ) - .clip(shape = RoundedCornerShape(8.dp)) + .clip(shape = PreviewItemShape) .clickable(onClick = onSelect), content = content, ) }Then update line 121 in
AutoItemsimilarly.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/CheckboxSettingsRow.kt (1)
35-40: Magic number for InfoText alignment could be fragile.The
29.dppadding is presumably calculated to align the info text with the checkbox label, but this value is coupled to Jewel's internalCheckboxRowlayout. If Jewel updates its checkbox sizing, this alignment will break.Consider adding a brief comment explaining the value, or verify if Jewel exposes a constant/token for this measurement.
💡 Suggested documentation
infoText?.let { InfoText( + // 29.dp = checkbox size (24.dp) + internal spacing (5.dp) to align with label modifier = Modifier.padding(start = 29.dp), text = infoText, ) }tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/RadioButtonGroup.kt (1)
29-29: Consider i18n implications of hardcoded colon.The hardcoded colon in
"$text:"may not be appropriate for all locales (e.g., some languages use different punctuation or spacing). Consider either including the colon in the localized string itself, or making the suffix configurable.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/DropdownSettingsRow.kt (1)
54-54: Same i18n consideration for hardcoded colon.Similar to
RadioButtonGroup, the hardcoded colon in"$text:"may not suit all locales.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/general/GeneralSettingsScreen.kt (3)
77-84: Clarify the mode stabilization logic.The pattern of remembering
initialModeand falling back to it whenmodeisUnspecifiedprevents UI flickering during transitions. However, this meanscurrentModecan show a stale value if the actual mode becomesUnspecified. Consider adding a comment explaining this intentional behavior.📝 Suggested documentation
+ // Stabilize the displayed mode during navigation transitions. + // When mode becomes Unspecified (e.g., after reset), we show the + // previous mode briefly to avoid UI flicker before navigating away. val mode = generalSettings.mode val initialMode = remember { mode } val currentMode = remember(mode) {
137-140: Inconsistent string resource usage.This block uses
message()(non-composable) while the rest of the file usesstringResource()(composable). Both work, butstringResource()is the preferred pattern in Compose code for consistency.♻️ Suggested fix
val yesNo = MessageDialogBuilder.yesNo( - title = message("settings.general.reset.dialog.title"), - message = message("settings.general.reset.dialog.message"), + title = stringResource("settings.general.reset.dialog.title"), + message = stringResource("settings.general.reset.dialog.message"), ).ask(component)
151-155: Consider null safety for parent navigation.
navController.parentcould theoretically be null. While in practice this screen is always nested, adding explicit handling or an assertion would make the contract clearer.♻️ Suggested defensive handling
private fun openIntro(navController: NavController) { - navController.parent?.run { + val parentController = navController.parent + ?: error("GeneralSettingsScreen must be nested within a parent NavController") + parentController.run { editNavStack(transition = navigationFadeInOut()) { _ -> listOf(IntroScreen.toNavEntry()) } } }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/HorizontalDivider.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/highlight/KtCodeViewer.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/CheckboxSettingsRow.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/DropdownSettingsRow.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/Group.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/InfoSettingsRow.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/RadioButtonGroup.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/RadioButtonTooltipRow.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/tooling/PreviewTheme.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/SettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/about/AboutSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/general/GeneralSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/GeneratorSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/ui/PreviewAnnotationSection.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ImageVectorPreviewSettingsScreen.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ui/PreviewBgSection.kt
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 801
File: gradle/libs.versions.toml:14-14
Timestamp: 2026-01-01T18:09:41.901Z
Learning: In the Valkyrie project (ComposeGears/Valkyrie), compose-ui-tooling-preview must use version 1.10.0 (even though runtime Compose is 1.8.2) because 1.10.0+ provides unified KMP preview annotation support required for IntelliJ IDEA 2025.3+ preview functionality.
📚 Learning: 2026-01-01T18:09:41.901Z
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 801
File: gradle/libs.versions.toml:14-14
Timestamp: 2026-01-01T18:09:41.901Z
Learning: In the Valkyrie project (ComposeGears/Valkyrie), compose-ui-tooling-preview must use version 1.10.0 (even though runtime Compose is 1.8.2) because 1.10.0+ provides unified KMP preview annotation support required for IntelliJ IDEA 2025.3+ preview functionality.
Applied to files:
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/tooling/PreviewTheme.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ui/PreviewBgSection.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/DropdownSettingsRow.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/ui/PreviewAnnotationSection.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/highlight/KtCodeViewer.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ImageVectorPreviewSettingsScreen.kt
📚 Learning: 2026-01-13T10:00:32.135Z
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 807
File: tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/imagevectortoxml/conversion/ui/XmlCodeViewer.kt:19-22
Timestamp: 2026-01-13T10:00:32.135Z
Learning: The Highlights library used in the codebase does not support XML syntax highlighting. When displaying XML content (e.g., in XmlCodeViewer), Kotlin syntax highlighting is used as a fallback workaround.
Applied to files:
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/highlight/KtCodeViewer.kt
🧬 Code graph analysis (13)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/CheckboxSettingsRow.kt (1)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/SettingsScreen.kt (5)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (2)
message(22-23)stringResource(14-18)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/TopAppBar.kt (1)
AppBarTitle(50-61)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/HorizontalDivider.kt (1)
HorizontalDivider(9-15)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/theme/Theme.kt (1)
PreviewTheme(51-74)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/RadioButtonTooltipRow.kt (3)
sdk/compose/foundation/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/foundation/layout/Row.kt (1)
CenterVerticalRow(10-20)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (1)
stringResource(14-18)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/InfoSettingsRow.kt (2)
sdk/compose/foundation/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/foundation/layout/Row.kt (1)
CenterVerticalRow(10-20)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/general/GeneralSettingsScreen.kt (5)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/Group.kt (2)
Group(24-47)GroupSpacing(49-51)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (2)
stringResource(14-18)message(22-23)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/InfoSettingsRow.kt (1)
InfoSettingsRow(17-37)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/platform/CurrentProject.kt (1)
rememberCurrentProject(7-12)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/GeneratorSettingsScreen.kt (5)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/Group.kt (2)
Group(24-47)GroupSpacing(49-51)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/DropdownSettingsRow.kt (1)
DropdownSettingsRow(33-80)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/highlight/KtCodeViewer.kt (1)
KtCodeViewer(14-24)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/ui/PreviewAnnotationSection.kt (1)
PreviewAnnotationSection(30-84)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ui/PreviewBgSection.kt (4)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/Group.kt (1)
Group(24-47)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (1)
stringResource(14-18)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/previewbg/PreviewBackground.kt (1)
PreviewBackground(17-47)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/DropdownSettingsRow.kt (3)
sdk/compose/foundation/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/foundation/layout/Row.kt (1)
CenterVerticalRow(10-20)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (1)
stringResource(14-18)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/RadioButtonGroup.kt (1)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/Group.kt (2)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/CheckboxSettingsRow.kt (1)
CheckboxSettingsRow(18-42)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/about/AboutSettingsScreen.kt (6)
sdk/compose/foundation/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/foundation/layout/Row.kt (1)
CenterVerticalRow(10-20)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (1)
stringResource(14-18)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/intro/util/PluginVersion.kt (1)
rememberPluginVersion(9-17)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/HorizontalDivider.kt (1)
HorizontalDivider(9-15)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/Group.kt (1)
GroupSpacing(49-51)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/ui/PreviewAnnotationSection.kt (8)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/CheckboxSettingsRow.kt (1)
CheckboxSettingsRow(18-42)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ValkyrieStrings.kt (1)
stringResource(14-18)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/RadioButtonGroup.kt (1)
RadioButtonGroup(21-38)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/RadioButtonTooltipRow.kt (1)
RadioButtonTooltipRow(18-41)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/highlight/KtCodeViewer.kt (1)
KtCodeViewer(14-24)sdk/compose/highlights-core/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/highlights/core/Common.kt (1)
rememberCodeHighlight(17-31)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/tooling/PreviewTheme.kt (1)
PreviewTheme(12-25)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/PreviewAnnotationSection.kt (1)
PreviewAnnotationSectionPreview(97-106)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/highlight/KtCodeViewer.kt (1)
sdk/compose/highlights-core/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/highlights/core/Common.kt (1)
rememberCodeHighlight(17-31)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (ubuntu-latest)
🔇 Additional comments (33)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ImageVectorPreviewSettingsScreen.kt (4)
1-28: LGTM!The imports are well-organized and properly updated to support the new Jewel-based UI components. The removal of the
Modifierparameter from the UI function signature is reflected by removing its import.
29-37: Good ViewModel scoping for shared settings state.Using
viewModelStoreOwner = navController()correctly scopes theSettingsViewModelto the navigation controller, enabling state sharing across the tabbed settings interface. This is appropriate for a multi-tab settings UI where all tabs operate on the same settings model.
39-68: Clean UI refactoring with proper Jewel components.The migration to Jewel-based settings components is well-executed:
VerticallyScrollableContainerprovides proper scroll behavior.GroupandCheckboxSettingsRowoffer consistent settings UI patterns.- Localization via
stringResourceensures i18n support.- The structure is readable and maintainable.
70-91: Well-structured interactive preview.The preview function properly uses
rememberMutableStatefor interactive state management, allowing UI interactions to be tested in the IDE preview. Theelse -> {}branch appropriately handles otherSettingsActiontypes that aren't relevant to this specific preview.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/ui/PreviewAnnotationSection.kt (3)
30-84: LGTM!The composable is well-structured with clean separation of concerns. The animation logic correctly shows/hides the radio button group based on the checkbox state, and the tooltip integration with
KtCodeViewerandrememberCodeHighlightfollows the established patterns in the codebase.
86-104: LGTM!The preview hint constants correctly demonstrate the import paths for both annotation types:
- AndroidX:
androidx.compose.ui.tooling.preview.Previewfor standard Compose Multiplatform- JetBrains:
androidx.compose.desktop.ui.tooling.preview.Previewfor desktop-specific toolingThis aligns with the Jewel migration context where different preview annotations may be needed based on the target platform.
106-118: LGTM!The preview function is well-implemented with interactive state management via
rememberMutableState, allowing developers to test the component behavior directly in the IDE preview. This follows the established preview patterns in the codebase usingPreviewTheme.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/RadioButtonTooltipRow.kt (2)
18-41: LGTM!Well-structured composable with proper accessibility support via
stringResource("accessibility.help"). The layout usingCenterVerticalRowand the trailing lambda pattern fortooltipContentfollows project conventions.One minor consideration: the
@OptIn(ExperimentalFoundationApi::class)is likely required by Jewel'sTooltipcomponent. If this becomes stable in a future Jewel version, the annotation can be removed.
43-54: LGTM!Preview follows project conventions using
PreviewThemeand provides representative sample data for visual verification.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/highlight/KtCodeViewer.kt (1)
14-24: LGTM!The
KtCodeViewercomposable is clean and correctly integratesHighlightswith Jewel's theming by usingJewelTheme.typography.editorTextStylefor code display.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/HorizontalDivider.kt (1)
9-15: LGTM!Clean and minimal wrapper around Jewel's
Dividercomponent. The modifier chaining correctly allows callers to customize the divider while ensuring it fills the available width.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/InfoSettingsRow.kt (1)
17-37: LGTM!Well-structured settings row component. The use of
weight(1f)on the text column ensures proper layout when a trailing component is present, and the@Nlsannotations correctly indicate i18n-aware strings.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/tooling/PreviewTheme.kt (1)
12-33: LGTM!Good approach to standardize preview rendering. Setting
LocalInspectionModetotrueviaPreviewWrapperensures preview-specific code paths are correctly activated during design-time rendering in the IDE.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ui/PreviewBgSection.kt (1)
224-233: LGTM!Good use of
rememberMutableStatefor interactive preview state, allowing design-time testing of selection behavior.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/CheckboxSettingsRow.kt (1)
30-34: Verified: Infinite recursion issue is resolved.The
CheckboxRowon line 30 now correctly references the imported Jewel component from line 15, resolving the previously flagged infinite recursion.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/Group.kt (3)
24-47: LGTM! Well-structured Group component.The component correctly wraps Jewel's
GroupHeaderwith configurable padding, spacing, and an optional start component. The internal spacer before content provides consistent visual separation.
49-51: LGTM! Appropriate use of extension function.The
ColumnScopereceiver ensures this spacing function is only callable within aColumn, and the ktlint suppression is justified since this is a simple spacer helper, not a reusable component requiring a modifier parameter.
53-75: LGTM! Good preview coverage.The preview demonstrates the component's key features: header text, optional start component (icon), and nested content with a checkbox row.
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/RadioButtonGroup.kt (1)
21-38: LGTM! Clean implementation.The component follows the same structural pattern as
Groupwith appropriate defaults. The preview demonstrates proper state management withrememberMutableState.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/DropdownSettingsRow.kt (1)
42-79: Verify empty click handlers on OutlinedSplitButton.The
onClick = {}andsecondaryOnClick = {}handlers are empty (lines 62-63). If the button's primary action should be a no-op (only using the dropdown), consider adding a brief comment explaining this design choice, or verify this is the intended behavior for this settings component.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/general/GeneralSettingsScreen.kt (1)
47-66: LGTM! Clean navigation destination setup.The screen correctly wires the ViewModel, collects state, and provides callbacks for clearing settings and changing mode. Both actions appropriately navigate to IntroScreen after completion.
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/SettingsScreen.kt (3)
39-80: LGTM! Clean tab-based navigation implementation.The screen correctly:
- Uses
message()in the non-composablerememberblock (appropriate)- Uses
stringResource()in the composable context for the header- Derives tab selection from navigation state
- Properly replaces destinations on tab click
112-123: LGTM! Good preview coverage.The preview demonstrates the
Tabscomponent in isolation with mutable state for tab selection.
89-102: InteractionSource is recreated on every recomposition.The
remember { MutableInteractionSource() }on line 101 is called inside themapIndexedlambda during composition, not at theTabscomposable level. This means a newMutableInteractionSourceis created on every recomposition.🐛 Suggested fix
`@Composable` private fun Tabs( tabNames: List<String>, selectedTabIndex: () -> Int, onTabClick: (Int) -> Unit, ) { + val interactionSource = remember { MutableInteractionSource() } Column { TabStrip( tabs = tabNames.mapIndexed { index, string -> TabData.Default( selected = index == selectedTabIndex(), content = { SimpleTabContent(label = string, state = it) }, onClick = { onTabClick(index) }, closable = false, ) }, style = JewelTheme.defaultTabStyle, - interactionSource = remember { MutableInteractionSource() }, + interactionSource = interactionSource, ) HorizontalDivider() } }Likely an incorrect or invalid review comment.
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/about/AboutSettingsScreen.kt (3)
31-33: LGTM!Clean navigation destination declaration using Tiamat's delegate pattern. Follows the established project conventions for screen registration.
35-73: Well-structured settings UI with proper Jewel integration.The composable follows good practices:
- Uses
VerticallyScrollableContainerfor proper scroll handling in the settings context.- Consistent 16dp spacing pattern between elements.
- Proper accessibility with
contentDescriptionfor the logo image.- Theme-aware typography via
JewelTheme.typography.- Version display gracefully handles preview mode via
rememberPluginVersion().
75-79: LGTM!Preview setup is correct with
TopCenteralignment appropriate for a scrollable settings screen, and thePreviewThemewrapper ensures consistent theming during design-time rendering.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/generator/GeneratorSettingsScreen.kt (6)
46-54: LGTM!The ViewModel sharing pattern via
viewModelStoreOwner = navController()correctly ensures the settings state persists across navigation within the settings tabs.
65-87: LGTM!The codestyle group is well-organized with explicit mode checkbox and indent size dropdown. The indent size range of 1-10 covers common use cases.
90-146: LGTM!The ImageVector generation group is comprehensive with output format selection, flat package option, Compose colors, trailing comma, and preview annotation settings. The tooltip integration with
KtCodeViewerandrememberCodeHighlightfollows the established pattern fromPreviewAnnotationSection.
152-172: LGTM!The code hint strings accurately illustrate the two output formats (backing property vs lazy delegate), providing useful tooltip context for users.
174-212: LGTM!The preview function is well-structured with local state management for all settings. The
onActiondispatcher handles all relevantSettingsActiontypes, enabling interactive testing of the UI. Theelse -> {}branch appropriately handles any unrelated actions in this preview-only context.
56-63: Reconsider the opt-in reasoning. The@OptIn(ExperimentalFoundationApi::class)annotation is required not forVerticallyScrollableContainer, but becauseRadioButtonTooltipRowusesTooltipfrom Jewel, which requires this opt-in. Other files usingVerticallyScrollableContainer(e.g.,GeneralSettingsScreen.kt,AboutSettingsScreen.kt) do not have this annotation, confirming that the experimental API requirement comes from the tooltip component, not the scrollable container.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| val highlights = rememberCodeHighlight( | ||
| codeBlock = """ | ||
| package io.github.composegears.valkyrie | ||
|
|
||
| val Pack.MyIcon: ImageVector | ||
| get() { | ||
| if (_MyIcon != null) { | ||
| return _MyIcon!! | ||
| } | ||
| ... | ||
| } | ||
| """.trimIndent(), | ||
| emphasisLocation = arrayOf(PhraseLocation(start = 8, end = 39)), | ||
| isLight = false, | ||
| ) |
There was a problem hiding this comment.
Invalid vararg parameter usage will cause compilation error.
The rememberCodeHighlight function signature (from the SDK) defines emphasisLocation as a vararg parameter. In Kotlin, you cannot pass an array to a vararg parameter using named argument syntax (emphasisLocation = arrayOf(...)).
🐛 Proposed fix
Either pass the PhraseLocation values directly as positional arguments after the named ones:
val highlights = rememberCodeHighlight(
codeBlock = """
package io.github.composegears.valkyrie
val Pack.MyIcon: ImageVector
get() {
if (_MyIcon != null) {
return _MyIcon!!
}
...
}
""".trimIndent(),
- emphasisLocation = arrayOf(PhraseLocation(start = 8, end = 39)),
isLight = false,
+ PhraseLocation(start = 8, end = 39),
)Or use the spread operator with an array (without named argument):
val highlights = rememberCodeHighlight(
codeBlock = """
...
""".trimIndent(),
- emphasisLocation = arrayOf(PhraseLocation(start = 8, end = 39)),
isLight = false,
+ *arrayOf(PhraseLocation(start = 8, end = 39)),
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val highlights = rememberCodeHighlight( | |
| codeBlock = """ | |
| package io.github.composegears.valkyrie | |
| val Pack.MyIcon: ImageVector | |
| get() { | |
| if (_MyIcon != null) { | |
| return _MyIcon!! | |
| } | |
| ... | |
| } | |
| """.trimIndent(), | |
| emphasisLocation = arrayOf(PhraseLocation(start = 8, end = 39)), | |
| isLight = false, | |
| ) | |
| val highlights = rememberCodeHighlight( | |
| codeBlock = """ | |
| package io.github.composegears.valkyrie | |
| val Pack.MyIcon: ImageVector | |
| get() { | |
| if (_MyIcon != null) { | |
| return _MyIcon!! | |
| } | |
| ... | |
| } | |
| """.trimIndent(), | |
| isLight = false, | |
| *arrayOf(PhraseLocation(start = 8, end = 39)), | |
| ) |
🤖 Prompt for AI Agents
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/highlight/KtCodeViewer.kt`
around lines 29 - 43, The code passes an array to the vararg parameter
`emphasisLocation` of `rememberCodeHighlight` using named-argument syntax, which
won't compile; fix by either providing `PhraseLocation(...)` values directly as
vararg positional arguments after the named parameters (e.g., call
rememberCodeHighlight(..., PhraseLocation(...), PhraseLocation(...))) or
construct an array and use the spread operator when calling the vararg (e.g.,
use *arrayOf(PhraseLocation(...))) and ensure you reference the
`emphasisLocation` vararg correctly; see `rememberCodeHighlight`,
`emphasisLocation`, and `PhraseLocation` in your diff to locate the call to
update.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/components/previewer/ImageVectorPreviewPanel.kt (1)
128-138: Bug:initialHeightshould be used instead ofinitialWidthon line 133.The
min()calculation usesinitialWidth.toPx()for both divisions, which means non-square images will not scale correctly to fit the window. The second division should useinitialHeight.toPx().🐛 Proposed fix
fitToWindow = { scope.launch { with(density) { val scaleFactor = min( zoomState.maxPreviewSize.toPx() / initialWidth.toPx(), - zoomState.maxPreviewSize.toPx() / initialWidth.toPx(), + zoomState.maxPreviewSize.toPx() / initialHeight.toPx(), ) zoomState.animateToScale(scaleFactor) } } },tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/SettingsScreen.kt (1)
64-72: Handle potential -1 index fromindexOfFirst.If
tabsDestinationdoesn't match any screen (e.g., during initial composition before navigation settles),indexOfFirstreturns-1. This could causeTabStripto render incorrectly or crash when checkingindex == selectedTabIndex().Consider coercing to a valid range:
Suggested fix
Tabs( tabNames = tabNames, - selectedTabIndex = { tabScreens.indexOfFirst { destination -> destination.name == tabsDestination?.name } }, + selectedTabIndex = { + tabScreens.indexOfFirst { destination -> destination.name == tabsDestination?.name } + .coerceAtLeast(0) + }, ) { tabIndex ->
🧹 Nitpick comments (12)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/components/previewer/PreviewerToolbar.kt (1)
27-61: Good toolbar composition with a naming suggestion.The toolbar cleanly composes action buttons and layout elements. However,
defaultWidthanddefaultHeightsuggest fallback/initial values, but they appear to represent the current dimensions being displayed.Consider renaming to
width/heightorcurrentWidth/currentHeightfor clarity.♻️ Suggested parameter rename
`@Composable` fun PreviewerToolbar( - defaultWidth: Float, - defaultHeight: Float, + width: Float, + height: Float, onBgChange: (BgType) -> Unit, zoomIn: () -> Unit, zoomOut: () -> Unit, onActualZoom: () -> Unit, onFitToWindow: () -> Unit, modifier: Modifier = Modifier, ) { Toolbar(modifier = modifier) { // ... actions ... Text( modifier = Modifier.padding(horizontal = 4.dp), - text = "${defaultWidth.toInt()}x${defaultHeight.toInt()}", + text = "${width.toInt()}x${height.toInt()}", // ... ) } }tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/button/TooltipHintToggleButton.kt (1)
41-46: Unusedstateparameter - verify if icon should reflect toggle state.The
stateparameter provided byToggleableIconButtonis not used. The icon appearance (including the badge) remains the same regardless of whethervalueistrueorfalse.If this is intentional (badge always visible), consider renaming
stateto_to indicate it's intentionally unused. Otherwise, consider usingstateto adjust the icon's appearance based on the toggle value.♻️ If intentionally unused
- ) { state -> + ) { _ -> Icon(tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/icons/BlackCircle.kt (1)
21-24: UnusedstrokeLineWidthparameter.The
strokeLineWidth = 1fhas no effect when nostrokebrush is provided — the stroke will not be rendered. If the stroke was intentionally removed, consider also removingstrokeLineWidthfor clarity.♻️ Suggested cleanup
path( fill = SolidColor(Color(0xFF000000)), - strokeLineWidth = 1f, ) {tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/icons/WhiteCircleDark.kt (1)
21-24: UnusedstrokeLineWidthparameter.Same as
BlackCircle.kt—strokeLineWidth = 1fhas no effect without astrokebrush. Consider removing it for consistency with the light-themeWhiteCirclewhich does specify a stroke, or add an explicit stroke if a border is intended.♻️ Suggested cleanup
path( fill = SolidColor(Color(0xFFCED0D6)), - strokeLineWidth = 1f, ) {tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/Toolbar.kt (1)
275-277: Consider renaming the preview function.The preview is named
TopAppBarPreviewbut the component isToolbar. Consider renaming toToolbarPreviewfor consistency.♻️ Suggested rename
`@Preview` `@Composable` -private fun TopAppBarPreview() = PreviewTheme(alignment = Alignment.TopCenter) { +private fun ToolbarPreview() = PreviewTheme(alignment = Alignment.TopCenter) {tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/iconpack/conversion/ui/picker/IconPackPickerStateUi.kt (1)
26-31: Consider usingstringResourcefor the Title text.The title "IconPack generation" is hardcoded while other screens use localized strings (e.g.,
stringResource("editor.select.header")in EditorSelectScreen). For consistency and i18n support, consider extracting this to a string resource.Suggested change
Toolbar { BackAction(onBack = onBack) - Title(text = "IconPack generation") + Title(text = stringResource("iconpack.generation.header")) WeightSpacer() SettingsAction(openSettings = openSettings) }tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/picker/GenericPickerScreen.kt (1)
140-145: Consider localizing the hardcoded "Browse" string.This pre-existing hardcoded string should use
stringResourcefor localization consistency with the rest of the picker text.Suggested change
append( AnnotatedString( - text = "Browse", + text = stringResource("picker.browse"), spanStyle = SpanStyle(MaterialTheme.colorScheme.primary), ), )tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/iconpack/creation/IconPackCreationScreen.kt (1)
69-72: Consider usingstringResourcefor the Title text.The title "IconPack setup" is hardcoded. For consistency with EditorSelectScreen (which uses
stringResource("editor.select.header")) and i18n support, consider extracting this to a string resource.Suggested change
Toolbar { BackAction(onBack = onBack) - Title("IconPack setup") + Title(text = stringResource("iconpack.setup.header")) }Note: Lines 98-99 also have hardcoded strings ("Create new", "Existing pack") that would benefit from localization, though those are pre-existing and outside the scope of this migration.
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/simple/conversion/SimpleConversionScreen.kt (1)
84-93: Consider localizing the hardcoded title string.The title "Simple conversion" is hardcoded, while other screens in this PR use
stringResource()for localization (e.g.,EditorSelectUiusesstringResource("editor.edit.header")). For consistency, consider adding a localization key.The Toolbar/Title/Actions composition is otherwise correct.
Suggested change
Toolbar { BackAction(onBack = navController::back) - Title(text = "Simple conversion") + Title(text = stringResource("simple.conversion.header")) WeightSpacer()And add to
Valkyrie.properties:simple.conversion.header=Simple conversiontools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/iconpack/conversion/ui/batch/BatchProcessingStateUi.kt (1)
107-120: Toolbar migration looks correct; consider localizing the title.The
NotificationToggleActioncorrectly implements the boolean toggle pattern—settingcurrentActiontoImportIssueswhen selected andNoneotherwise.Similar to
SimpleConversionScreen, the title "IconPack generation" is hardcoded. Consider adding a localization key for consistency.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ui/PreviewBgSection.kt (2)
119-122: Minor: Redundant clip modifier.The
clip(RoundedCornerShape(8.dp))here is redundant sincePreviewItemapplies the same clip at line 218. ThedrawBehindwill be clipped byPreviewItem's clip regardless. Consider removing this to avoid the duplicate clipping operation.♻️ Optional simplification
PreviewItem( modifier = Modifier - .clip(RoundedCornerShape(8.dp)) .drawBehind {
196-222: Correct Jewel theme integration.The selection color properly uses
JewelTheme.globalColors.outlines.focusedinstead of MaterialTheme, aligning with the Jewel migration.Nit: Consider extracting the shape to a local
valto avoid creating the sameRoundedCornerShape(8.dp)twice:♻️ Optional extraction
+ val shape = RoundedCornerShape(8.dp) val color by animateColorAsState( when { selected -> JewelTheme.globalColors.outlines.focused else -> Color.Transparent }, ) Box( modifier = modifier .size(72.dp) .border( width = 2.dp, color = color, - shape = RoundedCornerShape(8.dp), + shape = shape, ) - .clip(shape = RoundedCornerShape(8.dp)) + .clip(shape = shape) .clickable(onClick = onSelect), content = content, )
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/DropdownSettingsRow.kt`:
- Around line 35-41: The DropdownSettingsRow signature should annotate the
user-facing infoText parameter with `@Nls` for localization consistency; update
the function declaration of DropdownSettingsRow to change the infoText parameter
to `@Nls` infoText: String? (keeping the existing default and nullability) so it
matches the annotations used on text and buttonText and aligns with
InfoSettingsRow.
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/SettingsScreen.kt`:
- Around line 81-103: The Tabs composable can receive a selectedTabIndex() of -1
(e.g., when tabsDestination isn't found), causing all tabs to be unselected and
a flicker; inside Tabs (the Tabs function) compute a safe selected index before
mapping tabNames — e.g., val safeSelected = selectedTabIndex().takeIf { it in
tabNames.indices } ?: (if (tabNames.isNotEmpty()) 0 else -1) — then use
safeSelected for TabData.Default(selected = index == safeSelected) so a valid
tab is selected when the list is non-empty.
In `@tools/idea-plugin/src/main/resources/messages/Valkyrie.properties`:
- Line 92: The tooltip key component.toolbar.chessboard.tooltip (and the related
accessibility.chessboard) uses ambiguous text "Show pixel grid"; verify whether
the UI displays a transparency checkerboard or pixel-boundary overlay and then
update both properties to accurate wording (e.g., "Show transparency grid" or
"Toggle chessboard background" if it's the transparency checkerboard, or keep
"Show pixel grid" only if it truly displays pixel boundaries); update the values
for component.toolbar.chessboard.tooltip and accessibility.chessboard
accordingly so both keys match the actual behavior.
🧹 Nitpick comments (10)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/common/picker/UniversalPicker.kt (1)
129-142: Simplify redundant layout nesting.The outer
CenterVerticalRowadds no behavior on Line 129. Flattening reduces hierarchy without changing layout.♻️ Suggested simplification
- CenterVerticalRow { - CenterVerticalRow { - Icon( - key = AllIconsKeys.Actions.AddFile, - contentDescription = null, - ) - Spacer(8.dp) - Text( - modifier = Modifier.padding(vertical = 8.dp), - text = stringResource("universal.picker.dnd"), - maxLines = 2, - ) - } - } + CenterVerticalRow { + Icon( + key = AllIconsKeys.Actions.AddFile, + contentDescription = null, + ) + Spacer(8.dp) + Text( + modifier = Modifier.padding(vertical = 8.dp), + text = stringResource("universal.picker.dnd"), + maxLines = 2, + ) + }tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/DragAndDropBox.kt (1)
37-79: Consider consolidating with the similarDragAndDropBoxinUniversalPicker.kt.There's a nearly identical
DragAndDropBoxcomposable inUniversalPicker.kt(lines 174-207) with the same Jewel theming, shape defaults, and dashed border styling. The main differences are:
- This version has hover state handling and is clickable
UniversalPicker.ktversion only responds toisDraggingand is not clickableYou could extract the shared visual styling (dashed border, background, shape) into a common internal composable to reduce duplication.
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/platform/Os.kt (1)
11-17: Prefer keyingrememberwithhostOs(or compute inline).This avoids caching a stale value if
hostOsis overridden in tests or previews and makes the dependency explicit.♻️ Proposed tweak
-fun rememberCurrentOs(): Os = remember { +fun rememberCurrentOs(): Os = remember(hostOs) { when (hostOs) { MacOS -> Os.MacOS Windows -> Os.Windows Linux -> Os.Linux else -> Os.Unsupported } }tools/idea-plugin/src/main/resources/messages/Valkyrie.properties (1)
19-26: Consider standardizing picker key prefixes.The picker-related keys use inconsistent prefixes:
generic.picker.*(lines 19-22) anduniversal.picker.*(lines 23-26). Standardizing on a single prefix (e.g., either "generic" or "universal") would improve consistency and maintainability of the resource bundle.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/preview/ui/PreviewBgSection.kt (1)
121-123: Consider removing redundant clip modifier.The
clip(RoundedCornerShape(8.dp))here is redundant sincePreviewItemalready applies.clip(shape = shape)with the same default shape at line 221. ThedrawBehindcontent is already constrained by the Box's bounds inPreviewItem.♻️ Suggested simplification
PreviewItem( modifier = Modifier - .clip(RoundedCornerShape(8.dp)) .drawBehind {tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/InfoCard.kt (1)
70-111: RedundantColumnwrapper and potential dual icon/image rendering.Two observations:
The
Columnat line 71 wraps onlyCenterVerticalRowand adds no additional children or modifiers, making it redundant.If both
iconandimageare provided, both will render in the same row. If this is intentional, consider documenting it; if not, consider using anelsebranch or making them mutually exclusive.♻️ Suggested simplification
) { - Column { - CenterVerticalRow( - modifier = Modifier.padding(16.dp), - horizontalArrangement = Arrangement.spacedBy(16.dp), - ) { - icon?.let { + CenterVerticalRow( + modifier = Modifier.padding(16.dp), + horizontalArrangement = Arrangement.spacedBy(16.dp), + ) { + when { + icon != null -> Icon( + modifier = Modifier + .align(Alignment.Top) + .size(36.dp), + imageVector = icon, + contentDescription = null, + tint = JewelTheme.contentColor, + ) + image != null -> Image( + modifier = Modifier + .align(Alignment.Top) + .size(36.dp), + imageVector = image, + contentDescription = null, + ) + } ... - } } }tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/general/GeneralSettingsScreen.kt (1)
102-108: Consider disabling the Link when mode isUnspecified.The
Linkis always rendered but does nothing whenmode == Unspecified(line 105). This could confuse users who click it expecting an action. Consider using theenabledparameter or conditionally hiding the link.♻️ Suggested improvement
trailing = { Link( text = stringResource("settings.general.current.mode.action"), + enabled = mode != Unspecified, onClick = { if (mode != Unspecified) onChangeMode() }, ) },tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/button/TooltipIconButton.kt (1)
16-55: Consider making focusability configurable for accessibility.
Hardcodingfocusable = falsecan block keyboard navigation if these buttons are the only way to trigger actions. Exposing a parameter lets callers opt out when needed.♻️ Suggested refactor
fun TooltipIconButton( key: IconKey, contentDescription: String?, onClick: () -> Unit, tooltipText: String, + focusable: Boolean = true, modifier: Modifier = Modifier, ) { IconActionButton( modifier = modifier, key = key, onClick = onClick, - focusable = false, + focusable = focusable, contentDescription = contentDescription, tooltip = { Text(text = tooltipText) }, ) } @@ fun TooltipIconButton( imageVector: ImageVector, contentDescription: String?, onClick: () -> Unit, tooltipText: String, + focusable: Boolean = true, modifier: Modifier = Modifier, ) { IconActionButton( modifier = modifier, painter = rememberVectorPainter(imageVector), onClick = onClick, - focusable = false, + focusable = focusable, contentDescription = contentDescription, tooltip = { Text(text = tooltipText) }, ) }tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/Toolbar.kt (1)
274-303: Preview covers the key toolbar interactions.The preview demonstrates various action types including regular buttons, toggle buttons, and layout with
WeightSpacer. Consider adding additional preview variants for different toolbar configurations (e.g., minimal toolbar, zoom-only toolbar) to aid design-time iteration.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/tabs/about/AboutSettingsScreen.kt (1)
60-68: Hardcoded URLs are acceptable here but consider extracting.The GitHub repository URLs are hardcoded, which is reasonable for an About screen. If these URLs are used elsewhere in the codebase, consider extracting them to a constants file for maintainability.
| fun DropdownSettingsRow( | ||
| @Nls text: String, | ||
| @Nls buttonText: String, | ||
| modifier: Modifier = Modifier, | ||
| paddingValues: PaddingValues = PaddingValues(start = 4.dp), | ||
| infoText: String? = null, | ||
| menuContent: MenuScope.() -> Unit, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's read the complete file to understand the context and how infoText is used
wc -l tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/DropdownSettingsRow.ktRepository: ComposeGears/Valkyrie
Length of output: 172
🏁 Script executed:
# Read the full file
cat -n tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/DropdownSettingsRow.ktRepository: ComposeGears/Valkyrie
Length of output: 5405
🏁 Script executed:
# Search for similar SettingsRow components and how they annotate infoText/info-like parameters
rg -n "SettingsRow\|@Nls.*String\?" tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/ -C2 --max-count=50Repository: ComposeGears/Valkyrie
Length of output: 47
🏁 Script executed:
# Look for patterns of infoText usage in the codebase
rg -n "infoText" tools/idea-plugin/src/main/kotlin -C3 --max-count=30Repository: ComposeGears/Valkyrie
Length of output: 24330
Add @Nls annotation to infoText parameter for localization consistency.
infoText is user-facing text displayed via the InfoText component and should be marked with @Nls like text and buttonText. This ensures consistent localization inspections across similar components—InfoSettingsRow already marks infoText as @Nls.
♻️ Proposed change
- infoText: String? = null,
+ `@Nls` infoText: String? = null,🤖 Prompt for AI Agents
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/DropdownSettingsRow.kt`
around lines 35 - 41, The DropdownSettingsRow signature should annotate the
user-facing infoText parameter with `@Nls` for localization consistency; update
the function declaration of DropdownSettingsRow to change the infoText parameter
to `@Nls` infoText: String? (keeping the existing default and nullability) so it
matches the annotations used on text and buttonText and aligns with
InfoSettingsRow.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that the `@Nls` annotation was added to the infoText parameter
cat tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/DropdownSettingsRow.kt | grep -A 2 "infoText:"
Length of output: 72
@egorikftp Thanks for the quick response! However, I'm still seeing the infoText parameter without the @Nls annotation in the current file:
infoText: String? = null,Could you please verify that the change was committed and pushed? The parameter should look like:
`@Nls` infoText: String? = null,This will ensure localization consistency with the other parameters (text and buttonText in the diff snippet, or text in the current full implementation).
🧬 Code Graph Analysis Results
Filename: tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/DropdownSettingsRow.kt
Line ranges: Not provided for this file (the full content is given)
[Relevant snippets]
Filename: tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/tooling/PreviewTheme.kt
Lines 22-33:
`@Composable`
fun PreviewTheme(
modifier: Modifier = Modifier,
alignment: Alignment = Alignment.TopStart,
content: `@Composable` BoxScope.() -> Unit,
) {
Box(
modifier = modifier.fillMaxSize(),
content = content,
contentAlignment = alignment,
)
}
Filename: sdk/compose/foundation/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/foundation/layout/Row.kt
Lines 10-20:
`@Composable`
inline fun CenterVerticalRow(
modifier: Modifier = Modifier,
horizontalArrangement: Arrangement.Horizontal = Arrangement.Start,
content: `@Composable` RowScope.() -> Unit,
) = Row(
verticalAlignment = Alignment.CenterVertically,
horizontalArrangement = horizontalArrangement,
modifier = modifier,
content = content,
)
Filename: tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/DropdownList.kt
Lines 24-53:
`@OptIn`(ExperimentalJewelApi::class)
`@Composable`
fun <T : Any> DropdownList(
selected: T,
items: List<T>,
transform: (T) -> String,
onSelectItem: (T) -> Unit,
modifier: Modifier = Modifier,
maxPopupWidth: Dp = Dp.Unspecified,
) {
val updatedTransform by rememberUpdatedState(transform)
// TODO: Remove workaround in IDEA 2027 🌚
// https://youtrack.jetbrains.com/issue/JEWEL-1244/ListComboBox-saves-the-selected-index-internally-even-if-it-is-reset-externally
val listState = rememberSelectableLazyListState()
LaunchedEffect(selected) {
listState.selectedKeys = setOf(updatedTransform(selected))
}
ListComboBox(
items = items.map(transform),
selectedIndex = items.indexOf(selected),
onSelectedItemChange = { onSelectItem(items[it]) },
maxPopupWidth = maxPopupWidth,
modifier = modifier,
itemKeys = { _, item -> item },
listState = listState,
)
}
🧠 Learnings used
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 822
File: tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/DropdownList.kt:42-49
Timestamp: 2026-01-24T10:43:07.152Z
Learning: In the Valkyrie project, dropdown components (like DropdownList in tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/DropdownList.kt) are used with hardcoded lists where the selected item is always guaranteed to be in the items list, so defensive checks for indexOf returning -1 are not needed.
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 805
File: tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/RadioButtonTooltipRow.kt:18-18
Timestamp: 2026-01-15T08:37:06.618Z
Learning: In JetBrains Jewel UI library, some components (such as Tooltip and others) are marked as experimental and require `OptIn(ExperimentalFoundationApi::class)`. Do not flag this annotation as unnecessary when used with Jewel components in the ComposeGears/Valkyrie project.
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 801
File: gradle/libs.versions.toml:14-14
Timestamp: 2026-01-01T18:09:41.901Z
Learning: In the Valkyrie project (ComposeGears/Valkyrie), compose-ui-tooling-preview must use version 1.10.0 (even though runtime Compose is 1.8.2) because 1.10.0+ provides unified KMP preview annotation support required for IntelliJ IDEA 2025.3+ preview functionality.
| @Composable | ||
| private fun Tabs( | ||
| tabNames: List<String>, | ||
| selectedTabIndex: () -> Int, | ||
| onTabClick: (Int) -> Unit, | ||
| ) { | ||
| Column { | ||
| TabStrip( | ||
| tabs = tabNames.mapIndexed { index, string -> | ||
| TabData.Default( | ||
| selected = index == selectedTabIndex(), | ||
| content = { | ||
| SimpleTabContent(label = string, state = it) | ||
| }, | ||
| onClick = { onTabClick(index) }, | ||
| closable = false, | ||
| ) | ||
| }, | ||
| style = JewelTheme.defaultTabStyle, | ||
| interactionSource = remember { MutableInteractionSource() }, | ||
| ) | ||
| HorizontalDivider() | ||
| } |
There was a problem hiding this comment.
Guard against -1 selected index to avoid “no tab selected” flicker.
When tabsDestination is null or not in the list, indexOfFirst returns -1, leaving all tabs unselected.
💡 Suggested fix
`@Composable`
private fun Tabs(
tabNames: List<String>,
selectedTabIndex: () -> Int,
onTabClick: (Int) -> Unit,
) {
Column {
+ val selectedIndex = selectedTabIndex().coerceAtLeast(0)
TabStrip(
tabs = tabNames.mapIndexed { index, string ->
TabData.Default(
- selected = index == selectedTabIndex(),
+ selected = index == selectedIndex,
content = {
SimpleTabContent(label = string, state = it)
},🤖 Prompt for AI Agents
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/settings/SettingsScreen.kt`
around lines 81 - 103, The Tabs composable can receive a selectedTabIndex() of
-1 (e.g., when tabsDestination isn't found), causing all tabs to be unselected
and a flicker; inside Tabs (the Tabs function) compute a safe selected index
before mapping tabNames — e.g., val safeSelected = selectedTabIndex().takeIf {
it in tabNames.indices } ?: (if (tabNames.isNotEmpty()) 0 else -1) — then use
safeSelected for TabData.Default(selected = index == safeSelected) so a valid
tab is selected when the list is non-empty.
There was a problem hiding this comment.
@egorikftp, understood! You're right that the defensive check isn't necessary here. Since the tabs list is hardcoded and navigation is controlled to only use those predefined destinations, the selected index will always resolve to a valid tab. Thank you for the clarification.
🧠 Learnings used
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 822
File: tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/DropdownList.kt:42-49
Timestamp: 2026-01-24T10:43:07.152Z
Learning: In the Valkyrie project, dropdown components (like DropdownList in tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/DropdownList.kt) are used with hardcoded lists where the selected item is always guaranteed to be in the items list, so defensive checks for indexOf returning -1 are not needed.
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 801
File: gradle/libs.versions.toml:14-14
Timestamp: 2026-01-01T18:09:41.901Z
Learning: In the Valkyrie project (ComposeGears/Valkyrie), compose-ui-tooling-preview must use version 1.10.0 (even though runtime Compose is 1.8.2) because 1.10.0+ provides unified KMP preview annotation support required for IntelliJ IDEA 2025.3+ preview functionality.
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 805
File: tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/RadioButtonTooltipRow.kt:18-18
Timestamp: 2026-01-15T08:37:06.618Z
Learning: In JetBrains Jewel UI library, some components (such as Tooltip and others) are marked as experimental and require `OptIn(ExperimentalFoundationApi::class)`. Do not flag this annotation as unnecessary when used with Jewel components in the ComposeGears/Valkyrie project.
| imagevectortoxml.edit.name.label=Icon Name | ||
| component.toolbar.back.tooltip=Back | ||
| component.toolbar.close.tooltip=Close | ||
| component.toolbar.chessboard.tooltip=Show pixel grid |
There was a problem hiding this comment.
Clarify "chessboard" tooltip text.
The tooltip text "Show pixel grid" appears misleading. In image editors, a "chessboard pattern" typically refers to the alternating-color transparency grid, not a pixel grid overlay. Consider updating to:
- "Show transparency grid" or "Toggle chessboard background" if it shows transparency
- "Show pixel grid" only if it actually displays pixel boundaries
This same issue appears on both line 92 (component.toolbar.chessboard.tooltip) and line 107 (accessibility.chessboard).
#!/bin/bash
# Search for chessboard-related code to verify the actual functionality
rg -n -C3 'chessboard' --type=ktAlso applies to: 107-107
🤖 Prompt for AI Agents
In `@tools/idea-plugin/src/main/resources/messages/Valkyrie.properties` at line
92, The tooltip key component.toolbar.chessboard.tooltip (and the related
accessibility.chessboard) uses ambiguous text "Show pixel grid"; verify whether
the UI displays a transparency checkerboard or pixel-boundary overlay and then
update both properties to accurate wording (e.g., "Show transparency grid" or
"Toggle chessboard background" if it's the transparency checkerboard, or keep
"Show pixel grid" only if it truly displays pixel boundaries); update the values
for component.toolbar.chessboard.tooltip and accessibility.chessboard
accordingly so both keys match the actual behavior.
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/components/previewer/ImageVectorPreviewPanel.kt (1)
131-134: Pre-existing bug:fitToWindowscale calculation usesinitialWidthtwice.The
min()calculation on line 133 should useinitialHeight.toPx()instead ofinitialWidth.toPx(). Currently, the fit-to-window feature will not scale correctly for non-square images.🐛 Proposed fix
val scaleFactor = min( zoomState.maxPreviewSize.toPx() / initialWidth.toPx(), - zoomState.maxPreviewSize.toPx() / initialWidth.toPx(), + zoomState.maxPreviewSize.toPx() / initialHeight.toPx(), )tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/highlights/CodeViewerTooltip.kt (1)
24-39: Blank tooltip content after CodeViewer removalWith
CodeViewercommented out, the tooltip renders an emptySurface, so users get no preview on hover. IfCodeViewerisn’t ready, consider replacing it with a Jewel-compatible viewer or removing the tooltip (render only the icon) until a real preview is available.
🤖 Fix all issues with AI agents
In
`@sdk/compose/foundation/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/foundation/Modifier.kt`:
- Line 27: The import of org.jetbrains.skia.Matrix33 and the animatedBorder
implementation are JVM-only and will break WASM compilation; move the
animatedBorder function (and its Matrix33 usage) out of commonMain into a
JVM-only source set (create jvmMain and place the file or the function there) or
alternatively implement a WASM-compatible shader approach that avoids Matrix33
and Skia types (e.g., provide a multiplatform abstraction used by animatedBorder
and platform-specific implementations in jvmMain and wasmMain); update
references to animatedBorder so consumers in commonMain call the multiplatform
abstraction or the platform-specific implementation.
In `@tools/idea-plugin/build.gradle.kts`:
- Around line 19-39: The call to Configuration.exclude uses a positional
argument exclude("org.jetbrains.skiko") inside the configurations.all block;
change it to use the named parameter form (e.g., exclude(group =
"org.jetbrains.skiko")) to match the other exclude(...) usages and the Gradle
Kotlin DSL signature, and while here consider removing the large commented-out
block of explored exclusions (lines with excluded Compose/Android/Kotlin
entries) to keep build.gradle.kts tidy once the migration is stable; locate this
inside the configurations.all block where exclude(...) is invoked.
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/banner/BannerData.kt`:
- Around line 14-21: Replace the fragile isActive check in
BannerDataImpl.dismiss with an idempotent AtomicBoolean guard: add a private
AtomicBoolean (e.g., dismissed) to BannerDataImpl and in dismiss call
dismissed.compareAndSet(false, true) and only if it returns true attempt to
resume the continuation (optionally still check continuation.isActive before
calling continuation.resume(Unit)); this ensures a single resume without using
internal coroutines APIs like tryResume/completeResume.
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/banner/BannerHost.kt`:
- Around line 75-82: BannerRender maps InfoBanner to InlineSuccessBanner causing
semantic mismatch; update the code to either (A) detect if the Jewel dependency
includes InlineInfoBanner and use it instead of InlineSuccessBanner, or (B) add
a new composable InlineInfoBanner wrapper (e.g., implement InlineInfoBanner that
accepts text and renders a neutral-styled banner using existing primitives) and
replace the mapping in BannerRender (BannerData -> InfoBanner ->
InlineInfoBanner), or (C) add an inline comment/docstring near
BannerRender/BannerData explicitly documenting that InfoBanner intentionally
falls back to InlineSuccessBanner if no InlineInfoBanner exists; choose one
approach and implement it so InfoBanner no longer semantically maps silently to
a “success” banner.
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/button/OutlineIconButton.kt`:
- Around line 21-48: Change OutlineIconButton to require a non-null
contentDescription: String (remove default null) so icon-only buttons are always
accessible; update all call sites and previews to pass a localized label using
stringResource(...) (do not use hardcoded text) and ensure any preview uses a
sample stringResource key. Locate the OutlineIconButton(...) function and its
preview usages and replace nullable contentDescription handling with the
required String parameter, and use stringResource(...) when invoking the
composable.
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/editor/CodeEditor.kt`:
- Around line 68-81: The gutter width is fixed at 32.dp in the CodeEditor
BasicTextField which will truncate multi‑digit line numbers; change the
Modifier.width(...) to compute width dynamically based on the number of digits
in linesCount (e.g. multiply a per‑digit dp value by
linesCount.toString().length) so the BasicTextField showing IntRange(1,
linesCount) can expand for large files; update the Modifier on the
BasicTextField in CodeEditor to use that dynamic width calculation and keep
existing verticalScroll(linesTextScroll) and textStyle settings.
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/InfoCard.kt`:
- Around line 41-59: The current InfoCard Composable renders both Icon and Image
when both parameters are provided (icon and image), causing unintended double
visuals; update the rendering logic in InfoCard.kt so they are mutually
exclusive by prioritizing icon: check icon first and render Icon(...) only if
non-null, otherwise render Image(...) when image is non-null (i.e., replace the
two independent icon?.let and image?.let blocks with a single conditional that
prefers icon over image).
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/LinkIcon.kt`:
- Around line 51-55: The clipboard action currently calls
clipboard.setClipEntry(ClipEntry(StringSelection(url))) directly inside
scope.launch and lacks error handling; wrap that call in a runCatching { ... }
(same pattern used by openUri) and log failures with the same logger used by
openUri (use onFailure or getOrElse to call logger.error with a descriptive
message and the throwable), keeping the scope.launch and preserving
ClipEntry/StringSelection usage.
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/textfield/ConfirmTextField.kt`:
- Around line 51-69: The Escape handler in ConfirmTextField's onKeyEvent uses
the captured parameter text (stale closure) when calling
state.setTextAndPlaceCursorAtEnd(text); change this to read the latest parent
value via rememberUpdatedState so the Escape branch restores the current prop
instead of the composition-time value — add a rememberUpdatedState(text) (e.g.,
latestText) near the top of the composable and replace uses of the captured text
in the onKeyEvent Escape branch with that latestText variable.
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/common/ui/SidePanel.kt`:
- Around line 54-62: Replace the incorrect use of
JewelTheme.globalColors.borders.normal as the Box background with the semantic
panel fill color JewelTheme.globalColors.panelBackground inside the SidePanel
Box modifier; update the background(...) call in the Box (in SidePanel.kt) to
use panelBackground instead of borders.normal so the panel uses the proper theme
token.
♻️ Duplicate comments (2)
tools/idea-plugin/src/main/resources/messages/Valkyrie.properties (1)
109-109: Unresolved: Clarify "chessboard" vs "pixel grid" terminology.This issue was previously flagged but remains unresolved. The key name contains "chessboard" but the tooltip text says "Show pixel grid", which are typically different features in image editors:
- Chessboard pattern: alternating-color transparency grid
- Pixel grid: overlay showing pixel boundaries
Please verify the actual UI functionality and update both
component.toolbar.chessboard.tooltip(line 109) andaccessibility.chessboard(line 125) to use consistent and accurate terminology.Also applies to: 125-125
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/DropdownSettingsRow.kt (1)
31-44: Add@Nlsannotation toinfoTextparameter for localization consistency.The
infoTextparameter is user-facing text displayed viaInfoTextand should be marked with@Nlslike thetextparameter.Also,
comboxModifierappears to be a typo—consider renaming tocomboModifierorcomboBoxModifier.,
♻️ Suggested fix
fun <T : Any> DropdownSettingsRow( `@Nls` text: String, current: T, items: List<T>, onSelectItem: (T) -> Unit, modifier: Modifier = Modifier, - comboxModifier: Modifier = Modifier, + comboBoxModifier: Modifier = Modifier, paddingValues: PaddingValues = PaddingValues(start = 4.dp), dropdownHorizontalPadding: Dp = 0.dp, transform: (T) -> String = { it.toString() }, - infoText: String? = null, + `@Nls` infoText: String? = null, )
🧹 Nitpick comments (11)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/LinkIcon.kt (1)
72-78: Consider passing the exception to the logger for stack trace capture.Using
$itin string interpolation callstoString()on the exception, which loses the stack trace. IfJewelLogger.errorsupports a throwable parameter, consider using it for better diagnostics.Suggested improvement
private fun openUri(uriHandler: UriHandler, link: String) { runCatching { uriHandler.openUri(link) - }.onFailure { - JewelLogger.getInstance("LinkIcon").error("Unable to open link ($link). Error: $it") + }.onFailure { throwable -> + JewelLogger.getInstance("LinkIcon").error("Unable to open link ($link)", throwable) } }tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/common/ui/WebImportTopActions.kt (1)
95-104: Rename shadowedstateparameter to avoid confusion.The lambda parameter
stateon line 98 shadows the outerstatevariable (theTextFieldStatefrom line 71). While the code works correctly—the outerstateis captured inonClickand the innerstate(anInteractiveComponentState) is used for theStatefulhint—this naming collision reduces readability.♻️ Suggested fix
IconButton( onClick = { state.setTextAndPlaceCursorAtEnd("") }, style = JewelTheme.textFieldStyle.iconButtonStyle, - ) { state -> + ) { buttonState -> Icon( key = AllIconsKeys.General.Close, contentDescription = stringResource("accessibility.clear"), - hint = Stateful(state), + hint = Stateful(buttonState), ) }tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/FocusableTextField.kt (1)
65-71: Consider adding aReplaceWithto the deprecation.
Helps callers migrate toConfirmTextFieldwith IDE quick-fix support.sdk/compose/foundation/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/foundation/Modifier.kt (1)
54-83: Avoid per-frame ShaderBrush allocation to preserve cache benefits.
remember(angle, colors)recreates theShaderBrushevery frame (angle changes each tick), which defeatsdrawWithCachecaching and can add GC churn. Consider keeping the brush stable and reading the latest angle viarememberUpdatedState.♻️ Suggested refactor
+import androidx.compose.runtime.rememberUpdatedState ... - val shader = remember(angle, colors) { + val angleState = rememberUpdatedState(angle) + val shader = remember(colors) { object : ShaderBrush() { override fun createShader(size: Size): Shader { val x = size.width / 2f val y = size.height / 2f return SweepGradientShader( center = Offset(x, y), colors = colors, ).makeWithLocalMatrix( localMatrix = Matrix33.makeRotate( - deg = angle, + deg = angleState.value, pivotx = x, pivoty = y, ), ) } } }tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/banner/BannerHost.kt (1)
55-64: Transition spec condition appears to always be true.The condition
initialState != targetStateon line 56 is effectively always true whenAnimatedContenttriggers an animation (since animations occur precisely when the target state differs from the initial state). The else branch (lines 60-61) appears unreachable.If both branches should use the same animation, consider simplifying:
Suggested simplification
transitionSpec = { - if (initialState != targetState) { - (slideInVertically { height -> height } + fadeIn()) togetherWith - slideOutVertically { height -> height } + fadeOut() - } else { - slideInVertically { height -> -height } + fadeIn() togetherWith - slideOutVertically { height -> -height } + fadeOut() - }.using( + (slideInVertically { height -> height } + fadeIn() togetherWith + slideOutVertically { height -> height } + fadeOut()).using( SizeTransform(clip = false), ) },tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/InfoCard.kt (1)
35-76: RedundantColumnwrapper.The outer
Columnon line 36 contains onlyCenterVerticalRowas its child and provides no additional layout behavior. Consider removing it.Simplified structure
FocusContainer(modifier = modifier, onClick = onClick) { - Column { - CenterVerticalRow( - modifier = Modifier.padding(16.dp), - horizontalArrangement = Arrangement.spacedBy(16.dp), - ) { + CenterVerticalRow( + modifier = Modifier.padding(16.dp), + horizontalArrangement = Arrangement.spacedBy(16.dp), + ) { // ... content ... - } } }tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/textfield/ConfirmTextField.kt (2)
82-92: Inconsistent behavior: trailing button doesn't clear focus.The Enter key (line 60) clears focus after calling
onValueChange, but the trailing check button (line 88) only callsonValueChangewithout clearing focus. This inconsistency may confuse users who expect both confirmation methods to behave identically.Proposed fix for consistency
IconActionButton( key = AllIconsKeys.Actions.Checked, contentDescription = null, onClick = { + focusManager.clearFocus() onValueChange(state.text.toString()) }, )
39-43: TextField state doesn't sync with externaltextchanges.
rememberTextFieldState(text)only usestextas the initial value. If the parent component updates thetextprop (e.g., from a ViewModel), the TextField won't reflect the new value.If external updates should sync to the TextField, consider adding a
LaunchedEffect:Optional: sync external text changes
val state = rememberTextFieldState(text) + val currentText by rememberUpdatedState(text) + + LaunchedEffect(currentText) { + if (state.text.toString() != currentText) { + state.setTextAndPlaceCursorAtEnd(currentText) + } + }Note: This changes the component from "uncontrolled with initial value" to "controlled". Evaluate whether this matches your intended behavior.
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/settings/DropdownSettingsRow.kt (1)
47-81: Consider simplifying the offset calculation logic.The nested
onGloballyPositionedcallbacks to calculate InfoText alignment work correctly but add complexity. If the dropdown width is known or can be constrained, a simpler approach using fixed padding orModifier.alignBymight be cleaner.That said, this approach handles dynamic layouts well, and since this is settings UI (not performance-critical), the current implementation is acceptable.
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/material/ui/FontCustomization.kt (1)
161-176: Consider using a more semantic background color.Line 171 uses
JewelTheme.globalColors.borders.normalas a background color. Border colors are semantically intended for outlines, not fill areas. Consider using a background-specific color fromJewelTheme.globalColorsfor clearer intent.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/editor/CodeEditor.kt (1)
91-103: Potential performance concern with syntax highlighting in visualTransformation.The
visualTransformationlambda rebuilds theHighlightsobject on every keystroke/recomposition. For large files, this could cause noticeable lag. Consider memoizing the highlighted result or usingrememberwith appropriate keys.Consider memoizing highlights outside visualTransformation
+ val highlights = remember(text, syntaxLanguage, isDark) { + Highlights.Builder() + .code(text) + .language(syntaxLanguage) + .theme(SyntaxThemes.darcula(darkMode = isDark)) + .build() + } + BasicTextField( ... visualTransformation = { text -> - val code = Highlights.Builder() - .code(text.text) - .language(syntaxLanguage) - .theme(SyntaxThemes.darcula(darkMode = isDark)) - .build() - .buildAnnotatedString() - TransformedText( - text = code, + text = highlights.buildAnnotatedString(), offsetMapping = OffsetMapping.Identity, ) },
...ion/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/compose/foundation/Modifier.kt
Show resolved
Hide resolved
| internal class BannerDataImpl( | ||
| private val continuation: CancellableContinuation<Unit>, | ||
| override val message: BannerMessage, | ||
| ) : BannerData { | ||
|
|
||
| override fun dismiss() { | ||
| if (continuation.isActive) continuation.resume(Unit) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "BannerData.kt" | head -20Repository: ComposeGears/Valkyrie
Length of output: 159
🏁 Script executed:
cat -n ./tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/banner/BannerData.ktRepository: ComposeGears/Valkyrie
Length of output: 727
🏁 Script executed:
# Check what tryResume and completeResume are and where they come from
rg -t kotlin "tryResume|completeResume" --max-count=20Repository: ComposeGears/Valkyrie
Length of output: 47
🏁 Script executed:
# Let me check the kotlinx.coroutines library version and documentation
# First, check if there's a build file to see dependencies
find . -type f \( -name "build.gradle.kts" -o -name "build.gradle" -o -name "pom.xml" \) | head -10Repository: ComposeGears/Valkyrie
Length of output: 540
🏁 Script executed:
find ./tools/idea-plugin -type f \( -name "build.gradle.kts" -o -name "build.gradle" \)Repository: ComposeGears/Valkyrie
Length of output: 101
🏁 Script executed:
cat -n ./tools/idea-plugin/build.gradle.ktsRepository: ComposeGears/Valkyrie
Length of output: 7157
🏁 Script executed:
# Check the gradle libs catalog for kotlinx-coroutines version
find . -type f \( -name "libs.versions.toml" -o -name "gradle.properties" \) | xargs cat 2>/dev/null | grep -i "coroutines"Repository: ComposeGears/Valkyrie
Length of output: 47
🏁 Script executed:
find . -type f -name "libs.versions.toml"Repository: ComposeGears/Valkyrie
Length of output: 92
🏁 Script executed:
cat -n ./gradle/libs.versions.tomlRepository: ComposeGears/Valkyrie
Length of output: 4099
🌐 Web query:
Kotlin coroutines CancellableContinuation tryResume completeResume API
💡 Result:
In kotlinx.coroutines, CancellableContinuation.tryResume(...) + completeResume(token) is/was an internal coroutines machinery pair used to make “resume exactly once” atomic without throwing when multiple threads/callbacks race to resume the same continuation.
What the API does
tryResume(value, idempotent)attempts to move the continuation from Active → Resumed and returns a token if it succeeded, ornullif it was already completed/cancelled (or otherwise couldn’t be resumed). Signature (historical/experimental docs):tryResume(...): Any?andcompleteResume(token: Any).[1]- If
tryResumereturns non-null, you must callcompleteResume(token)to finalize the resume.[1]
Conceptually:
val token = cont.tryResume(value)
if (token != null) cont.completeResume(token)Why you usually should not use it
- In current coroutines,
CancellableContinuationis thread-safe, but concurrentresume(...)calls are a programming error and can throwIllegalStateException.[2] tryResume/completeResumeis treated as internal API (often marked@InternalCoroutinesApi/ not public-stable), and JetBrains explicitly recommends not using it because it may change or be removed without compatibility guarantees.[3]
What to use instead (typical fixes)
- If you’re adapting a multi-shot callback, prefer
callbackFlow(explicitly recommended by the docs for multi-shot APIs).[4] - If you just need “resume once, ignore later callbacks”, use an
AtomicBoolean(recommended in the coroutines Slack thread).[3]
Sources:
[1] Historical/experimental CancellableContinuation docs showing tryResume(...): Any? and completeResume(token: Any).[1]
[2] Current CancellableContinuation docs: concurrent resumes are an error and can throw.[2]
[3] Kotlin coroutines Slack: tryResume is internal/unstable; suggestion to use AtomicBoolean.[3]
[4] suspendCancellableCoroutine docs: single-shot callbacks; for multi-shot use callbackFlow.[4]
🏁 Script executed:
# Check if tryResume and completeResume are available in the current Kotlin version and what their status is
python3 << 'EOF'
import urllib.request
import json
# Query the Maven Central API for kotlinx-coroutines latest version
try:
url = "https://repo1.maven.org/maven2/org/jetbrains/kotlinx/kotlinx-coroutines-core/maven-metadata.xml"
with urllib.request.urlopen(url) as response:
content = response.read().decode('utf-8')
# Extract latest version
import re
match = re.search(r'<latest>([\d.]+)</latest>', content)
if match:
print(f"Latest kotlinx-coroutines version: {match.group(1)}")
except Exception as e:
print(f"Error: {e}")
EOFRepository: ComposeGears/Valkyrie
Length of output: 151
Guard against double-dismiss, but avoid internal coroutines APIs.
The race condition between the isActive check and resume() call is real—double-dismiss will throw IllegalStateException. However, tryResume/completeResume are internal coroutines APIs (@InternalCoroutinesApi) and not suitable for production code; they lack stability guarantees and may change without notice.
Use an AtomicBoolean instead for a stable, idempotent guard:
✓ Recommended fix
+import java.util.concurrent.atomic.AtomicBoolean
import kotlin.coroutines.resume
import kotlinx.coroutines.CancellableContinuation
internal class BannerDataImpl(
private val continuation: CancellableContinuation<Unit>,
override val message: BannerMessage,
+ private val dismissed: AtomicBoolean = AtomicBoolean(false),
) : BannerData {
override fun dismiss() {
- if (continuation.isActive) continuation.resume(Unit)
+ if (dismissed.compareAndSet(false, true)) {
+ continuation.resume(Unit)
+ }
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| internal class BannerDataImpl( | |
| private val continuation: CancellableContinuation<Unit>, | |
| override val message: BannerMessage, | |
| ) : BannerData { | |
| override fun dismiss() { | |
| if (continuation.isActive) continuation.resume(Unit) | |
| } | |
| internal class BannerDataImpl( | |
| private val continuation: CancellableContinuation<Unit>, | |
| override val message: BannerMessage, | |
| private val dismissed: AtomicBoolean = AtomicBoolean(false), | |
| ) : BannerData { | |
| override fun dismiss() { | |
| if (dismissed.compareAndSet(false, true)) { | |
| continuation.resume(Unit) | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/banner/BannerData.kt`
around lines 14 - 21, Replace the fragile isActive check in
BannerDataImpl.dismiss with an idempotent AtomicBoolean guard: add a private
AtomicBoolean (e.g., dismissed) to BannerDataImpl and in dismiss call
dismissed.compareAndSet(false, true) and only if it returns true attempt to
resume the continuation (optionally still check continuation.isActive before
calling continuation.resume(Unit)); this ensures a single resume without using
internal coroutines APIs like tryResume/completeResume.
| @Composable | ||
| private fun BannerRender(bannerData: BannerData) { | ||
| when (val message = bannerData.message) { | ||
| is InfoBanner -> InlineSuccessBanner(text = message.text) | ||
| is WarningBanner -> InlineWarningBanner(text = message.text) | ||
| is ErrorBanner -> InlineErrorBanner(text = message.text) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the imports in the file to understand the Jewel version and available components
grep -n "import.*Jewel\|import.*Inline.*Banner" tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/banner/BannerHost.ktRepository: ComposeGears/Valkyrie
Length of output: 251
🌐 Web query:
JetBrains Jewel InlineBanner components types InlineSuccessBanner InlineWarningBanner InlineErrorBanner InlineInfoBanner documentation
💡 Result:
JetBrains doesn’t currently publish separate, browsable “component docs” pages for Jewel composables like InlineBanner, InlineSuccessBanner, InlineWarningBanner, InlineErrorBanner, and InlineInfoBanner. The primary “documentation” for these APIs is their KDoc in the source.
Where to look:
- Current source of truth (active development): Jewel was moved into the IntelliJ Platform repo under
platform/jewelinJetBrains/intellij-community. [1] - Legacy/mirrored repo (archived/read-only):
JetBrains/jewelis archived and points to the new location above. [1] - In the source tree, these composables live in the styled UI components (the
uimodule in Jewel’s structure). [1]
If you need the exact signatures/parameters and usage examples, the reliable approach is:
- open
JetBrains/intellij-community, navigate toplatform/jewel, then use GitHub search (or IDE “Find in Files”) forInlineBanner/InlineSuccessBannerto read their KDoc. [1][2]
Sources:
[1] https://github.com/JetBrains/jewel (README: project moved; module structure)
[2] https://github.com/JetBrains/intellij-community (repo containing platform/jewel)
🏁 Script executed:
# Search for all InlineBanner imports/usages across the codebase
rg "InlineInfoBanner|InlineSuccessBanner|InlineWarningBanner|InlineErrorBanner" --type kotlin -A 1 -B 1Repository: ComposeGears/Valkyrie
Length of output: 1526
🏁 Script executed:
# Check gradle files for Jewel version
find . -name "*.gradle" -o -name "build.gradle.kts" | head -10 | xargs grep -l "jewel" 2>/dev/null | head -5Repository: ComposeGears/Valkyrie
Length of output: 47
🏁 Script executed:
# Check for any gradle build file with Jewel dependency to see the version
fd "build.gradle.kts|build.gradle" | xargs grep -A 2 -B 2 "jewel" 2>/dev/null | head -30Repository: ComposeGears/Valkyrie
Length of output: 452
🏁 Script executed:
# Search more broadly in the build file for Jewel version/dependency declaration
cat tools/idea-plugin/build.gradle.kts | grep -A 5 -B 5 "jewel" -i | head -50Repository: ComposeGears/Valkyrie
Length of output: 541
🏁 Script executed:
# Also check root level gradle files
find . -maxdepth 2 -name "*.gradle.kts" -o -name "*.gradle" | xargs grep "jewel" -i 2>/dev/nullRepository: ComposeGears/Valkyrie
Length of output: 49
🏁 Script executed:
# Let's read the actual dependency declarations in the build file
cat tools/idea-plugin/build.gradle.kts | head -200Repository: ComposeGears/Valkyrie
Length of output: 5967
🏁 Script executed:
# Search for "dependencies" section
grep -n "dependencies\|jewel" tools/idea-plugin/build.gradle.kts -iRepository: ComposeGears/Valkyrie
Length of output: 258
🏁 Script executed:
# Let's check the domain model to understand the BannerMessage types
find . -name "*.kt" -type f | xargs grep -l "InfoBanner\|WarningBanner\|ErrorBanner" | grep -v build | head -5Repository: ComposeGears/Valkyrie
Length of output: 652
🏁 Script executed:
# Look at the domain model definition
cat tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/banner/BannerHost.kt | head -40Repository: ComposeGears/Valkyrie
Length of output: 1878
🏁 Script executed:
# Check the BannerMessage domain model to understand the semantics
cat tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/banner/BannerMessage.ktRepository: ComposeGears/Valkyrie
Length of output: 744
🏁 Script executed:
# Let's check if there are any other InlineBanner components or custom banners being used elsewhere
rg "InlineBanner|Inline.*Banner" tools/idea-plugin/src/main/kotlin -t kotlin --no-headingRepository: ComposeGears/Valkyrie
Length of output: 1000
🏁 Script executed:
# Check if there's any custom implementation or wrapper for info banner
rg "fun.*[Ii]nfo.*[Bb]anner|class.*[Ii]nfo.*[Bb]anner" tools/idea-plugin/src/main/kotlin -t kotlinRepository: ComposeGears/Valkyrie
Length of output: 187
InfoBanner mapped to InlineSuccessBanner lacks semantic alignment.
Mapping InfoBanner to InlineSuccessBanner creates a semantic mismatch: "Info" typically represents neutral informational content, while "Success" conveys positive outcomes. However, InlineInfoBanner does not exist in the current Jewel library (only InlineSuccessBanner, InlineWarningBanner, and InlineErrorBanner are available).
Consider either:
- Checking if a newer Jewel version provides
InlineInfoBanner - Creating a custom
InlineInfoBannerwrapper component - Explicitly documenting this mapping as a deliberate fallback
🤖 Prompt for AI Agents
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/banner/BannerHost.kt`
around lines 75 - 82, BannerRender maps InfoBanner to InlineSuccessBanner
causing semantic mismatch; update the code to either (A) detect if the Jewel
dependency includes InlineInfoBanner and use it instead of InlineSuccessBanner,
or (B) add a new composable InlineInfoBanner wrapper (e.g., implement
InlineInfoBanner that accepts text and renders a neutral-styled banner using
existing primitives) and replace the mapping in BannerRender (BannerData ->
InfoBanner -> InlineInfoBanner), or (C) add an inline comment/docstring near
BannerRender/BannerData explicitly documenting that InfoBanner intentionally
falls back to InlineSuccessBanner if no InlineInfoBanner exists; choose one
approach and implement it so InfoBanner no longer semantically maps silently to
a “success” banner.
| fun OutlineIconButton( | ||
| key: IconKey, | ||
| onClick: () -> Unit, | ||
| modifier: Modifier = Modifier, | ||
| contentDescription: String? = null, | ||
| ) { | ||
| val size = JewelTheme.outlinedButtonStyle.metrics.minSize.height | ||
| val style = ButtonStyle( | ||
| colors = JewelTheme.outlinedButtonStyle.colors, | ||
| focusOutlineAlignment = JewelTheme.outlinedButtonStyle.focusOutlineAlignment, | ||
| metrics = ButtonMetrics( | ||
| cornerSize = JewelTheme.outlinedButtonStyle.metrics.cornerSize, | ||
| padding = PaddingValues(0.dp), | ||
| minSize = DpSize(size, size), | ||
| borderWidth = JewelTheme.outlinedButtonStyle.metrics.borderWidth, | ||
| focusOutlineExpand = JewelTheme.outlinedButtonStyle.metrics.focusOutlineExpand, | ||
| ), | ||
| ) | ||
|
|
||
| OutlinedButton( | ||
| modifier = modifier, | ||
| style = style, | ||
| onClick = onClick, | ||
| ) { | ||
| Icon( | ||
| key = key, | ||
| contentDescription = contentDescription, | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "OutlineIconButton.kt" --type fRepository: ComposeGears/Valkyrie
Length of output: 164
🌐 Web query:
Jetpack Compose icon-only button accessibility contentDescription best practices
💡 Result:
Best practices for icon-only buttons (Jetpack Compose)
- Use
IconButton(or Material3 variants) for icon-only actions; it provides proper click semantics and a 48dp touch target by default. (developer.android.com) - Always provide an accessible label via
contentDescriptionon theIconinside theIconButton, using a localized string (typicallystringResource(...)). (developer.android.com) - Set
contentDescription = nullonly when the icon is purely decorative and not conveying meaning or acting as the only label for a control. (developer.android.com) - Rely on semantics merging: for
IconButton { Icon(...) }, the icon’scontentDescriptionis merged into the button’s semantics so TalkBack can announce it as a labeled button. (android.googlesource.com) - If you build a custom clickable icon (e.g.,
Modifier.clickableon anIcon), ensure you still provide a label (contentDescription) and consider addingonClickLabelfor clearer action announcement. (developer.android.com)
Recommended pattern
IconButton(onClick = onClose) {
Icon(
imageVector = Icons.Default.Close,
contentDescription = stringResource(R.string.cd_close)
)
}
``` ([developer.android.com](https://developer.android.com/codelabs/jetpack-compose-accessibility?utm_source=openai))
Citations:
- 1: https://developer.android.com/codelabs/jetpack-compose-accessibility?utm_source=openai
- 2: https://developer.android.com/develop/ui/compose/accessibility/api-defaults?utm_source=openai
- 3: https://developer.android.com/codelabs/jetpack-compose-accessibility?utm_source=openai
- 4: https://android.googlesource.com/platform/frameworks/support/%2B/androidx-main/compose/docs/compose-component-api-guidelines.md?utm_source=openai
- 5: https://developer.android.com/codelabs/jetpack-compose-accessibility?utm_source=openai
- 6: https://developer.android.com/codelabs/jetpack-compose-accessibility?utm_source=openai
---
🏁 Script executed:
```shell
cat -n "tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/button/OutlineIconButton.kt"Repository: ComposeGears/Valkyrie
Length of output: 2517
Require an accessibility label for icon-only button.
contentDescription defaults to null, allowing callers to omit it and produce an unlabeled button that screen readers cannot announce. Per Jetpack Compose accessibility guidelines, icon-only buttons must always have a contentDescription (unless the icon is purely decorative, which is not the case here). Use a localized string via stringResource(...) rather than hardcoded text.
Update the function signature to require contentDescription: String and provide it in the preview:
♿ Proposed fix
fun OutlineIconButton(
key: IconKey,
onClick: () -> Unit,
modifier: Modifier = Modifier,
- contentDescription: String? = null,
+ contentDescription: String,
) { `@Preview`
`@Composable`
private fun OutlineIconButtonPreview() = PreviewTheme(alignment = Alignment.Center) {
OutlineIconButton(
key = AllIconsKeys.General.Close,
onClick = {},
+ contentDescription = "Close",
)
}🤖 Prompt for AI Agents
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/button/OutlineIconButton.kt`
around lines 21 - 48, Change OutlineIconButton to require a non-null
contentDescription: String (remove default null) so icon-only buttons are always
accessible; update all call sites and previews to pass a localized label using
stringResource(...) (do not use hardcoded text) and ensure any preview uses a
sample stringResource key. Locate the OutlineIconButton(...) function and its
preview usages and replace nullable contentDescription handling with the
required String parameter, and use stringResource(...) when invoking the
composable.
| BasicTextField( | ||
| modifier = Modifier | ||
| .fillMaxHeight() | ||
| .width(32.dp) | ||
| .verticalScroll(linesTextScroll), | ||
| value = IntRange(1, linesCount).joinToString(separator = "\n"), | ||
| readOnly = true, | ||
| textStyle = textStyle.copy( | ||
| color = JewelTheme.globalColors.text.disabled, | ||
| textAlign = TextAlign.End, | ||
| ), | ||
| onValueChange = {}, | ||
| ) | ||
| Spacer(8.dp) |
There was a problem hiding this comment.
Fixed line number width may truncate for large files.
The line number gutter uses a fixed width of 32.dp (line 71). For files with 1000+ lines (4+ digits), this may truncate the line numbers. The existing SDK CodeEditor uses a dynamic width calculation: 12.dp * linesCount.toString().length.
Suggested fix for dynamic line number width
BasicTextField(
modifier = Modifier
.fillMaxHeight()
- .width(32.dp)
+ .width(12.dp * linesCount.toString().length)
.verticalScroll(linesTextScroll),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| BasicTextField( | |
| modifier = Modifier | |
| .fillMaxHeight() | |
| .width(32.dp) | |
| .verticalScroll(linesTextScroll), | |
| value = IntRange(1, linesCount).joinToString(separator = "\n"), | |
| readOnly = true, | |
| textStyle = textStyle.copy( | |
| color = JewelTheme.globalColors.text.disabled, | |
| textAlign = TextAlign.End, | |
| ), | |
| onValueChange = {}, | |
| ) | |
| Spacer(8.dp) | |
| BasicTextField( | |
| modifier = Modifier | |
| .fillMaxHeight() | |
| .width(12.dp * linesCount.toString().length) | |
| .verticalScroll(linesTextScroll), | |
| value = IntRange(1, linesCount).joinToString(separator = "\n"), | |
| readOnly = true, | |
| textStyle = textStyle.copy( | |
| color = JewelTheme.globalColors.text.disabled, | |
| textAlign = TextAlign.End, | |
| ), | |
| onValueChange = {}, | |
| ) | |
| Spacer(8.dp) |
🤖 Prompt for AI Agents
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/editor/CodeEditor.kt`
around lines 68 - 81, The gutter width is fixed at 32.dp in the CodeEditor
BasicTextField which will truncate multi‑digit line numbers; change the
Modifier.width(...) to compute width dynamically based on the number of digits
in linesCount (e.g. multiply a per‑digit dp value by
linesCount.toString().length) so the BasicTextField showing IntRange(1,
linesCount) can expand for large files; update the Modifier on the
BasicTextField in CodeEditor to use that dynamic width calculation and keep
existing verticalScroll(linesTextScroll) and textStyle settings.
There was a problem hiding this comment.
i expect only small files
There was a problem hiding this comment.
@egorikftp, understood! If the component is designed for small files only, the fixed 32.dp width is perfectly fine and keeps the implementation simpler. Thank you for the clarification.
🧠 Learnings used
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 807
File: tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/imagevectortoxml/conversion/ui/XmlCodeViewer.kt:19-22
Timestamp: 2026-01-13T10:00:32.135Z
Learning: The Highlights library used in the codebase does not support XML syntax highlighting. When displaying XML content (e.g., in XmlCodeViewer), Kotlin syntax highlighting is used as a fallback workaround.
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 801
File: gradle/libs.versions.toml:14-14
Timestamp: 2026-01-01T18:09:41.901Z
Learning: In the Valkyrie project (ComposeGears/Valkyrie), compose-ui-tooling-preview must use version 1.10.0 (even though runtime Compose is 1.8.2) because 1.10.0+ provides unified KMP preview annotation support required for IntelliJ IDEA 2025.3+ preview functionality.
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 805
File: tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/uikit/RadioButtonTooltipRow.kt:18-18
Timestamp: 2026-01-15T08:37:06.618Z
Learning: In JetBrains Jewel UI library, some components (such as Tooltip and others) are marked as experimental and require `OptIn(ExperimentalFoundationApi::class)`. Do not flag this annotation as unnecessary when used with Jewel components in the ComposeGears/Valkyrie project.
| icon?.let { | ||
| Icon( | ||
| modifier = Modifier | ||
| .align(Alignment.Top) | ||
| .size(36.dp), | ||
| imageVector = it, | ||
| contentDescription = null, | ||
| tint = JewelTheme.contentColor, | ||
| ) | ||
| } | ||
| image?.let { | ||
| Image( | ||
| modifier = Modifier | ||
| .align(Alignment.Top) | ||
| .size(36.dp), | ||
| imageVector = it, | ||
| contentDescription = null, | ||
| ) | ||
| } |
There was a problem hiding this comment.
Both icon and image will render if both are provided.
If a caller provides both icon and image parameters, both will be displayed side-by-side. This is likely unintended behavior. Consider either:
- Making them mutually exclusive with a sealed class parameter
- Prioritizing one over the other (e.g.,
icontakes precedence) - Documenting this as intentional behavior
Option 2: Prioritize icon over image
- icon?.let {
+ val visual = icon ?: image
+ visual?.let {
Icon(
modifier = Modifier
.align(Alignment.Top)
.size(36.dp),
imageVector = it,
contentDescription = null,
tint = JewelTheme.contentColor,
)
}
- image?.let {
- Image(
- modifier = Modifier
- .align(Alignment.Top)
- .size(36.dp),
- imageVector = it,
- contentDescription = null,
- )
- }Note: This approach loses the distinction between tinted Icon and untinted Image. A sealed class approach would preserve this distinction while enforcing mutual exclusivity.
🤖 Prompt for AI Agents
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/InfoCard.kt`
around lines 41 - 59, The current InfoCard Composable renders both Icon and
Image when both parameters are provided (icon and image), causing unintended
double visuals; update the rendering logic in InfoCard.kt so they are mutually
exclusive by prioritizing icon: check icon first and render Icon(...) only if
non-null, otherwise render Image(...) when image is non-null (i.e., replace the
two independent icon?.let and image?.let blocks with a single conditional that
prefers icon over image).
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/LinkIcon.kt
Show resolved
Hide resolved
| .onKeyEvent { | ||
| when (it.key) { | ||
| Key.Escape -> { | ||
| focusManager.clearFocus() | ||
| state.setTextAndPlaceCursorAtEnd(text) | ||
| true | ||
| } | ||
| Key.Enter -> { | ||
| if (!isError) { | ||
| focusManager.clearFocus() | ||
| onValueChange(state.text.toString()) | ||
| true | ||
| } else { | ||
| false | ||
| } | ||
| } | ||
| else -> false | ||
| } | ||
| }, |
There was a problem hiding this comment.
Stale closure: Escape key may restore outdated text.
On line 55, pressing Escape restores text from the parameter, but this captures the value at composition time. If the parent subsequently updates the text prop, Escape will restore the stale original value rather than the current parent value.
Use rememberUpdatedState to capture the latest value:
Proposed fix
`@Composable`
fun ConfirmTextField(
text: String,
errorPlaceholder: String,
modifier: Modifier = Modifier,
onValueChange: (String) -> Unit,
) {
val state = rememberTextFieldState(text)
val focusManager = LocalFocusManager.current
+ val currentText by rememberUpdatedState(text)
val isError by remember { derivedStateOf { state.text.isEmpty() } }
var focused by rememberMutableState { false }
TextField(
modifier = modifier
.heightIn(min = 32.dp)
.onFocusChanged {
focused = it.isFocused
}
.onKeyEvent {
when (it.key) {
Key.Escape -> {
focusManager.clearFocus()
- state.setTextAndPlaceCursorAtEnd(text)
+ state.setTextAndPlaceCursorAtEnd(currentText)
true
}🤖 Prompt for AI Agents
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/textfield/ConfirmTextField.kt`
around lines 51 - 69, The Escape handler in ConfirmTextField's onKeyEvent uses
the captured parameter text (stale closure) when calling
state.setTextAndPlaceCursorAtEnd(text); change this to read the latest parent
value via rememberUpdatedState so the Escape branch restores the current prop
instead of the composition-time value — add a rememberUpdatedState(text) (e.g.,
latestText) near the top of the composable and replace uses of the captured text
in the onKeyEvent Escape branch with that latestText variable.
...n/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/common/ui/SidePanel.kt
Show resolved
Hide resolved
egorikftp
force-pushed
the
feature/idea/jewel-migration
branch
2 times, most recently
from
2bdaa80 to
07b859b
Compare
…eName, reduce plugin size
egorikftp
force-pushed
the
feature/idea/jewel-migration
branch
from
07b859b to
68ed8a1
Compare
Uh oh!
There was an error while loading. Please reload this page.