Rework toolbar design #5706

julienamsellem · 2021-09-17T09:01:30Z

Purpose of this PR

Jira
https://jira.unity3d.com/browse/GVFXG-143
Design docs
https://docs.google.com/document/d/1oG7-tKzlEDqQCRnw0KVsRqldVUyTY5gBfVuF-5tPx2Q/edit
https://docs.google.com/document/d/1VS4LKjZmfx40kutvb39h3sIL6UCSM9m92IxuluxGTeI/edit#

⚠This PR is a duplicate of this PR #5671 which I merged too soon by accident.

Before

After

Testing status

Tested on dark and light themes with 100% and 150% DPI screens

Comments to reviewers

Known bug: when DPI is different than 100% the drop down popups can be position at a "not ideal" position.
The bug is most probably on UIToolkit side. I warned the #dev-uitoolkit channel about it

Improved the toolbar dropdown button design

Take the left separator into account for popup alignment and prefer bottom left or top left alignment.

…tation

… button

…e the popup is visible

…ment issue The remaining issue is when the window spans on two screens (even if it's only 1px)

When the VFX Graph window is docked (because then it's destroyed and recreated)

…gn-2

Also always keep the help dropdown enabled and moved the "back" button to the right of the attach buttons. Dropdown buttons are now grayed out when disabled

github-actions · 2021-09-17T09:01:46Z

Hi! This comment will help you figure out which jobs to run before merging your PR. The suggestions are dynamic based on what files you have changed.
Link to Yamato: https://yamato.cds.internal.unity3d.com/jobs/902-Graphics
Search for your PR branch using the sidebar on the left, then add the following segment(s) to the end of the URL (you may need multiple tabs depending on how many packages you change)

VFX
/.yamato%252Fall-vfx.yml%2523PR_VFX_trunk

Depending on the scope of your PR, you may need to run more jobs than what has been suggested. Please speak to your lead or a Graphics SDET (#devs-graphics-automation) if you are unsure.

github-actions · 2021-09-17T09:02:00Z

It appears that you made a non-draft PR!
Please convert your PR to draft (button on the right side of the page).
See the PR template for more information.
Thank you!

VitaVFX

Looking good! All the raised issues are fixed, couldn't find anything outstanding on the last QA pass. 🟢

Thanks for the great PR, new toolbar looking very elegant in VFX window!

Test Doc

julienamsellem · 2021-09-17T09:10:42Z

com.unity.visualeffectgraph/Editor/GraphView/VFXViewWindow.cs

+
+            if (!graphView.TryAttachTo(effectToAttach))
+            {
+                s_LastAttachedComponent = null;


The introduction of this static variable was done to fix a bug when "unmaximizing" the window. Because in that process the window is recreated and then losing the attachment.

julienamsellem · 2021-09-17T09:11:33Z

com.unity.visualeffectgraph/Editor/GraphView/VFXViewWindow.cs

+            graphView.UpdateIsSubgraph();
+        }
+
+        public bool CanPopResource()


I introduced a "back" button when we entered a subgraph, this is to know if the button should be displayed or not

julienamsellem · 2021-09-17T09:12:35Z

com.unity.visualeffectgraph/Editor/GraphView/Views/DropDownButtonBase.cs

@@ -0,0 +1,197 @@
+using System;


This is a reusable class to add dropdowns (a button and an arrow button to open a popup).
It's used three times in this PR

julienamsellem · 2021-09-17T09:13:08Z

com.unity.visualeffectgraph/Editor/GraphView/Views/DropDownButtonBase.cs

+            m_PopupClosedTimestamp = DateTime.UtcNow;
+        }
+
+        private void OnKeyUp(KeyUpEvent evt)


These last three methods were added to handle keyboard navigation (either with Tab or arrow keys). This was requested by UX designers

julienamsellem · 2021-09-17T09:13:37Z

com.unity.visualeffectgraph/Editor/GraphView/Views/VFXView.cs

        {
            string path = string.Format("{0}/VFX/{1}.png", VisualEffectAssetEditorUtility.editorResourcesPath, text);
-            return AssetDatabase.LoadAssetAtPath<Texture2D>(path);
+            return EditorGUIUtility.LoadIcon(path);


This change has nothing to do with the PR, but while managing icons I discovered that our icons were blurred in the graph because only the low res (16px) versions were used. Now if relevant it uses the @2x version.

julienamsellem · 2021-09-17T09:15:59Z

com.unity.visualeffectgraph/Editor/GraphView/Views/VFXView.cs

        [NonSerialized]
        List<IconBadge> m_CompileBadges = new List<IconBadge>();

+        private void SetToolbarEnabled(bool enabled)


Even when no asset is opened we want the help button to stay available, so we don't disable the whole toolbar anymore

iTris666

A very few non blocking remarks. Approved

com.unity.visualeffectgraph/Editor/GraphView/VFXComponentBoard.cs

com.unity.visualeffectgraph/Editor/GraphView/Views/VFXCompileDropdownButton.cs

…olbar-design-2' into vfx/feature/GVFXG-143-improve-toolbar-design-2

Vic-Cooper

No docs changes in this PR :)

julienamsellem added 30 commits August 11, 2021 13:40

First step in Toolbar UI/UX rework

be358b1

[WIP] Reorganize VFX graph toolbar with dropdowns

1c33600

Created a base class for the Dropdown toolbar button and added an icon

657da4e

Implemented behaviors for all submenu commands

d3f71ab

Improved the toolbar dropdown button design

Better control dropdown popup position

c47b092

Take the left separator into account for popup alignment and prefer bottom left or top left alignment.

Moved dropdown button classes as separate files

3298f90

Implemented a "responsiveness" behavior in the vfx toolbar

465ac3f

Fixed compile menu commands

45d6fc8

Added a new icon for compile button and better theme support implemen…

ceb838d

…tation

Updated icons and removed labels. Also added missing resync materials…

eb65d1b

… button

Refined a few borders and added an icon for target game object panel

108a1f3

Updated changelog

0739519

Fixed formatting issues

72ea906

Added missing 3 dots for command that open an popup

cd3ca14

Added support for arrow navigation in custom dropdown menus

8a2a97e

Do not close and reopen the popup if the arrow button is pressed whil…

4d796e6

…e the popup is visible

Force popup position after call to show. This partially fix DPI place…

8c96576

…ment issue The remaining issue is when the window spans on two screens (even if it's only 1px)

Force expected popup size otherwise it can be overridden by the API

f3c03e6

Tweaked a little bit the popup sizes

43ea1d6

Implemented a working "save as" command

5127a63

Internet links are now represented as link instead of buttons

43e3122

Removed unexpected debug log

f5242fb

Make the main help button action more obvious (opens a web page)

af848d1

[Github] is nicer than Github:

af37f36

In SaveAs, use the same file extension as the original file

4c03eba

Fixed ".." in file extension and avoid saving if "save as" is canceled

3958aa3

Added a back button when the graph is opened in a subgraph

a84f53f

Allow to re-import a sample package that has already been imported

bd0cd62

Hide "Checkout" button if not available

a7e70c5

Added tooltips on almost all dropdown menu entries

4ebf9d0

julienamsellem added 12 commits August 31, 2021 18:24

Fixed code formatting issues

7008a0b

Removed commented out code

c6f4e64

Removed unused code

44c9d40

Fixed VFXViewWindow.currentWindow could be null after unmaximizing

39ce14d

Show the red dot in the record button from the beginning

4ed6177

Updated the "Resync material" tooltip

3f83b3b

Keep attached component when unmaximizing the scene view

16e2e4f

When the VFX Graph window is docked (because then it's destroyed and recreated)

Fixed wrong tooltip

294a443

Fixed a small typo in a style

799b47a

Merge branch 'master' into vfx/feature/GVFXG-143-improve-toolbar-desi…

49a60e7

…gn-2

Fixed help popup placement when on a screen edged.

353aeb0

Also always keep the help dropdown enabled and moved the "back" button to the right of the attach buttons. Dropdown buttons are now grayed out when disabled

Fixed sample could not be imported because of wrong package version

2cdd218

github-actions bot added the vfx label Sep 17, 2021

julienamsellem requested a review from iTris666 September 17, 2021 09:01

julienamsellem requested review from Vic-Cooper, VitaVFX and peeweek September 17, 2021 09:02

VitaVFX approved these changes Sep 17, 2021

View reviewed changes

VitaVFX changed the title ~~Vfx/feature/gvfxg 143 improve toolbar design 2~~ Rework toolbar design Sep 17, 2021

julienamsellem commented Sep 17, 2021

View reviewed changes

iTris666 approved these changes Sep 17, 2021

View reviewed changes

com.unity.visualeffectgraph/Editor/GraphView/VFXComponentBoard.cs Outdated Show resolved Hide resolved

com.unity.visualeffectgraph/Editor/GraphView/Views/VFXCompileDropdownButton.cs Show resolved Hide resolved

julienamsellem added 2 commits September 21, 2021 11:57

Applied code review comments

31d136d

Merge remote-tracking branch 'origin/vfx/feature/GVFXG-143-improve-to…

32c2ff5

…olbar-design-2' into vfx/feature/GVFXG-143-improve-toolbar-design-2

peeweek approved these changes Sep 22, 2021

View reviewed changes

Vic-Cooper approved these changes Sep 22, 2021

View reviewed changes

julienamsellem merged commit fd214da into master Sep 22, 2021

julienamsellem deleted the vfx/feature/GVFXG-143-improve-toolbar-design-2 branch September 22, 2021 11:42

julienamsellem mentioned this pull request Sep 23, 2021

Vfx/fix/missing lfs icons #5787

Merged

Rework toolbar design #5706

Rework toolbar design #5706

Uh oh!

Conversation

julienamsellem commented Sep 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose of this PR

Testing status

Comments to reviewers

Uh oh!

github-actions bot commented Sep 17, 2021

Uh oh!

github-actions bot commented Sep 17, 2021

Uh oh!

VitaVFX left a comment

Choose a reason for hiding this comment

Uh oh!

julienamsellem Sep 17, 2021

Choose a reason for hiding this comment

Uh oh!

julienamsellem Sep 17, 2021

Choose a reason for hiding this comment

Uh oh!

julienamsellem Sep 17, 2021

Choose a reason for hiding this comment

Uh oh!

julienamsellem Sep 17, 2021

Choose a reason for hiding this comment

Uh oh!

julienamsellem Sep 17, 2021

Choose a reason for hiding this comment

Uh oh!

julienamsellem Sep 17, 2021

Choose a reason for hiding this comment

Uh oh!

iTris666 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Vic-Cooper left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

julienamsellem commented Sep 17, 2021 •

edited

Loading