DYN-8850: Reconfigure preference settings for node autocomplete#16230
DYN-8850: Reconfigure preference settings for node autocomplete#16230chubakueno merged 7 commits intoDynamoDS:masterfrom
Conversation
There was a problem hiding this comment.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-8850
| /// <summary> | ||
| /// Looks up a localized string similar to Node Type Match. | ||
| /// </summary> | ||
| public static string NodeTypeMatch { |
There was a problem hiding this comment.
Is our control auto-resizable horizontally ? This text will probably be bigger in certain languages.
I see it, it's auto.
There was a problem hiding this comment.
The whole flyout has a fixed size but the NodeTypeMatch textview is sized as Auto, so it will compress the other views sized as * (the buttons) to make space for itself while possible.
| /// <summary> | ||
| /// Event that is fired when a node is removed from the workspace. | ||
| /// </summary> | ||
| public event Action PreferencesChanged; |
There was a problem hiding this comment.
I'm not sure about this one.
We are calling this from the onClose but there is no guarantee that the preferences have actually changed.
Plus the underlying object we care about is the PreferenceSettings instance and one can subscribe to that.
This can be modified from the API as well not only from the dialog.
However the Preferences dlg is modal so we should only do the update onClose when the dialog is present.
But I don't think we need to do the real time update so I'm ok with removing this.
So I would say to only consider the options on first occasion like the flyout show.
There was a problem hiding this comment.
Its weirder if its not realtime as the visual state gets desynchronized with the internal state. Its fine if we reload the view each time the user opens and closes the preferences, which shouldn't happen too often while the flyout is still visible.
We can have the best of both if we introduce an object to represent the latest copy of the preferences settings we're interested in and comparing with it, but it could introduce maintenance bugs when modifying the code and not updating that object.
So, I'd prefer it to load every time instead of get bugged when not refreshing. Plus, this may be useful for other parts of the application that would benefit from dynamically updating.
There was a problem hiding this comment.
Let's rename it then OnPreferencesClosed or something and maybe make it internal.
There was a problem hiding this comment.
Can't make it internal as we're using it from another assembly (the autocomplete view extension) but we can rename it
There was a problem hiding this comment.
We can access internals from our assembly since we have this [assembly: InternalsVisibleTo("NodeAutoCompleteViewExtension")].
But thinking more about it maybe we can avoid it all together and be more inline with the way other settings are handled.
So how about we define out DNA event in preferences for all the DNA settings and subscribe/react to it from the extension as usual.
The preferences is modal so we'll just need to postpone the action on Iddling - we do this in other contexts as well.
There was a problem hiding this comment.
Very nice! Now we also have the api scenario covered and better handling of multiple req that could come from anywhere.
|
So in Node Type Match mode we are showing the total number of matches which is big. |
|
Oh and I guess we need to deprecate some of previous options like HideNodesBelowSpecificConfidenceLevel ? |
Those are not new, but the mockup no longer has that option. If we're sure it will stay this way in the future we can delete nonpublic members and obsolote public ones, but would need to confirm. |
Ok, we'll handle it later before 3.6. |
| Dynamo.ViewModels.PreferencesViewModel.PackagePathsViewModel.set -> void | ||
| Dynamo.ViewModels.PreferencesViewModel.PersistExtensionsIsChecked.get -> bool | ||
| Dynamo.ViewModels.PreferencesViewModel.PersistExtensionsIsChecked.set -> void | ||
| Dynamo.ViewModels.PreferencesViewModel.PreferencesChanged -> System.Action |
|
Let's remove that leftover api and we're good to go. |
|
Tests failed on unrelated |
Purpose
Reconfigure preference settings for node autocomplete
2025-05-16.21-07-29.mp4
Declarations
Check these if you believe they are true
*.resxfilesRelease Notes
Reconfigure preference settings for node autocomplete
Reviewers
@BogdanZavu
@johnpierson
FYIs
@QilongTang