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.

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

Graph properties view extension #11675

Merged
merged 35 commits into from
Jun 9, 2021

Conversation

SHKnudsen
Copy link
Contributor

@SHKnudsen SHKnudsen commented May 11, 2021

Purpose

This PR adds a new extension that enables setting graph properties from a UI, the extension currently supports these properties:

  • Description
  • Author
  • GraphDocumentationURL
  • Thumbnail

in addition to the above graph properties, the extension also allows storing CustomProperties in the graphs ExtensionWorkspaceData. The UI adds simple controls to add new custom key/value properties.

UI

GraphPropertiesExt_x250

Opening graph with properties set

GraphPropertiesExt_Open

json sample after setting properties

{
  "Uuid": "bd2ba3e8-f006-409b-a34e-98aaeea3e5fe",
  "IsCustomNode": false,
  "Description": "Some description about the graph.......",
  "Name": "CustompropertyTest",
  "ElementResolver": {
    "ResolutionMap": {}
  },
  "Inputs": [],
  "Outputs": [],
  "Nodes": [],
  "Connectors": [],
  "Dependencies": [],
  "NodeLibraryDependencies": [],
  "Thumbnail": "/9j/4AAQSkZJRgABAQEASABIAAD/2wBDAAMCAgMCAgMDAwMEAwMEBQgFBQQEBQoHBwYIDAoMDAsKCwsNDhIQDQ4RDgsLEBYQERMUFRUVDA8XGBYUGBIUFRTUDQsNFBQ.....
  "GraphDocumentationURL": "https://dynamobim.org/",
  "ExtensionWorkspaceData": [
    {
      "ExtensionGuid": "28992e1d-abb9-417f-8b1b-05e053bee670",
      "Name": "Properties",
      "Version": "1.0",
      "Data": {
        "My prop 1": "My value 1",
        "My prop 2": "My Value 2",
        "Custom Property 3": ""
      }
    }
  ],
  "Author": "Name of author",
  "Bindings": [],
  "View": {
    "Dynamo": {
      "ScaleFactor": 1.0,
      "HasRunWithoutCrash": true,
      "IsVisibleInDynamoLibrary": true,
      "Version": "2.12.0.4955",
      "RunType": "Automatic",
      "RunPeriod": "1000"
    },
    "Camera": {
      "Name": "Background Preview",
      "EyeX": -17.0,
      "EyeY": 24.0,
      "EyeZ": 50.0,
      "LookX": 12.0,
      "LookY": -13.0,
      "LookZ": -58.0,
      "UpX": 0.0,
      "UpY": 1.0,
      "UpZ": 0.0
    },
    "NodeViews": [],
    "Annotations": [],
    "X": 0.0,
    "Y": 0.0,
    "Zoom": 1.0
  }
}

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

@BogdanZavu @mjkkirschner

FYIs

@saintentropy @nate-peters

SHKnudsen and others added 28 commits April 14, 2021 08:28
-Validation Rule
-ValidationErrorTemplate
-Fixed empty textbox error.
Custom Property Control Add/Edit/Delete
@BogdanZavu
Copy link
Contributor

Is there a design agreement already or it is still under discussion ?

@SHKnudsen
Copy link
Contributor Author

Is there a design agreement already or it is still under discussion ?

Ill leave it to @nate-peters to answer this, but here is the mockups its based on https://www.figma.com/file/u0q3lZqxa0CxbDkZDABwhI/Dynamo-Graph-Documentation-Tools?node-id=221%3A1057

@BogdanZavu
Copy link
Contributor

@SHKnudsen I think this is ready, @nate-peters can you have another look at UI-styling ?

@nate-peters
Copy link

@SHKnudsen @BogdanZavu just a few minor UI comments and then I think we're good to go on the design side.

  • Let's use thinner icons for the X and + symbols. I'll send you the files in Slack.
  • It looks like the text in the custom properties is slightly misaligned with the section above - can you left-justify all of the labels in that section?

Screen Shot 2021-05-20 at 12 49 07 PM

@saintentropy
Copy link
Contributor

LGTM. Thanks for all the fixes

@saintentropy
Copy link
Contributor

@mjkkirschner @BogdanZavu are you done with your reviews on this

@BogdanZavu
Copy link
Contributor

There were a few more items from our last discussion but we can make a different PR for them.
One small issue : the dirty flag is not set when a custom property value or name is changed.

@mjkkirschner
Copy link
Member

@BogdanZavu it looks okay to me - one thing - are you all sure you want this called Graph Properties? I ask because other extensions that mention a graph/workspace/dyn file seem to call it Workspace?

@Jingyi-Wen FYI.

@Jingyi-Wen
Copy link

@BogdanZavu it looks okay to me - one thing - are you all sure you want this called Graph Properties? I ask because other extensions that mention a graph/workspace/dyn file seem to call it Workspace?

@Jingyi-Wen FYI.

IMO Graph Properties does make perfect sense here.

@BogdanZavu BogdanZavu merged commit 3c861d2 into DynamoDS:master Jun 9, 2021
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.

7 participants