-
-
Notifications
You must be signed in to change notification settings - Fork 445
Improve Report Window Design #3264
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This comment has been minimized.
This comment has been minimized.
📝 WalkthroughWalkthroughThis pull request applies layout modifications across several XAML files in the Flow.Launcher project. The grid structures are simplified by removing extra Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant RW as ReportWindow
participant API as App.API
U->>RW: Encounter error (triggers SetException)
RW->>API: Request translation for error text
RW->>RW: Assemble error message using translations
U->>RW: Click Cancel Button
RW->>RW: Execute BtnCancel_OnClick event
RW-->>U: Close the error report window
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Pro Plan! Simply use the command 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (7)
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this 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 (4)
Flow.Launcher/SelectBrowserWindow.xaml (1)
194-198
: Consider adding a tooltip for better UX.The radio buttons for tab/window preferences could benefit from tooltips explaining the difference between the options.
<RadioButton IsChecked="{Binding OpenInTab}" - Content="{DynamicResource defaultBrowser_newTab}"></RadioButton> + Content="{DynamicResource defaultBrowser_newTab}" + ToolTip="{DynamicResource defaultBrowser_newTab_tooltip}"></RadioButton> <RadioButton IsChecked="{Binding OpenInNewWindow, Mode=OneTime}" - Content="{DynamicResource defaultBrowser_newWindow}"></RadioButton> + Content="{DynamicResource defaultBrowser_newWindow}" + ToolTip="{DynamicResource defaultBrowser_newWindow_tooltip}"></RadioButton>Flow.Launcher/SelectFileManagerWindow.xaml (1)
114-114
: Consider using DynamicResource for consistency.The Rectangle's Fill property uses StaticResource while other UI elements use DynamicResource. Consider changing it for theme consistency.
- Fill="{StaticResource Color03B}" /> + Fill="{DynamicResource SeparatorForeground}" />Flow.Launcher/ReportWindow.xaml (1)
71-79
: Consider adding theme-aware colors to RichTextBox.The RichTextBox should also respect the dark theme.
<RichTextBox x:Name="ErrorTextbox" Grid.Row="1" Margin="10" BorderThickness="0" FontSize="14" + Background="{DynamicResource PopuBGColor}" + Foreground="{DynamicResource PopupTextColor}" HorizontalScrollBarVisibility="Auto" IsDocumentEnabled="True" VerticalScrollBarVisibility="Auto" />Flow.Launcher/ReportWindow.xaml.cs (1)
98-108
: Consider adding more runtime information.The RuntimeInfo method could include more useful debugging information.
private static string RuntimeInfo() { var info = $""" Flow Launcher {App.API.GetTranslation("reportWindow_version")}: {Constant.Version} OS {App.API.GetTranslation("reportWindow_version")}: {ExceptionFormatter.GetWindowsFullVersionFromRegistry()} IntPtr {App.API.GetTranslation("reportWindow_length")}: {IntPtr.Size} x64: {Environment.Is64BitOperatingSystem} + CLR Version: {Environment.Version} + Process Architecture: {Environment.GetEnvironmentVariable("PROCESSOR_ARCHITECTURE")} """; return info; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
Flow.Launcher/CustomQueryHotkeySetting.xaml
(1 hunks)Flow.Launcher/CustomShortcutSetting.xaml
(1 hunks)Flow.Launcher/Languages/en.xaml
(1 hunks)Flow.Launcher/PriorityChangeWindow.xaml
(1 hunks)Flow.Launcher/ReportWindow.xaml
(1 hunks)Flow.Launcher/ReportWindow.xaml.cs
(3 hunks)Flow.Launcher/SelectBrowserWindow.xaml
(1 hunks)Flow.Launcher/SelectFileManagerWindow.xaml
(1 hunks)Flow.Launcher/WelcomeWindow.xaml
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: gitStream.cm
🔇 Additional comments (16)
Flow.Launcher/PriorityChangeWindow.xaml (1)
31-37
: LGTM! Grid structure simplified.The grid layout has been streamlined by removing unnecessary columns while maintaining proper alignment and functionality.
Flow.Launcher/WelcomeWindow.xaml (1)
35-39
: LGTM! Efficient grid layout with proper alignment.The simplified column structure maintains proper alignment between the app icon, title, and close button.
Flow.Launcher/CustomQueryHotkeySetting.xaml (1)
34-37
: LGTM! Simplified grid structure with proper element alignment.The streamlined column structure maintains proper spacing and alignment while reducing layout complexity.
Flow.Launcher/CustomShortcutSetting.xaml (1)
32-35
: LGTM! Consistent grid structure with proper alignment.The simplified column structure aligns with the changes in other windows, maintaining consistency across the application.
Flow.Launcher/SelectBrowserWindow.xaml (2)
11-11
: LGTM! Proper implementation of theme support.The window correctly uses DynamicResource for background and foreground colors, ensuring proper dark theme support.
Also applies to: 13-13
35-35
: LGTM! Grid layout change maintains proper alignment.The button's Grid.Column property is correctly set to maintain proper alignment within the simplified grid structure.
Flow.Launcher/SelectFileManagerWindow.xaml (3)
11-11
: LGTM! Proper implementation of theme support.The window correctly uses DynamicResource for background and foreground colors, ensuring proper dark theme support.
Also applies to: 13-13
35-35
: LGTM! Grid layout change maintains proper alignment.The button's Grid.Column property is correctly set to maintain proper alignment within the simplified grid structure.
248-259
: LGTM! Proper validation implementation.The Done button's validation logic is well implemented, ensuring both profile name and path are provided before enabling the button.
Flow.Launcher/ReportWindow.xaml (3)
13-14
: LGTM! Dark theme support added correctly.The window now properly binds to theme-aware dynamic resources:
PopuBGColor
for backgroundPopupTextColor
for foreground
20-22
: LGTM! WindowChrome improves window styling.The WindowChrome addition provides better window styling with proper caption height and resize border thickness.
24-70
: LGTM! Grid layout is well-structured.The grid layout is organized effectively:
- Clear row definitions
- Proper column spacing
- Consistent margins and alignments
Flow.Launcher/ReportWindow.xaml.cs (3)
51-54
: LGTM! Localization support added correctly.The error messages are now properly localized using App.API.GetTranslation.
58-62
: LGTM! Error information is well-organized.The error report now includes:
- Runtime information
- Dependencies information
- Timestamp
110-118
: Verify dependency paths exist.The DependenciesInfo method should verify that the paths exist before including them in the report.
private static string DependenciesInfo() { + var pythonPath = File.Exists(Constant.PythonPath) ? Constant.PythonPath : "Not found"; + var nodePath = File.Exists(Constant.NodePath) ? Constant.NodePath : "Not found"; var info = $""" - {App.API.GetTranslation("pythonFilePath")}: {Constant.PythonPath} - {App.API.GetTranslation("nodeFilePath")}: {Constant.NodePath} + {App.API.GetTranslation("pythonFilePath")}: {pythonPath} + {App.API.GetTranslation("nodeFilePath")}: {nodePath} """; return info; }Flow.Launcher/Languages/en.xaml (1)
386-391
: LGTM! Localization strings are well-structured.The new error reporting strings are:
- Clear and user-friendly
- Properly use placeholders
- Follow existing naming conventions
@onesounds Hello, if you have time, could you please take a look? |
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. If the flagged items are 🤯 false positivesIf items relate to a ...
|
Good design. 😇 |
Remove useless columns in Xaml files
#4173361
Improve report window with dark theme & localization support
#44aa17d
Test
Original:
After: