Skip to content

Conversation

@SHKnudsen
Copy link
Contributor

Purpose

This PR makes sure that Connector Pins are added to groups.

Only pins where both the Connector Start and End owner belong to the group get added.

pinsIncludedInGroups

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

Reviewers

@QilongTang

FYIs

@SHKnudsen SHKnudsen force-pushed the And-connector-pins-to-groups branch from 338c79b to 8f98c95 Compare September 28, 2021 15:54

public ConnectorPinView()
{
Resources.MergedDictionaries.Add(SharedDictionaryManager.DynamoConvertersDictionary);
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @SHKnudsen Can you remind me what is this for? I thought at some point we conclude this does not work? Or maybe I misunderstood

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need to load the converters to be able to use InverseBoolToVisibilityCollapsedConverter so that we can collapse the pins
https://github.com/DynamoDS/Dynamo/pull/12070/files#diff-bb986eed1172de44f4bd46182f227792c03db1a77bbb4d76a206b7f8c0c126e0R15

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you put this as a comment? Once that is done, this is good to go, thanks for doing it

Copy link
Contributor

@QilongTang QilongTang left a comment

Choose a reason for hiding this comment

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

One comment then LGTM

@QilongTang QilongTang merged commit 4359496 into DynamoDS:master Sep 30, 2021
M-JULIANI pushed a commit to M-JULIANI/Dynamo that referenced this pull request Sep 30, 2021
* add pins to groups

* Update ConnectorPinView.xaml.cs
QilongTang added a commit that referenced this pull request Oct 4, 2021
* Polyline + thick outline removed.

* Removed 'ConnectorType` menu from view->connectors.

* Updated connector styles.

* New resources added/old ones removed. Minor UI updates to reflect mockups.

* First commit.

* Cleanup. On-the-fly ContextMenu working.

* Applying ContextMenu styling.

* Added (stole) ContextMenu styling.

* Added resource strings to ContextMenu control.

* Update AssemblySharedInfo.cs

* Unused ref removal.

* Cleanup.

* Fixes.

* First commit.

* Cleanup. On-the-fly ContextMenu working.

* Applying ContextMenu styling.

* Added (stole) ContextMenu styling.

* Added resource strings to ContextMenu control.

* Update AssemblySharedInfo.cs

* Cleanup.

* Adding 10 to margin (#12076)

* Update AnnotationModel.cs (#12086)

* Crash Fix.  Add visibility trigger to empty tooltip strings. (#12069)

* Add visiblity trigger to empty tooltip strings

* PR comment

* Pr comments

Co-authored-by: Craig Long <craig.long@autodesk.com>

* DYN-3992: Upgrade LibG (#12060)

* mark test as failure temporarily to fix build issues (#12090)

* add pins to groups (#12070)

* add pins to groups

* Update ConnectorPinView.xaml.cs

* Remove outdated logic (#12081)

* Wire connector UI modifications (#12072)

* Polyline + thick outline removed.

* Removed 'ConnectorType` menu from view->connectors.

* Updated connector styles.

* New resources added/old ones removed. Minor UI updates to reflect mockups.

* [DYN-4105] Make sure nodes gets added to parent group (#12084)

* make sure nodes gets added to parent group

* remove commented code

Co-authored-by: Aaron (Qilong) <173288704@qq.com>

* Updates resource strings from Remove to Delete (#12088)

* Improved ContextMenu behaviour.

* PR fix.

* Removed unnecessary refs.

* Added unit tests for ConnectorContextMenu construction/disposal/commands.

Co-authored-by: filipeotero <89042471+filipeotero@users.noreply.github.com>
Co-authored-by: Sylvester Knudsen <sylvesterknudsen@gmail.com>
Co-authored-by: Craig Long <craig.alan.long@gmail.com>
Co-authored-by: Craig Long <craig.long@autodesk.com>
Co-authored-by: Jorgen Dahl <sm6srw@users.noreply.github.com>
Co-authored-by: aparajit-pratap <aparajit.pratap@autodesk.com>
Co-authored-by: Aaron (Qilong) <173288704@qq.com>
Co-authored-by: Ollie Green <oliveredwardgreen@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants