Add maximum results shown warning when always preview is on #3936
Conversation
|
🥷 Code experts: onesounds onesounds, Jack251970 have most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame:
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 adds a warning message to inform users that the "maximum results shown" setting may not work properly when "Always Preview" is enabled, since the preview panel requires a minimum height that could override the results limit.
- Adds an InfoBar warning component that displays when Always Preview is enabled
- Introduces a new localized warning message explaining the interaction between these two settings
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Flow.Launcher/SettingPages/Views/SettingsPaneTheme.xaml | Adds InfoBar warning component with visibility binding to AlwaysPreview setting |
| Flow.Launcher/Languages/en.xaml | Adds localized warning message for the maximum results/always preview interaction |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
📝 WalkthroughWalkthroughAdds a localized warning string and displays an InfoBar in Theme settings. The InfoBar warns that “Max show results” may not apply when “Always Preview” is enabled. Visibility is bound to the AlwaysPreview setting via a BoolToVisibilityConverter. No logic changes beyond XAML bindings/resources. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant SettingsUI as SettingsPaneTheme.xaml
participant Settings as Settings.AlwaysPreview
participant Converter as BoolToVisibilityConverter
participant InfoBar
User->>SettingsUI: Toggle "Always Preview"
SettingsUI->>Settings: Update AlwaysPreview value
SettingsUI->>Converter: Convert(AlwaysPreview)
Converter-->>SettingsUI: Visibility (Visible/Collapsed)
SettingsUI->>InfoBar: Set Visibility, Bind Message (resource)
Note over InfoBar: Shows warning when Always Preview = true
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Nitpick comments (2)
Flow.Launcher/Languages/en.xaml (1)
330-330: Polish the copy for clarity and consistency with setting labels.Current text is slightly awkward and missing articles. Suggested rewording adds punctuation and references the setting label verbatim.
Apply this diff:
- <system:String x:Key="MaxShowResultsCannotWorkWithAlwaysPreview">Since Always Preview is on, maximum results shown may not take effect because preview panel requires a certain minimum height</system:String> + <system:String x:Key="MaxShowResultsCannotWorkWithAlwaysPreview">When Always Preview is enabled, Maximum results shown may not apply because the preview panel requires a minimum height.</system:String>Flow.Launcher/SettingPages/Views/SettingsPaneTheme.xaml (1)
548-556: Remove redundant attributes and empty Title.Title="" renders no value and can be dropped; UpdateSourceTrigger is meaningless for OneWay bindings; Mode=OneWay is default. Simplify.
Apply this diff:
- <cc:InfoBar - Title="" - Margin="0 4 0 0" - Closable="False" - IsIconVisible="True" - Length="Long" - Message="{DynamicResource MaxShowResultsCannotWorkWithAlwaysPreview}" - Type="Warning" - Visibility="{Binding Settings.AlwaysPreview, Converter={StaticResource BoolToVisibilityConverter}, Mode=OneWay, UpdateSourceTrigger=PropertyChanged}" /> + <cc:InfoBar + Margin="0 4 0 0" + Closable="False" + IsIconVisible="True" + Length="Long" + Message="{DynamicResource MaxShowResultsCannotWorkWithAlwaysPreview}" + Type="Warning" + Visibility="{Binding Settings.AlwaysPreview, Converter={StaticResource BoolToVisibilityConverter}}" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
Flow.Launcher/Languages/en.xaml(1 hunks)Flow.Launcher/SettingPages/Views/SettingsPaneTheme.xaml(1 hunks)
⏰ 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: gitStream.cm
- GitHub Check: build
🔇 Additional comments (3)
Flow.Launcher/Languages/en.xaml (1)
330-330: All locales includeMaxShowResultsCannotWorkWithAlwaysPreview; no additional translations needed.Flow.Launcher/SettingPages/Views/SettingsPaneTheme.xaml (2)
548-556: Good UX addition—clear, contextual warning.
548-556: (Optional) Inspect HideableVisibilityConverter to confirm if it implements multi-boolean AND logic; if not, no existing AND converter—keep single binding.
…ew_notification Add maximum results shown warning when always preview is on
Add maximum results shown warning when always preview is on
Since preview panel requires a certain minimum height, maximum results shown may not take effect. So we need to add a warning to notify users for that. (I even thought this is a bug originally😢)
Test
When we enable the Always Preview and set a small number for maximum results shown, it will not take effect since preview panel requires a minimum height.