Skip to content

DYN-6518 - Cherry Picks for 2.19.5 #15019

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
Closed
saintentropy wants to merge 2 commits into from
Closed

DYN-6518 - Cherry Picks for 2.19.5 #15019

saintentropy wants to merge 2 commits into from

Conversation

@saintentropy
Copy link
Contributor

@saintentropy saintentropy commented Mar 14, 2024

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
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

QilongTang commented Mar 20, 2024

@saintentropy @twastvedt Close this PR?

@twastvedt
Copy link
Contributor

twastvedt commented Mar 20, 2024

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

@twastvedt twastvedt twastvedt approved these changes

@BogdanZavu BogdanZavu BogdanZavu approved these changes

+1 more reviewer

@Mikhinja Mikhinja Mikhinja left review comments

Reviewers whose approvals may not affect merge requirements

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