-
Notifications
You must be signed in to change notification settings - Fork 636
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
Dynamo Default Python engine selection using child MenuItems #11088
Conversation
@Amoursol Can you confirm this is the expected behavior you want? If yes, I will try to clean up some code since old UI code will need to be removed as part of this. |
LGTM @QilongTang, thank you for updating this - looks much more cohesive now :) |
@mmisol Any comments? |
@QilongTang I guess there was no way to make this dynamic, right? Is that why this is going to 2.8 and not master? Is the plan to create a task to improve this? |
@mmisol Unfortunately I dont have the purely dynamic way working for me either. For master, I am open for discussion, maybe because my WPF knowledge is not sufficient but I saw examples online about dynamically create child menuItems, I just failed to leverage it. I plan to cherry-pick this into master as well but I dont know how practical another Jira task would be. What do you think? |
Thanks for the context @QilongTang . Maybe we can file a task , but in any case this is probably going to be done whenever we remove the Python engines from Core, so it would be hard to miss anyway. I'll review based in the fact these changes will go to master as well. |
@mmisol I was thinking the same thing, I think these options will be removed next year after Global launch. |
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.
Thanks @QilongTang . Looks good to me
@mmisol Thanks for the review. This should be the last task of RC2.8.0. Will do some testing tomorrow to wrap up release. |
* minimal changes for child menu items * Code Clean up * Comments * Comments
Please Note:
DynamoRevit
repo will need to be cherry-picked into all the DynamoRevit Release branches that Dynamo supports. Contributors will be responsible for cherry-picking their reviewed commits to the other branches after aLGTM
label is added to the PR.Purpose
Make default Python option picking similar to Number Format. The dynamic collection binding to menuitems solution did not work for me, so I had to hard code the child menu items. This is not ideal but remain consistent with Dynamo UX. There is still some clean up to do.
Declarations
Check these if you believe they are true
*.resx
filesReviewers
@DynamoDS/dynamo
FYIs
(FILL ME IN, Optional) Names of anyone else you wish to be notified of