Footer #12774
Merged
Footer #12774
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- added 2 new resources related to run completing with error - the HomeWorkspaceViewModel now captures the case when the run has finished with error
- aligned the visual style of the footer notification text element to the design intent - changed border colors to match current design style - fixed animation (it wasn't working at all before) of the element, but question remains if that's the desired behaviour?
Major - added a new ItemsControl icon element to the footer displaying the number of errors, warnings and info states (not implemented yet) on Run - added a new ObservableCollection to the HomeWorkspaceViewModel to bind to - added a new FooterNotificationItem class to contain the properties of the new element Minor - a new visual brush style added - HomeSpaceViewModel property of the DynamoViewModel changed to cast to HomeWorkspaceViewModel (from WorkspaceViewModel) to rectify some of the resolve issues in the NotificationsControl view
- remove the text label from the footer notification item when 0 of the notification type (errors, warnings, info) are found
- added a new DynamoViewTests class for Dynamo UI tests - added a test to check for -- notification text when Running as script with an Error, Warning and no state -- the correct states of the Footer Notification Items - added names to some of the elements of the NotificationsControl for ease of targetting
mjkkirschner
reviewed
Apr 4, 2022
| footerItems.Add( new FooterNotificationItem() { NotificationCount = Model.Nodes.Count(n => n.State == ElementState.Error), NotificationImage = "/DynamoCoreWpf;component/UI/Images/error.png" }); | ||
| footerItems.Add( new FooterNotificationItem() { NotificationCount = Model.Nodes.Count(n => n.State == ElementState.Warning || n.State == ElementState.PersistentWarning), NotificationImage = "/DynamoCoreWpf;component/UI/Images/warning_16px.png" }); | ||
|
|
||
| FooterNotificationItems = new ObservableCollection<FooterNotificationItem>( footerItems ); |
There was a problem hiding this comment.
I don't think this is a good pattern, all the old changed events will be no longer be valid, I think it's better to reuse the same collection.
There was a problem hiding this comment.
Really appreciate the comment, @mjkkirschner! I made a tweak to the code and hopefully addressed your concern. Let me know if that's still iffy and I will follow up to the best of my abilities!
|
@dnenov looking pretty awesome! A couple of comments:
|
|
In addition, the point of the persistent box was to stop the "Bouncing" of the boxes around - so IMHO we should still keep the warning boxes but put them as grey, so that users know that they "Can be counted". Grey isn't scary, and we can have a state-based hover message if needed to bring clarity. |
That makes sense! We can grey them out and shows "No warning/error" when hovering |
- change to the way the FooterNotificationItems ObservableCollection is initialized and updated introducing reusability
- correctly implemented the NotificationObject on the level of the properties of the FooterNotificationItem class to allow the update of the binding
- added the ability to grey-out notification images if counter value is 0 meaning that after the run has completed there are no Error/Warning/Info nodes respectively
- removed legacy animation to the message box - added a WIP default text to the message box - minor visual adjustments
- added tooltips to the notification items
- constraint to the Run Message Box by giving it a minimal width. The minimal width should accomodate the longest typical messages while retaining the ability of the control to expand on longer messages (different languages) - added a Resource Property value for a pre-run state called "RunRead"
- swap the places of the message box and the footer icons. This should prevent the icons from "jumping" as the text length changes between runs
reddyashish
reviewed
Apr 8, 2022
reddyashish
reviewed
Apr 8, 2022
reddyashish
reviewed
Apr 8, 2022
reddyashish
reviewed
Apr 8, 2022
reddyashish
reviewed
Apr 8, 2022
reddyashish
reviewed
Apr 8, 2022
- revert HomeWorkspaceViewModel to WorkspaceViewModel because of return type issue - recreated a property inside the OutPortViewModel that was deleted by accident
- implemented the info state - implemented mouse interaction with notification elements to zoom around the respective nodes
- allows the user to cycle through the affected nodes zooming on each individual one at a time - finalized design intent for location and behavior of the notification elements
|
Looks good to me. You have some conflicts on this PR. |
- revert HomeWorkspaceViewModel return type
- implemented user interactivity by introducing a hover state to the element
- closed tags in Resources and resources.en-US
|
Please check for the failing tests on self-serve and we can merge this after we fix it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose
This PR contains changes and additions to the footer of the Dynamo View and in relation to the Dynamo Graph Node Manager task.
Declarations
Check these if you believe they are true
*.resxfilesRelease Notes
Visual fix to UI feature + new UI feature around footer Notifications Control in Dynamo View.
Reviewers
@reddyashish
FYIs
@mjkkirschner
@Amoursol