-
Notifications
You must be signed in to change notification settings - Fork 199
Rename "Element Types" to "Element Classes" distinguished from "Eleme… #2641
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
Conversation
| } | ||
|
|
||
| [NodeName("Element Types")] | ||
| [NodeName("Element Classes")] |
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.
This class ElementTypes is subclass of AllChildrenOfType<Element>, and can't modify its output name.
The output name is set in DynamoCore - https://github.com/DynamoDS/Dynamo/blob/master/src/Libraries/CoreNodeModels/Enum.cs#L65 .
So I think I need submit a update here.
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.

Hi @ShengxiZhang , with this PR, the results as the screenshot shown. We can't rename the output name "Type" on DynamoRevit side.
|
I think it's important that before making this change testing is done on graphs and custom nodes that reference these nodes to make sure they open okay. |
|
@mjkkirschner I just run all DynamoRevit tests and all have passed. I also test the old script with this change, it can run without error or warning. |
|
I think we not necessary change name to class because engineer difficult for read document update . |
… ReNameElementTypes
But this does mislead some users and confuse them like #2587 . So I think this will be helpful for distinguishing Element Class and ElementType. |

…ntType"
Purpose
Rename Element Types node to make it more meaningful and distinguished from "ElementType"
Related to issue #2587
Declarations
Check these if you believe they are true
*.resxfilesReviewers
@ShengxiZhang @wangyangshi
FYIs
@Amoursol @QilongTang @mjkkirschner