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

LiveCharts Tooltip Update #13776

Merged
merged 22 commits into from
Mar 31, 2023

Conversation

dnenov
Copy link
Collaborator

@dnenov dnenov commented Feb 24, 2023

Purpose

A small PR that with updates to inport and outport tooltips for Live Chart nodes to follow the common pattern displaying the type of value associated with the port.
Also fixes a number of inconsistent port input combinations and behaviors.

default values

WIP - Ports Interaction and Combinations

node states

Heat Series

heat series

Pie Chart

pie chart

Scatter Plot

scatter plot

XY Line

xy line

Basic Line Chart

basic line chart

Bar Chart

bar chart

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

  • update to inport tooltips, including input type
  • update to outport tooltips, including output type
  • update to the in-depth description of the Index-Line Plot chart
  • now outputs default values to complement the Chart image
  • fixed output types for some of the charts
  • fixed resource strings for output description
  • fixed a bug where the Info state would not clear after Color value has been provided

Reviewers

@sm6srw

FYIs

@Amoursol
@hwahlstrom

- updated the in-depth description of the Index-Value Line Plot chart
- updated live charts input and ouptut tooltip for all charts
- rework of inport and outport tooltip messages from strings to resources
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.

LGTM!

dnenov added 2 commits March 10, 2023 18:13
- wip for default values
- now will output default values to go with the image of the chart
- adjusted outputs of some of the nodes
- changed resource strings accordingly
@Amoursol
Copy link
Contributor

@dnenov LGTM! The only remaining thing would be the blue bar vertically outside the node bounds which indicates that default values are assigned 😊 Is that possible to do, or too hard for UI nodes?

dnenov added 8 commits March 14, 2023 09:00
- the logic is not very clear, still working through it
WIP CHANGES ONLY FOR THE XYLineChartControl for now! The rest of the LiveCharts to follow after approving this one
- fixed a serious issue causing Dynamo to freeze because of a deadlock
- the issue was caused by the Info state of the node interaction inside the DataBridgeCallback
- minor addition to the chart node view - now restricts the minimum size to which the user can resize the window
- now checks all conditions and combinations of port values
- OK - all ports correct values, no ports - default values, label and value ports correct values + default colors
- Warning - all ports provided but mismatching lists
- Warning - all ports, but wrong port type
- No Warning, no run - missing ports
- fixed x-values, y-values port names
- fixed a state where the UI would not go back to default state if initialized with null data (after save/open), or would retain default data on UI when wrong/incomplete values were provided to the Node
- moved viewmodel properties initialization up before calling the view to prevent null values
- correctly checks for null and empty lists before initializing to allow proper initialization with empty series
- following XYLineChart and ScatterPlot now fixed all connectivity issues with the Heat Series
- a follow up for Bar Chart
@dnenov dnenov marked this pull request as draft March 21, 2023 14:57
dnenov added 2 commits March 21, 2023 16:17
- follow up with basic line chart
- finally the changes of pie chart
@reddyashish reddyashish marked this pull request as ready for review March 22, 2023 14:15
dnenov added 2 commits March 24, 2023 16:37
- detailed live chart tests to cover a lot more use cases and interactions
- modified ClearInfoMessages API - now removes `info` state if node is in it
@reddyashish
Copy link
Contributor

Build is failing on master-5. Is it building fine locally?

Copy link
Contributor

@reddyashish reddyashish left a comment

Choose a reason for hiding this comment

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

LGTM with one comment, triggered a new self-service run

@reddyashish reddyashish marked this pull request as draft March 24, 2023 20:45
@reddyashish
Copy link
Contributor

Making this as a draft for now, till we figure out the failing tests

dnenov added 2 commits March 24, 2023 21:16
- manually recovered lost resources after merge
@reddyashish reddyashish added this to the 2.18.0 milestone Mar 27, 2023
@reddyashish reddyashish marked this pull request as ready for review March 31, 2023 14:49
@reddyashish reddyashish marked this pull request as draft March 31, 2023 14:49
@reddyashish reddyashish marked this pull request as ready for review March 31, 2023 18:09
@reddyashish
Copy link
Contributor

reddyashish commented Mar 31, 2023

Job is running fine now: https://master-5.jenkins.autodesk.com/job/Dynamo/job/DynamoSelfServe/job/pullRequestValidation/8788/ with one flaky failing test.

@reddyashish reddyashish merged commit 8366345 into DynamoDS:master Mar 31, 2023
sm6srw pushed a commit that referenced this pull request Apr 5, 2023
* Updated Index-Value Line description

- updated the in-depth description of the Index-Value Line Plot chart

* Updated outport tooltipl

- updated live charts input and ouptut tooltip for all charts

* Port Description to Resources

- rework of inport and outport tooltip messages from strings to resources

* Default Values Part 1

- wip for default values

* Default values functional

- now will output default values to go with the image of the chart
- adjusted outputs of some of the nodes
- changed resource strings accordingly

* Port logic, warning and info logic WIP

- the logic is not very clear, still working through it

* Port interaction bugfix, min size for chart node

WIP CHANGES ONLY FOR THE XYLineChartControl for now! The rest of the LiveCharts to follow after approving this one
- fixed a serious issue causing Dynamo to freeze because of a deadlock
- the issue was caused by the Info state of the node interaction inside the DataBridgeCallback
- minor addition to the chart node view - now restricts the minimum size to which the user can resize the window

* All conditions satisfied

- now checks all conditions and combinations of port values
- OK - all ports correct values, no ports - default values, label and value ports correct values + default colors
- Warning - all ports provided but mismatching lists
- Warning - all ports, but wrong port type
- No Warning, no run - missing ports

* Fixed inport names

- fixed x-values, y-values port names

* Rework of default and update of UI control

- fixed a state where the UI would not go back to default state if initialized with null data (after save/open), or would retain default data on UI when wrong/incomplete values were provided to the Node

* Fix null checks

- moved viewmodel properties initialization up before calling the view to prevent null values
- correctly checks for null and empty lists before initializing to allow proper initialization with empty series

* Heat Series Fixed

- following XYLineChart and ScatterPlot now fixed all connectivity issues with the Heat Series

* BarChart fixed

- a follow up for Bar Chart

* Basic Line Chart fix

- follow up with basic line chart

* Pie Chart fix

- finally the changes of pie chart

* Added live chart test conditions

- detailed live chart tests to cover a lot more use cases and interactions
- modified ClearInfoMessages API - now removes `info` state if node is in it

* Recover lost resources

- manually recovered lost resources after merge

* Fix resources en-US file

* Add failure category to live chart tests

* Fix live chart tests

---------

Co-authored-by: reddyashish <[email protected]>
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