DYN-9063: New ui preferences toggle#16310
Conversation
There was a problem hiding this comment.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-9063
|
Can you add some tests? |
There was a problem hiding this comment.
Pull Request Overview
Adds a user preference to enable a new floating UI for node autocomplete and wires it through settings, UI, commands, and public APIs.
- Introduce
MLEnableNewUIinPreferenceSettings(with defaulttrue) and expose it via public API. - Add a toggle in the Preferences dialog (XAML + view model + resources) for the new UI.
- Update command and extension code to route autocomplete execution based on the new setting.
Reviewed Changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/DynamoCore/Configuration/PreferenceSettings.cs | Add MLEnableNewUI property with default initialization |
| src/DynamoCoreTests/Configuration/PreferenceSettingsTests.cs | Assert new setting is included in settings serialization |
| src/DynamoCoreWpf/Views/Menu/PreferencesView.xaml | Insert toggle button and tooltip for “Enable New UI” |
| src/DynamoCoreWpf/ViewModels/Menu/PreferencesViewModel.cs | Add MLEnableNewUIIsChecked VM property and property-change case |
| src/DynamoCoreWpf/Commands/PortCommands.cs | Use MLEnableNewUI to choose between cluster and legacy UI |
| src/NodeAutoCompleteViewExtension/NodeAutoCompleteViewExtension.cs | Remove original gating without replacing it with the new toggle |
| src/DynamoCoreWpf/Properties/Resources.resx | Add string and tooltip resources |
| src/DynamoCoreWpf/Properties/Resources.en-US.resx | Mirror new resources (remove extraneous entry) |
| src/DynamoCoreWpf/PublicAPI.Unshipped.txt | Expose new VM property getters/setters |
| src/DynamoCore/PublicAPI.Unshipped.txt | Expose new PreferenceSettings.MLEnableNewUI get/set |
Files not reviewed (1)
- src/DynamoCoreWpf/Properties/Resources.Designer.cs: Language not supported
Comments suppressed due to low confidence (2)
src/DynamoCoreWpf/Properties/Resources.en-US.resx:4166
- The resource entry named
Stringappears extraneous and may conflict with other keys. It looks unintended—please remove this redundant<data>element.
<data name="String" xml:space="preserve">
src/DynamoCoreWpf/Commands/PortCommands.cs:35
- [nitpick] Consider adding unit tests for
NodeAutoCompleteCommandto verify that it invokesAutoCompleteClusterorAutoCompletecorrectly based on theMLEnableNewUIsetting.
autoCompleteCommand ??= new DelegateCommand(
johnpierson
left a comment
There was a problem hiding this comment.
The toggle was already added in this PR: #16284
For 9063, lets rebase your branch from master, and only do the disabling (greying out) of the preferences items.
| var dynamoViewModel = viewLoadedParams.DynamoWindow.DataContext as DynamoViewModel; | ||
|
|
||
| if (dynamoViewModel.IsDNAClusterPlacementEnabled) | ||
| DependencyView = new NodeAutoCompleteView(this, viewLoadedParams); |
There was a problem hiding this comment.
So the side panel only makes sense for the cluster so I guess it should continue to be dependent on IsDNAClusterPlacementEnabled ?
If so then I guess we should hide the button in the bar that makes the side panel visible as well.
| /// <summary> | ||
| /// Enable New UI for cluster node autocomplete | ||
| /// </summary> | ||
| public bool MLEnableNewUI { get; set; } |
There was a problem hiding this comment.
Ideally we should not add anything to the api related to this. This new button will be obsolete once the new experience becomes the default one and we retire the other one.
|
I made some comments but otherwise is working as expected. Let's clarify that and then LGTM! |
Purpose
Add a toggle to use the new UI.
2025-06-18.01-38-09.mp4
Declarations
Check these if you believe they are true
*.resxfilesRelease Notes
Add a toggle to use the new UI.
Reviewers
@johnpierson
@BogdanZavu
FYIs
(FILL ME IN, Optional) Names of anyone else you wish to be notified of