CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 658
[DYN-7874] As a Dynamo user I do not want the default group description text #15885
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-7874] As a Dynamo user I do not want the default group description text #15885
Conversation
…he-default-group-description-text
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-7874
|
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.
Changes look good, need to resolve conflicts.
First, I like this idea quite a bit as descriptions are often not required. however, this is causing what I feel is a regression in user experience. If a user went through the effort of removing these descriptions in a previous version, they are now surfacing when a file from a previous version of Dynamo is opened in this dev version with "Use Default Description" enabled. And often times users will remove descriptions from some groups, but not all groups. This will surely tick off some users. is there a requirement for this to modify existing groups in a file that has been opened? If so, I would lean toward prompting the user before changing the groups for them. cc @jnealb |
One test failing: [Dynamo.Tests.Configuration.PreferenceSettingsTests.TestImportCopySettings] |
In that case, we can make this change only for newly created groups and i think that would be ideal for all. @achintyabhat @hwahlstrom What do you think? |
/// <param name="showDefault">Whether to apply the default description or clear it.</param> | ||
public void ApplyAnnotationDescriptionSetting(bool showDefault) | ||
{ | ||
foreach (var workspace in Model.Workspaces) |
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.
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.
@ivaylo-matov In this case, we want to check for groups only in the homeworkspace?
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.
@reddyashish , my understanding was that the problem was in marking the cunstom node as modified and not removing the group description from it in principle. This should be addressed now. I think the idea is to remove (tunr off) the descriptions consistently in every workspace. Hope that makes sense.
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.
So your changes won't remove the group description in the custom node workspace? Confused when you say every workspace as only one homeworkspace will be opened at one time.
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.
apologies if my explanation was unclear
If we have one graph open along with a couple of custom nodes, Model.Workspaces will return three items:
- 1x HomeWorkspaceModel
- 2x CustomNodeWorkspaceModels
When a user turns off the default group description in Preferences, Dynamo will:
- Apply this change to both the open graph and the two custom nodes.
- Ensure that default group descriptions are disabled everywhere, so that everuthing is consistent.
- Prevent a situation where the main graph (HomeWorkspaceModel) hides the default group description, while custom nodes (CustomNodeWorkspaceModels) still display it.
The purpose of RefreshAnnotationDescriptions() is not to remove the description value. Instead, it reassigns the same description to each group, triggering RaisePropertyChanged().
This allows AnnotationTextConverter to detect the change and update the group view by setting the description to string.Empty.
I agree, make it apply to newly created groups only. Would that resolve the issue, @johnpierson ? |
Thanks for the feedback @johnpierson . How do you test wilt different languages? Does not seem to work completely on the builds. If I chage the language only a few of the button will be translated and the new groups will have default text in EN. |
@hwahlstrom that would work for me! New groups only. @ivaylo-matov - I couldn't get the sandbox language flag to work either (https://dynamobim.org/multilingual-dynamo/), so I created a DYN in Revit with these instructions https://help.autodesk.com/view/RVT/2024/ENU/?guid=GUID-BD09C1B4-5520-475D-BE7E-773642EEBD6C Let me know when your new revisions are in and I can test! Also, not 100% sure the cross-langage thing applies if we only do it on newly created groups. :) |
@johnpierson, all ready for testing. As mentioned above, I changed the strategy, and it now works with existing groups as well. In my opinion, this solution is better and addresses the points you raised, but let me know if you still prefer applying the changes only to new groups. It also seems to work across different languages. See updated jifs in description. |
Looks great @ivaylo-matov ! Everything works how I would hope. The only odd (kinda) experience is if you copy a group that has had the default text modified with preferences, it will now be modified. But I do not think that is a problem. |
Triggered the job again: https://master-5.jenkins.autodesk.com/job/Dynamo/job/DynamoSelfServe/job/pullRequestValidation/17393/ as the previous one did not finish. |
@ivaylo-matov one comment left here: #15885 (comment) |
Looks like this test Dynamo.Tests.Configuration.PreferenceSettingsTests.TestImportCopySettings is failing here. Can you verify that? |
|
Purpose
This PR addresses DYN-7874. It adds a new preference setting to enable | disable the default description text for groups. Previously, users had to manually delete the default text each time they created a group. Now, they can toggle this behavior in Preferences.
Key Changes:
Declarations
Check these if you believe they are true
*.resx
filesRelease Notes
This PR adds a new preference setting to enable | disable the default description text for groups. Previously, users had to manually delete the default text each time they created a group. Now, they can toggle this behavior in Preferences.
Reviewers
@QilongTang
@reddyashish
FYIs
@dnenov
@achintyabhat