Skip to content

[IDEA Plugin] [Simple mode] Add Settings button on picker state#779

Merged
egorikftp merged 1 commit intomainfrom
feature/idea/add-settings-buttin
Dec 13, 2025
Merged

[IDEA Plugin] [Simple mode] Add Settings button on picker state#779
egorikftp merged 1 commit intomainfrom
feature/idea/add-settings-buttin

Conversation

@egorikftp
Copy link
Copy Markdown
Member

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Dec 12, 2025

Walkthrough

The changes introduce a new onOpenSettings: () -> Unit callback parameter to the GenericPickerScreen composable. A SettingsAction component is integrated into the TopAppBar, wired to trigger this callback when invoked. The callback is then implemented in both SimplePickerScreen and ImageVectorPickerScreen, each routing to the SettingsScreen upon activation. This enables users to access settings directly from the picker interface without additional navigation logic.

Possibly related issues

  • Issue 776: Directly implements the settings button feature by adding the onOpenSettings callback to GenericPickerScreen and integrating SettingsAction in the TopAppBar, with navigation wired through picker screens to the SettingsScreen.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding a Settings button on the picker screen in the IDEA plugin's simple mode.
Description check ✅ Passed The description references the linked issue #776 and includes a visual screenshot of the implemented feature, demonstrating the Settings button addition.
Linked Issues check ✅ Passed The pull request implements the requirement from issue #776 by adding a Settings button to the picker screen, with appropriate navigation and UI updates across multiple picker components.
Out of Scope Changes check ✅ Passed All changes are focused on adding the Settings button functionality to picker screens and navigating to the SettingsScreen, which is directly aligned with the stated objective.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/idea/add-settings-buttin

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/imagevectortoxml/picker/ImageVectorPickerScreen.kt (1)

59-61: Consider guarding against duplicate Settings navigations (double-click). If navController.navigate doesn’t de-dupe, rapid clicks can stack SettingsScreen. Consider launchSingleTop/equivalent if supported by your nav API.

tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/simple/picker/SimplePickerScreen.kt (1)

57-59: Consider guarding against duplicate Settings navigations (double-click). Same note as the other picker: if the nav layer doesn’t prevent duplicates, consider a singleTop/de-dupe option.

tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/picker/GenericPickerScreen.kt (1)

55-65: Consider giving onOpenSettings a default no-op to avoid a hard API break. If you don’t strictly need this to be required, a default keeps existing call sites compiling while still enabling the feature.

 fun GenericPickerScreen(
     title: String,
     onBack: () -> Unit,
     onFilePick: (Path) -> Unit,
     onTextPaste: (String) -> Unit,
     onBrowseClick: () -> Unit,
-    onOpenSettings: () -> Unit,
+    onOpenSettings: () -> Unit = {},
     modifier: Modifier = Modifier,
     description: String? = null,
     fileFilter: (Path) -> Boolean = { true },
 ) {
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8b28c99 and 7ee3acd.

📒 Files selected for processing (3)
  • tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/picker/GenericPickerScreen.kt (4 hunks)
  • tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/imagevectortoxml/picker/ImageVectorPickerScreen.kt (2 hunks)
  • tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/simple/picker/SimplePickerScreen.kt (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/picker/GenericPickerScreen.kt (1)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/TopAppBar.kt (1)
  • SettingsAction (107-120)
⏰ 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 (5)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/imagevectortoxml/picker/ImageVectorPickerScreen.kt (1)

15-15: Import addition looks fine.

tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/simple/picker/SimplePickerScreen.kt (1)

16-16: Import addition looks fine.

tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/foundation/picker/GenericPickerScreen.kt (3)

32-33: SettingsAction integration via import is fine.


79-84: TopAppBar layout change looks good (title left, settings right). WeightSpacer() + SettingsAction(...) reads well.


181-189: Preview update is correct. Passing onOpenSettings = {} keeps previews working.

@egorikftp egorikftp merged commit 6875085 into main Dec 13, 2025
4 checks passed
@egorikftp egorikftp deleted the feature/idea/add-settings-buttin branch December 13, 2025 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[IDEA Plugin] [Simple mode] Add settings button

1 participant