Skip to content

Fix Dictionary Preview in Code block node#9020

Merged
aparajit-pratap merged 1 commit intoDynamoDS:masterfrom
aparajit-pratap:fixDictPreview
Aug 2, 2018
Merged

Fix Dictionary Preview in Code block node#9020
aparajit-pratap merged 1 commit intoDynamoDS:masterfrom
aparajit-pratap:fixDictPreview

Conversation

@aparajit-pratap
Copy link
Contributor

@aparajit-pratap aparajit-pratap commented Jul 27, 2018 •
edited
Loading

Purpose

The following node preview for a Dictionary in a CBN containing multple output ports does not expand currently. This is the screenshot after the fix:
image

This is because preferredDictionaryOrdering was a list of empty strings (3 in this case, one for each output port) and even though it should have been ignored it passed the check in DefaultWatchHandler::ProcessThing. This fix it to prevent the list from containing empty strings. Note that preferredDictionaryOrdering must only be populated for multi output nodes that have a MultiReturnAttribute.

Declarations

Check these if you believe they are true

  • The code base 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.

Reviewers

@mjkkirschner

FYIs

@jnealb

@mjkkirschner
Copy link
Member

mjkkirschner commented Jul 27, 2018

@aparajit-pratap just curious where did the extra entries come from? Does anything need to be updated similarly to this on the TS side?

@jnealb
Copy link
Collaborator

jnealb commented Jul 27, 2018

@aparajit-pratap @mjkkirschner Is there a JIRA task for this?

@aparajit-pratap
Copy link
Contributor Author

aparajit-pratap commented Jul 28, 2018

@jnealb https://jira.autodesk.com/browse/QNTM-4916

@jnealb
Copy link
Collaborator

jnealb commented Jul 28, 2018

@aparajit-pratap Thanks.

@aparajit-pratap
Copy link
Contributor Author

aparajit-pratap commented Jul 28, 2018

@mjkkirschner not sure what you mean by extra entries? Nothing needs to be updated on the TS side as this is more of a UI (preview) change in Dynamo.

@mjkkirschner
Copy link
Member

mjkkirschner commented Jul 28, 2018

@aparajit-pratap misunderstood, so previously the dictionary was not displayed for multi return functions called in code blocks if one of the outputs was an empty string?

@aparajit-pratap
Copy link
Contributor Author

aparajit-pratap commented Jul 28, 2018

@mjkkirschner previously the dictionary could not be expanded in the preview if it was the output of a multi-output port code block node. Please don't confuse yourself with the contents of this Dictionary in the above example.

@aparajit-pratap
Copy link
Contributor Author

aparajit-pratap commented Jul 31, 2018

@mjkkirschner good to go?

@mjkkirschner
Copy link
Member

mjkkirschner commented Jul 31, 2018

@aparajit-pratap LGTM

@aparajit-pratap aparajit-pratap merged commit 1842c74 into DynamoDS:master Aug 2, 2018
@aparajit-pratap aparajit-pratap deleted the fixDictPreview branch August 2, 2018 14:11
alfarok pushed a commit to alfarok/Dynamo that referenced this pull request Nov 8, 2018
@aparajit-pratap @alfarok
fix Dictionary preview (DynamoDS#9020)
a1cb1c2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@aparajit-pratap @mjkkirschner @jnealb