-
-
Notifications
You must be signed in to change notification settings - Fork 381
Refactor ActionKeywords UI and improve keyword handling #3702
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
Updated `ActionKeywords.xaml` to use a new `Grid` layout, enhancing the structure and flexibility of the UI. Removed a couple of layers of nested grids and stackpanels. Changed tbOldActionKeyword from a TextBlock to a TextBox so user can copy the text. Modified `ActionKeywords.xaml.cs` so the old keywords are already filled in, in the TextBox.
This comment has been minimized.
This comment has been minimized.
📝 WalkthroughWalkthroughThe changes restructure the XAML layout for the ActionKeywords window to use a simplified grid, improving element placement and alignment. The associated C# code refines the logic for handling and validating action keyword changes, streamlining conflict detection, comparison, and updates using more concise LINQ operations. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ActionKeywordsWindow
participant KeywordManager
User->>ActionKeywordsWindow: Open window
ActionKeywordsWindow->>ActionKeywordsWindow: Initialize textbox with current keywords
User->>ActionKeywordsWindow: Edit and confirm new keywords
ActionKeywordsWindow->>ActionKeywordsWindow: Filter and deduplicate keywords
ActionKeywordsWindow->>KeywordManager: Check for keyword conflicts
alt Conflict found
ActionKeywordsWindow->>User: Show conflict message
else No conflict
alt Keyword count differs
ActionKeywordsWindow->>KeywordManager: Replace keywords
else
ActionKeywordsWindow->>ActionKeywordsWindow: Compare sorted keywords
alt Identical
ActionKeywordsWindow->>User: Show sequence changed message
else
ActionKeywordsWindow->>KeywordManager: Replace keywords
end
end
end
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
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. 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: 2
🧹 Nitpick comments (3)
Flow.Launcher/ActionKeywords.xaml (2)
32-55
: Add an accessibility label to the Close buttonScreen-reader users have no semantic hint for this control because the
Button
lacks anAutomationProperties.Name
(orToolTip
). Adding it is a quick win that does not affect visuals.<Button Grid.Row="0" Grid.Column="1" HorizontalAlignment="Right" Click="BtnCancel_OnClick" Style="{StaticResource TitleBarCloseButtonStyle}" + AutomationProperties.Name="{DynamicResource close}" <!-- or a hard-coded "Close" --> >
75-93
: Prevent the read-only textbox from stealing keyboard focus
tbOldActionKeyword
is meant for copy-only scenarios. Leaving it focusable puts an unnecessary element in the tab order.<TextBox x:Name="tbOldActionKeyword" @@ Foreground="{DynamicResource Color05B}" IsReadOnly="True" + IsTabStop="False" Text="List of old keyword(s)" />
Flow.Launcher/ActionKeywords.xaml.cs (1)
23-27
: CallFocus()
beforeSelectAll()
to guarantee full selection
SelectAll
does nothing when the control is not focused in some WPF themes. Swapping the calls is safer.- tbAction.SelectAll(); - tbAction.Focus(); + tbAction.Focus(); + tbAction.SelectAll();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Flow.Launcher/ActionKeywords.xaml
(2 hunks)Flow.Launcher/ActionKeywords.xaml.cs
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Report (PR)
- GitHub Check: build
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
I’ve made some minor adjustments to the margins, and also modified the button sizes — something I had been meaning to fix anyway, so I included it in this PR. Both the design and functionality look fine. I only checked the UI side; @Jack251970 will review the code part. It probably won’t be merged immediately due to the ongoing hotfix work. For future similar PRs, it would be helpful if you could include a screenshot. Thanks a lot~❤️ |
@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 ...
|
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.
LGTM👍
Let us hold it until 1.20.1 release |
@stefanroelofs Thanks for your contribution! |
What's the PR
Updated
ActionKeywords.xaml
to use a newGrid
layout, enhancing the structure and flexibility of the UI. Removed a couple of layers of nested grids and stackpanels. Changed tbOldActionKeyword from a TextBlock to a TextBox so user can copy the text. ModifiedActionKeywords.xaml.cs
so the old keywords are already filled in, in the TextBox.Before
After
Review