[DYN-8893] Group context menu bug fix #16369
Conversation
There was a problem hiding this comment.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-8893
|
|
||
| private void GroupContextMenu_KeyDown(object sender, KeyEventArgs e) | ||
| { | ||
| if (e.Key == Key.O && Keyboard.Modifiers == System.Windows.Input.ModifierKeys.None) |
There was a problem hiding this comment.
why do we even have this condition?
There was a problem hiding this comment.
to make sure the user has pressed only "O" and not something like "Ctrl+O".
There was a problem hiding this comment.
No, I mean why pressing O while context menu is visible should ungroup, but I guess that is unrelated to this PR.
There was a problem hiding this comment.
It did not look logical to me either, but that was the previous behavior and my aim was to replicate it.
There was a problem hiding this comment.
Where did you see this before, that you tried to replicate? in the node context menu?
There was a problem hiding this comment.
The group context menu.
Open Dynamo 3.5.2 (works down to al least 3.0.0), create a group, call the group context menu, pres "o" -> the group is ungrouped
There was a problem hiding this comment.
Pull Request Overview
This PR fixes two UI issues in the group context menu:
- Disables the "O" hotkey for ungrouping when the in-canvas search bar is focused.
- Adjusts the header grid’s column span to prevent group names from being cut off.
Comments suppressed due to low confidence (1)
src/DynamoCoreWpf/Views/Core/AnnotationView.xaml.cs:744
- Consider adding a UI/unit test to verify that the 'O' key no longer triggers ungrouping when the search bar is focused.
if (searchBar != null && searchBar.IsKeyboardFocusWithin) return;
| // Add ContentPresenter | ||
| var contentPresenterFactory = new FrameworkElementFactory(typeof(ContentPresenter)); | ||
| contentPresenterFactory.SetValue(Grid.ColumnSpanProperty, 3); | ||
| contentPresenterFactory.SetValue(Grid.ColumnSpanProperty, 4); |
There was a problem hiding this comment.
[nitpick] The hard-coded column span of 4 is a bit opaque; consider defining a named constant or adding a comment to explain why the span changed from 3 to 4.
| contentPresenterFactory.SetValue(Grid.ColumnSpanProperty, 4); | |
| // Use a named constant for column span to improve readability and maintainability | |
| contentPresenterFactory.SetValue(Grid.ColumnSpanProperty, ContentPresenterColumnSpan); |
There was a problem hiding this comment.
the current line is consistent with the rest of the code
Purpose
Small PR for DYN-8893.
Declarations
Check these if you believe they are true
*.resxfilesRelease Notes
Reviewers
@DynamoDS/eidos
@jasonstratton.
@zeusongit
FYIs
@dnenov
@achintyabhat