Improve JsonRPC Plugin Setting Panel #3979
Conversation
|
🥷 Code experts: no user but you matched threshold 10 Jack251970 has most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame: ✨ Comment |
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
There was a problem hiding this comment.
Pull Request Overview
This PR improves the JsonRPC Plugin Setting Panel layout by implementing column width constraints and consistent text wrapping. The changes ensure better visual formatting and readability of plugin settings.
- Constrains column 0 to 60% of the maximum grid width to prevent excessive horizontal expansion
- Changes all text wrapping from
WrapWithOverflowtoWrapfor consistent text behavior - Adds dynamic layout handling through a
SizeChangedevent handler
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Warning Rate limit exceeded@Jack251970 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 31 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughUpdated JsonRPCPluginSettings to constrain the settings panel’s left column width, switch text elements to TextWrapping.Wrap, and add a SizeChanged handler that recalculates column and child MaxWidth based on available width. Public APIs remain unchanged. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsPanel as Settings Panel (mainPanel)
participant Handler as MainPanel_SizeChanged
participant GridCol0 as Grid Column 0
participant Children as Column 0 Children
User->>SettingsPanel: Resize window/panel
SettingsPanel-->>Handler: SizeChanged event
Handler->>Handler: Compute workingWidth = ActualWidth - VScrollBarWidth
alt workingWidth > 0
Handler->>GridCol0: Set MaxWidth = 0.6 * workingWidth
Handler->>Children: For single-column-span items, set MaxWidth to column 0 MaxWidth
else
Handler->>GridCol0: No change
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (1)
Flow.Launcher.Core/Plugin/JsonRPCPluginSettings.cs (1)
204-216: Label text may still not wrap due to StackPanel measure; propagate MaxWidth to children.In a vertical StackPanel, children are measured with infinite width, so TextBlocks won’t wrap even with TextWrapping unless their own width is constrained. Your handler sets MaxWidth on the column-0 container (the StackPanel), but not on its children. Also, avoid int-casting and subtracting scrollbar width (ScrollViewer usually already constrains ActualWidth).
Apply this diff to fix both points:
@@ - var workingWidth = - (int)(grid.ActualWidth - SystemParameters.VerticalScrollBarWidth); // take into account vertical scrollbar + // Use the actual available width; ScrollViewer's viewport already excludes the vertical scrollbar. + var workingWidth = grid.ActualWidth; @@ - var constrainedWidth = MainGridColumn0MaxWidthRatio * workingWidth; + var constrainedWidth = MainGridColumn0MaxWidthRatio * workingWidth; @@ - grid.ColumnDefinitions[0].MaxWidth = constrainedWidth; - foreach (var child in grid.Children) - { - if (child is FrameworkElement element && Grid.GetColumn(element) == 0 && Grid.GetColumnSpan(element) == 1) - { - element.MaxWidth = constrainedWidth; - } - } + grid.ColumnDefinitions[0].MaxWidth = constrainedWidth; + foreach (var child in grid.Children) + { + if (child is FrameworkElement element && Grid.GetColumn(element) == 0 && Grid.GetColumnSpan(element) == 1) + { + element.MaxWidth = constrainedWidth; + // Important: StackPanel gives infinite width to children in the non‑stacking dimension. + // Constrain descendants so TextBlocks actually wrap. + if (element is Panel p) PropagateMaxWidthToChildren(p, constrainedWidth); + } + } + + static void PropagateMaxWidthToChildren(Panel panel, double width) + { + foreach (var c in panel.Children) + { + if (c is FrameworkElement fe) + { + fe.MaxWidth = width; + if (fe is Panel nested) PropagateMaxWidthToChildren(nested, width); + } + } + }Also applies to: 503-524
♻️ Duplicate comments (1)
Flow.Launcher.Core/Plugin/JsonRPCPluginSettings.cs (1)
494-494: Event handler detachment concern previously discussed.This was flagged before; author marked “No need”. Given the panel and handler share the same lifetime via the returned UserControl, acceptable to proceed.
🧹 Nitpick comments (2)
Flow.Launcher.Core/Plugin/JsonRPCPluginSettings.cs (2)
274-276: Don’t wrap file/folder path inputs.Paths are easier to scan and edit as single‑line. Suggest disabling wrapping here.
Apply this diff:
- ToolTip = attributes.Description, - TextWrapping = TextWrapping.Wrap + ToolTip = attributes.Description, + TextWrapping = TextWrapping.NoWrap
251-253: Check intent: TextBox wrapping without AcceptsReturn is typically a no‑op.If you want multi‑line behavior for generic “input”, set AcceptsReturn = true (and adjust height). Otherwise, consider dropping TextWrapping to avoid confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher.Core/Plugin/JsonRPCPluginSettings.cs(8 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3593
File: Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml.cs:143-165
Timestamp: 2025-09-13T06:04:26.965Z
Learning: In Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml, the ListView_SizeChanged handler in the C# file assumes 5 GridViewColumns but the XAML may have more columns. The user Jack251970 indicated this mismatch doesn't need to be fixed when pointed out during PR #3593 review.
📚 Learning: 2025-09-13T06:04:26.965Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3593
File: Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml.cs:143-165
Timestamp: 2025-09-13T06:04:26.965Z
Learning: In Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml, the ListView_SizeChanged handler in the C# file assumes 5 GridViewColumns but the XAML may have more columns. The user Jack251970 indicated this mismatch doesn't need to be fixed when pointed out during PR #3593 review.
Applied to files:
Flow.Launcher.Core/Plugin/JsonRPCPluginSettings.cs
⏰ 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). (3)
- GitHub Check: gitStream.cm
- GitHub Check: build
- GitHub Check: Check Spelling
🔇 Additional comments (3)
Flow.Launcher.Core/Plugin/JsonRPCPluginSettings.cs (3)
30-30: Good: extracted width ratio constant.Using a named constant improves clarity and makes future tweaks straightforward.
160-169: Grid column setup looks right.Auto for labels + Star for inputs is appropriate for this layout.
339-343: Textarea change looks good.TextWrapping + AcceptsReturn is correct for multi‑line inputs.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
…setting Improve JsonRPC Plugin Setting Panel
Improve JsonRPC Plugin Setting Panel
Improve JsonRPC Plugin Setting Panel
Test