Skip to content
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.

Sign up for GitHub

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

Jump to bottom

DYN-6518 - Cherry Picks for 2.19.5 #15019

Closed
wants to merge 2 commits into from
Closed

DYN-6518 - Cherry Picks for 2.19.5 #15019

wants to merge 2 commits into from

Conversation

saintentropy
Copy link
Contributor

Purpose

This Cherry Pick is bring in #14656 and #14744. These two PR's addressed a regression in behavior for some DropDown nodes in Revit introduced in 2.19.0 and fixed in 3.0.

This original regression was related to this PR #14122

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated
  • This PR contains no files larger than 50 MB

Release Notes

Some Drop down nodes had issues when Dynamo is set to Automatic Run Mode in DynamoForRevit. This change addresses this issue.

Reviewers

@twastvedt @BogdanZavu

FYIs

@Mikhinja @aparajit-pratap @QilongTang

@twastvedt @saintentropy
REVIT-215698: Restrict where OnNodeModified is called. (DynamoDS#14656)
0018fd5 
* Restrict where OnNodeModified is called.

* Clean up.

* Dispose

* No SuppressFinalize

(cherry picked from commit f1f9b74)
@twastvedt @saintentropy
Add selection handler after binding. (DynamoDS#14744)
875dd85 
(cherry picked from commit 8435171)
@twastvedt twastvedt changed the title DYN-6581 - Cherry Picks for 2.19.5 DYN-6518 - Cherry Picks for 2.19.5 Mar 14, 2024
@QilongTang
Copy link
Contributor

QilongTang commented Mar 14, 2024 •
edited
Loading

The pull request SelfServe results does not seem real, maybe ASM or libG related errors? Other than that looks good

Mikhinja
Mikhinja reviewed Mar 14, 2024
View reviewed changes
src/Libraries/CoreNodeModels/DropDown.cs
@@ -106,7 +106,6 @@ public int SelectedIndex
selectedString = GetSelectedStringFromItem(Items.ElementAt(value));
}

OnNodeModified();
Copy link
Contributor

@Mikhinja Mikhinja Mar 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the fix for one of those two issues in 3.0 caused an issue in Revit ( REVIT-217658 ) where a custom dropdown node of an internal-package for DynamoRevit that needed to update on idle was stuck always one update state delayed, sometimes leading to confusing situations where Dynamo Player showed content from the previously opened Revit model.

We might want to consider the risks/benefits of this change, and if we will backport this change into 2.19 then in Revit we must also integrate the changes for REVIT-217658. We should also test around nodes that have UI updates to make sure there aren't other issues not yet found.

@QilongTang QilongTang mentioned this pull request Mar 19, 2024
Merged
9 tasks
@QilongTang QilongTang added this to the 2.19.5 milestone Mar 19, 2024
@QilongTang
Copy link
Contributor

@saintentropy @twastvedt Close this PR?

@twastvedt
Copy link
Contributor

Closing (won't cherry pick for 2.19) due to the resulting bugs referenced by Vlad above.

@twastvedt twastvedt closed this Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mikhinja Mikhinja Mikhinja left review comments

@BogdanZavu BogdanZavu BogdanZavu approved these changes

@twastvedt twastvedt twastvedt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2.19.5
Development

Successfully merging this pull request may close these issues.
5 participants
@saintentropy @QilongTang @twastvedt @BogdanZavu @Mikhinja