Skip to content

Conversation

@johnpierson
Copy link
Member

@johnpierson johnpierson commented Jun 9, 2025

Purpose

This PR introduces a toggle for the new Node Autocomplete Flyout in the preferences menu and updates the underlying view models and configuration settings accordingly.

  • Added a new ToggleButton in the PreferencesView for enabling/disabling the new Node Autocomplete Flyout.
  • Introduced a new binding property (NodeAutocompleteNewFlyoutIsChecked) in the PreferencesViewModel and corresponding configuration in PreferenceSettings.
  • Updated logic in PortViewModel and PortCommands to use the new flyout flag and removed obsolete code pathways.

image

Declarations

Check these if you believe they are true

  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated
  • This PR contains no files larger than 50 MB
  • This PR introduces new feature code involve network connecting and is tested with no-network mode.

Release Notes

  • Users can now control the Node AutoComplete In-Canvas Experience.

Reviewers

@DynamoDS/synapse @Jingyi-Wen

FYIs

@Amoursol @QilongTang

@johnpierson johnpierson marked this pull request as ready for review June 10, 2025 14:21
Copilot AI review requested due to automatic review settings June 10, 2025 14:21
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a toggle for the new Node Autocomplete Flyout in the preferences menu and updates the underlying view models and configuration settings accordingly.

  • Added a new ToggleButton in the PreferencesView for enabling/disabling the new Node Autocomplete Flyout.
  • Introduced a new binding property (NodeAutocompleteNewFlyoutIsChecked) in the PreferencesViewModel and corresponding configuration in PreferenceSettings.
  • Updated logic in PortViewModel and PortCommands to use the new flyout flag and removed obsolete code pathways.

Reviewed Changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/DynamoCoreWpf/Views/Menu/PreferencesView.xaml Added a new RowDefinition and StackPanel with a new ToggleButton for the flyout feature.
src/DynamoCoreWpf/ViewModels/Menu/PreferencesViewModel.cs Added a new property to bind the new toggle state from the preference settings.
src/DynamoCoreWpf/ViewModels/Core/PortViewModel.cs Integrated conditional logic based on the new flyout flag and removed the obsolete AutoCompleteCluster method block.
src/DynamoCoreWpf/ViewModels/Core/DynamoViewModel.cs Added the IsNewDNAFlyoutEnabled property to facilitate controlling the new flyout behavior.
src/DynamoCoreWpf/PublicAPI.Unshipped.txt, src/DynamoCore/PublicAPI.Unshipped.txt Updated public API documentation to expose the new preference settings.
src/DynamoCoreWpf/Commands/PortCommands.cs Changed the DelegateCommand logic to remove the dependency on the DNAClusterPlacementEnabled flag.
src/DynamoCore/Configuration/PreferenceSettings.cs Added a new preference setting with a default value for the new flyout feature.
Files not reviewed (1)
  • src/DynamoCoreWpf/Properties/Resources.Designer.cs: Language not supported
Comments suppressed due to low confidence (3)

src/DynamoCoreWpf/Commands/PortCommands.cs:35

  • The removal of the condition based on DNAClusterPlacementEnabled may impact behavior; please verify that this change is intended and that the new logic conforms with feature requirements.
autoCompleteCommand ??= new DelegateCommand(AutoComplete, CanAutoComplete);

src/DynamoCoreWpf/ViewModels/Menu/PreferencesViewModel.cs:1134

  • [nitpick] Consider aligning naming conventions between 'NodeAutocompleteNewFlyoutIsChecked' and 'IsNewDNAFlyoutEnabled' for consistency across view models, if they represent the same feature state.
public bool NodeAutocompleteNewFlyoutIsChecked

src/DynamoCoreWpf/Views/Menu/PreferencesView.xaml:945

  • [nitpick] The margin for the new StackPanel has been updated to '0,12,0,0'; please confirm that this spacing aligns with the intended design across similar UI elements.
<StackPanel Orientation="Horizontal" Margin="0,12,0,0" Grid.Row="1">

@johnpierson johnpierson changed the title Add preferences view toggle for DNA menu DYN-8995 - Add preferences view toggle for DNA menu Jun 10, 2025
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-8995

@BogdanZavu
Copy link
Contributor

Pref options look nice and clear!
There appears to be a regression with the search animation which continues to run - in master seems to stop as expected.

@johnpierson
Copy link
Member Author

hmm, this search? It seems to be loading while I debug
20250610-searchLoading

@johnpierson johnpierson added the ai/ml 🧠 synapse team related PRs label Jun 10, 2025
@BogdanZavu
Copy link
Contributor

BogdanZavu commented Jun 10, 2025

So the animation in the search box in the regular dna not the cluster.

Well.. I made a rebuild and I'm not reproducing the issue anymore so I think we're good to go!

Update test settings file.
@johnpierson johnpierson merged commit ad65796 into DynamoDS:master Jun 11, 2025
26 of 27 checks passed
@johnpierson johnpierson deleted the dyn8995_preferenceToggle branch June 12, 2025 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai/ml 🧠 synapse team related PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants