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.

Sign up for GitHub

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

Jump to bottom

GraphNodeManager Update #13069

Merged
merged 37 commits into from
Aug 16, 2022
Merged

GraphNodeManager Update #13069

merged 37 commits into from
Aug 16, 2022

Conversation

dnenov
Copy link
Collaborator

@dnenov dnenov commented Jun 28, 2022 •
edited
Loading

Purpose

This is a compound PR targetting multiple bug fixes and updates for the GraphNodeManagerViewExtension. The list below shows the list of running comments and issues that are being solved with the PR.

  • Nested Empty Lists
  • Filter Label Consistent Capitalisation
  • Tooltip bubble arrow of Export and Copy needs to be flipped
  • Update Export to Excel tooltip
  • Warning/error font color update (brighten, currently low contrast)
  • Dismissed warning not showing up in GNM in Automatic Mode
  • Hover state (color) of alternating rows
  • Consistent selected bar color (no state) - currently affected by alternating row color
  • 'Search' placeholder text update
  • 'Search' exits focus when clicking outside the search field
  • Alignment: first icon to align to column header (title)
  • Vertical lines look dashed
  • Export to CSV/JSON - populate a default name
  • (Performance) - GNM + Custom Nodes (Spring, Clockwork) laggy behaviour
  • Blue Dot denoting renamed nodes
  • Icons to Filters
  • JSON exported file error on opening could not recreate the issue
  • Drop-down file option JSON/CSV bug
  • Navigating away from a selected node not resetting selection (when clicking again, won't zoom on the node again)
  • Export only exports unfiltered nodes
  • Alternative export CSV/JSON tooltip when filter(s) are on
  • Show any null or emptylist

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

A series of bug fixes or updates to the View Extension.

  • (nested empty lists) added a recursion to the IsNodeEmptyList method to factor in nested empty lists
    before-after
  • (dismissed alerts count) a few limitations: only the NodeModel is visible; the DismissedAlerts inside the NodeModel won't raise PropertyChanged. Therefore introduced a dedicated property for DismissedAlertsCount inside the NodeModel that raises PropertyChanged and can be detected from the View Extension
    after

Reviewers

@reddyashish
@mjkkirschner

FYIs

@Amoursol

dnenov added 3 commits June 28, 2022 14:09
@dnenov
Nested Empty List Not Showing
5ccedd2 
- added a recursion to the IsNodeEmptyList method to factor in nested empty lists
@dnenov
Filter Label Consistent Capitalisation
f272c2c 
- fixed inconsistency in capitalization of Filter titles
@dnenov
Export Tooltip
b0b940a 
- 'Export to Excel' changed to 'Export'
@dnenov dnenov changed the title Nested Empty List Not Showing GraphNodeManager Update Jun 28, 2022
dnenov added 8 commits June 28, 2022 16:58
@dnenov
Tooltip Export and Copy Flip
9978f7b 
- used a different style of Dynamo Tooltip supporting a positioning for the arrow to adjust for these 2 edge cases
@dnenov
DismissedAlerts Not Updated in Automatic Mode
fa1e4d2 
- added a dedicated property inside the NodeModel to raise PropertyChanged when updating the DismissedAlerts
- as the external view extension has no access to the NodeViewModel, the code previously acquired the DismissedAlerts count from the .Count value of the DismissedAlert ObservableCollection. However, this wasn't triggering an update and was not being detected inside the external code of the view extension
@dnenov
Merge branch 'master' of https://github.com/DynamoDS/Dynamo into dnen…
19b0211 
…ov-update-graphnodemanager
@dnenov
Icons to Filters
cc65e29 
- added Icons to the Filter elements in the UI
@dnenov
Aligned Filters to Figma
3090d39 
- reshuffled order of filters
- changed 'Information' to 'Info' title
@dnenov
Export Default Name
759c6ed 
- added a default file name based on the name of the current graph
- fixed a bug when using the CSV file extension dropdown
@dnenov
Export Unfiltered Nodes Only
ab01d8f 
- only export nodes that haven't been filtered in the UI
@dnenov
Renamed Node Blue Dot
30b59aa 
- added an identifier to the renamed nodes
- IMPORTANT - exposed a method from NodeModelExtensions to allow to check if a node has been renamed!
dnenov added 7 commits July 27, 2022 10:46
@dnenov
Comments: nameof
2658095 
- using nameof(<PropertyName>) instead of string value
@dnenov
Comments: GetOriginalName Internal, used InternalsVisibleTo instead
9b0a73b 
- reverted a change making GetOriginalName public
- included 'GraphNodeManagerViewExtension' to see internals from DynamoCore using InternalsVisibleTo
- comment fix
@dnenov
Summary Comments Added to FilterViewModel
cd5d3c1 
- added comments to public properties
@dnenov
Fixed Summary Comments After Change
7365210 
- added the additional parameter that was recently introduced to the comment
@dnenov
Comments: IEnumerable to Array Simplified
b73cf21 
- changed the way we get an array from the ItemsSource
@dnenov
Comments: Braces
bbb88f3 
- minor change
@dnenov
SearchTextBox Update
bfb9187 
- added a localization resource for 'Search' watermark text
- 'Search' watermark will now disappears on mouse entering the textbox
dnenov added 10 commits July 27, 2022 13:12
@dnenov
DataGrid Visual Changes
88db85f 
- removed cell margin to create 'continuous' vertical lines
- aligned icons and headings
@dnenov
Export Dual Tooltip
ec981e1 
- added an alternative tooltip that shows when nodes are being filtered
@dnenov
Select Node Behavior Change
2cf1df8 
- now would always select (zoom in around) a Node, so if the user navigates away in the Canvas, they can zoom back around the Node through GNM
@dnenov
Multiple Visual Updates
954a3ea 
- set negative left/right margin to create 'borderless' look
- updated hidden icon (resolution)
- multiple visual tweaks to the datagrid lement
@dnenov
Warning/Error Text Contrast
9b2ed4b 
- updated font families to default to ArtifaktElementBold for main text and ArtifaktElementRegular for detail information text
- updated detailed information text to the default Text color
@dnenov
Hover State Colors, Consistent Selection Color
a1ebe97 
- changed alternating row hover colors to be different to the selected row color
- replaced hard-coded color values with resource colors
@dnenov
Dismissed Warning Italic Font
ff962a9 
- now styles dismissed warnings message in Italic (missing Artefakt Italic resource, placehoder Ariel at the moment)
@dnenov
Any Empty Lists and Null
d2a4356 
- changed the methods to detect if a node contains Empty List or Null to any instead of all (will now show the respective symbol if any occurrences are found in the node outputs)
@dnenov
GNM EmptyList and Null Test
9622fc1 
- added unit test checking if the number of Nodes containing Null or Empty List matches what is shown on the UI
- added a new dynamo graph to test against
@dnenov
Remove Dismissed Icon
6ca914b 
- removed icon when a warning is dismissed
@reddyashish reddyashish marked this pull request as ready for review August 3, 2022 23:09
dnenov added 8 commits August 5, 2022 09:10
@dnenov
Naming Convention
0837fe3 
- removed underscore to field name for dismissedAlertsCount in NodeModel
@dnenov
Images as Resource
4580258 
- Filter Items now use images as resources converting them to ImageSource from bitmap
@dnenov
Removed/Replaced Converters
5615669 
- replaced converters with Dynamo converters
- removed old and unused converters
@dnenov
Color Resources Replaced
dcdf553 
- replaced most of the color resources with the identical counterparts already present in the main dynamo resource dictionaries
@dnenov
Converter Namespace Rename
d7a8e1f 
- renamed the namespace containing all extension specific converters to a more generic name
@dnenov
Merge branch 'master' of https://github.com/DynamoDS/Dynamo into dnen…
a97c133 
…ov-update-graphnodemanager
@dnenov
Merge Fix
07125c9 
- fixed small issue after merge on upstream master
@dnenov
Reverted Changes, Fix Tests Fail
5a07586 
- reverted changes made to the DynamoModern dictionary - the changes were breaking 2 tests
@reddyashish
Copy link
Contributor

Thanks Deyan for fixing the tests. Running the CI job again to make sure of no regressions.

@reddyashish
Copy link
Contributor

Self-serve CI passed: https://master-15.jenkins.autodesk.com/view/DYN/job/DYN-DevCI_Self_Service/1080/

@reddyashish reddyashish merged commit 85a4339 into DynamoDS:master Aug 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mjkkirschner mjkkirschner mjkkirschner left review comments

@reddyashish reddyashish reddyashish approved these changes

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
@dnenov @reddyashish @mjkkirschner