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

Live Chart Dynamo Implementation #13631

Merged
merged 25 commits into from
Jan 31, 2023
Merged

Conversation

dnenov
Copy link
Collaborator

@dnenov dnenov commented Dec 12, 2022

Purpose

This is a Dynamo Implementation of the NodeModelCharts from Keith Alfaro implements Graphing Utility Nodes.

The goal of this PR is to empower Dynamo users to better understand, work with, and visualize their data through interactive and responsive graphical charts that work both within the Dynamo workspace and allow propagation up through Dynamo Player.

before

nodeLibrary

after

final chart collection

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

Release Notes

  • Initial Release
  • Repository has been transferred to Dynamo
  • Currently placed under the CoreNodeModelsWpf and CoreNode
  • CoreNodeModelWpf contains the Views and ViewModels
  • CoreNode contains helper logic to allow output creation

Reviewers

@reddyashish
@mjkkirschner

FYIs

@Amoursol

- added Live Charts to Dynamo UI Nodes
- this is the first commit for this PR
@chuongmep
Copy link
Contributor

chuongmep commented Dec 13, 2022

Hi Team, Live Charts is old at the moment, can be consider use Live Charts 2 ?

https://github.com/beto-rodriguez/LiveCharts2

- created a resource dictionary for custom default charts theme
- added supporting files for Dynamo Dictionary (icons, dyn example files, image example files, markdown (markdowns are 'empty', to be detailed by UX team)
- changed namespace for Live Chart nodes
@Amoursol
Copy link
Contributor

Amoursol commented Dec 19, 2022

Hi Team, Live Charts is old at the moment, can be consider use Live Charts 2 ?

https://github.com/beto-rodriguez/LiveCharts2

Hi Chuong! LiveCharts2 is indeed way more expansive, but due to the dependencies involved we are opting for the original for now as we aim to keep Dynamo's dependency landscape as small as possible.

@mjkkirschner can probably elaborate more 😊

- renamed help files to match node full (type) names
- testing AstFarctory.BuildFunctionCall when method is in another assembly
- added Helper class to provide Colors from the LiveCharts xaml dictionary
- rework of BarChartNodeModel to allow for a non-nested value/input
- moved helper functions to the DynamoCore assembly to satisfy AstFactory conditions
- BarChart now works with both List<double> and List<List<double>>
- added LiveCharts to Dynamo license files
…Implementation"

This reverts commit 0ee85de, reversing
changes made to ce4a592.
- updated all the rest of the functions (besides the BarChart)
- all charts now work with/without Color input
- fixed old/wrong example dyn files
- multiple small visual updates
- added 26 default colors to cycle through, if the user is not providing custom colors
- changed visual style of the resizing thumb for the chart controls
- added tooltip
- tooltip text added to resources
…Implementation"

This reverts commit 321a326, reversing
changes made to 1023291.
@reddyashish reddyashish marked this pull request as ready for review January 19, 2023 15:13
- added the nodes to the library
- added icons to nodes
- added Alert message when default Colors are provided
- as discussed, undid the <UseWPF>true</UseWPF> setting
- swapped localization strings with resources
- the icon used for resizing the node UI window now consistent with other UI nodes, such as Watch, Watch3D
- added basic creation tests for LiveCharts
- threading issues does not allow output to be tested
- changed the notification text when user provides no colors or mismatching color structure
Copy link
Contributor

@sm6srw sm6srw left a comment

Choose a reason for hiding this comment

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

@dnenov I have done a partial review so far. Will continue tomorrow.

- added dispose methods to LiveChart View and ViewModel classes
- reverted small change previously made with no relevance to this PR
Copy link
Contributor

@sm6srw sm6srw left a comment

Choose a reason for hiding this comment

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

We are getting close. I have a question about the memory management in user controls and there is a couple of TODO's that needs to be addressed.

- removed old backup file
- changed Dispose (not being triggered) to Unload to dispose of eventhandlers
- cleaned up old TODO remarks
Copy link
Contributor

@sm6srw sm6srw left a comment

Choose a reason for hiding this comment

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

Looks good. I will kickoff the tests again just to make sure.

Copy link
Contributor

@sm6srw sm6srw left a comment

Choose a reason for hiding this comment

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

@sm6srw sm6srw merged commit 49847b7 into DynamoDS:master Jan 31, 2023
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.

5 participants