-
Notifications
You must be signed in to change notification settings - Fork 668
DYN-8879: Update graph properties from one location on the menu #16476
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
DYN-8879: Update graph properties from one location on the menu #16476
Conversation
- header resources in DynamoCoreWpf.Properties - using command in DynamoViewModel instead of injecting the whole menu item in the code-behind
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.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-8879
…location-on-the-menu
…he-menu' of https://github.com/ivaylo-matov/Dynamo into DYN-8879-Update-graph-properties-from-one-location-on-the-menu
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.
Pull Request Overview
This PR implements a new extensible menu system that allows view extensions to inject custom menu items into Dynamo's File menu. The Graph Metadata extension is updated to use this new system, replacing its previous direct menu integration approach.
Key changes:
- Introduces a new
IExtensionMenuProviderinterface for extensions to provide menu items - Updates the Graph Metadata extension to implement this interface and removes its hardcoded menu text
- Adds a new "Show Graph Properties" submenu structure in the File menu with "General" and placeholder "Advanced Sharing" options
Reviewed Changes
Copilot reviewed 11 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| IExtensionMenuProvider.cs | New interface defining the contract for extensions that provide menu items |
| GraphMetadataViewExtension.cs | Updated to implement IExtensionMenuProvider and removed direct menu text setting |
| DynamoView.xaml | Added new "Show Graph Properties" submenu structure to File menu |
| DynamoView.xaml.cs | Added event handler for ShowGraphPropertiesRequested to toggle extension visibility |
| DynamoViewModel.cs | Added ShowGraphProperties command and event handling |
| DynamoViewModelDelegateCommands.cs | Added ShowGraphPropertiesCommand delegate |
| Resources files | Moved menu text from extension-specific to shared resources and added new localized strings |
| PublicAPI.Unshipped.txt | Added new public API entries for the command and interface |
| DynamoCoreWpf.csproj | Added the new interface file to the project |
Files not reviewed (2)
- src/DynamoCoreWpf/Properties/Resources.Designer.cs: Language not supported
- src/GraphMetadataViewExtension/Properties/Resources.Designer.cs: Language not supported
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…he-menu' of https://github.com/ivaylo-matov/Dynamo into DYN-8879-Update-graph-properties-from-one-location-on-the-menu
|
@ivaylo-matov I think it would be nice if you could add some tests for this change |
|
@zeusongit, I've added GetFileMenuItemReturnsCheckableMenuItem to GraphMetadataViewExtensionTests (verifies the extension exposes a non-null, checkable MenuItem). I also created a new GraphMetadataFileMenuIntegrationTests class with three additional tests focused on the File menu integration. This class can be expanded further once the D4C work begins. |
| <data name="ImageSelector_Title_SelectImage" xml:space="preserve"> | ||
| <value>Select Image</value> | ||
| </data> | ||
| <data name="MenuItemText" xml:space="preserve"> |
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.
is this already removed in the default resources?
| // in AssemblyVersionInfo.cs so that it can be easily incremented by the | ||
| // automated build process. | ||
| [assembly: AssemblyVersion("4.0.0.2016")] | ||
| [assembly: AssemblyVersion("4.0.0.2403")] |
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.
undo this file commit.
reddyashish
left a comment
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 with couple comments.
…location-on-the-menu
…location-on-the-menu
…location-on-the-menu
…location-on-the-menu
Purpose
This PR addresses DYN-8879.
Adds support for view extensions to inject their own menu items into Dynamo’s File menu. It also updates the Graph Metadata extension to use this new system.
Changes:
IExtensionMenuProvider. Extensions that implement this can return a MenuItem to be shown in the main Dynamo UI.DynamoCoreWpf.ShowGraphPropertiesCommandin DynamoViewModel. When the "General" menu item is clicked, this command fires an event which toggles the visibility of the extension panel.DynamoCoreWpf.Declarations
Check these if you believe they are true
*.resxfilesRelease Notes
Adds support for view extensions to inject their own menu items into Dynamo’s File menu. It also updates the Graph Metadata extension to use this new system.
Reviewers
@DynamoDS/eidos
@jasonstratton
FYIs
@dnenov
@achintyabhat