Dyn-5473 workspace toolbar #14483

jesusalvino · 2023-10-12T23:48:30Z

Purpose

Implementing the Tech debt https://jira.autodesk.com/browse/DYN-5473 to deal with the Toolbar overlapping over the workspace tabs.

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
This PR contains no files larger than 50 MB

Reviewers

@QilongTang
@reddyashish

FYIs

@RobertGlobant20
@Enzo707

jesusalvino · 2023-10-13T00:01:26Z

The evidence gif exceeds the size to be enclosed, here you are :

https://myshare.autodesk.com/:i:/g/personal/jesus_alfredo_alvino_autodesk_com1/Eayv-qLmNXVPvoAnvpi3W6sBEZ31yEC8mALpMj8C9mZmSA?e=jHk5GK

reddyashish · 2023-10-13T18:40:21Z

src/DynamoCoreWpf/ViewModels/Core/DynamoViewModel.cs

@@ -830,11 +830,19 @@ void Watch3DViewModelPropertyChanged(object sender, PropertyChangedEventArgs e)
            }
        }

+        internal event EventHandler WindowRezised;
+        internal void OnWindowResized(object lessthan1000)


Not a good idea to have numerics in variable name. Change to something else?

reddyashish · 2023-10-13T18:45:08Z

src/DynamoCoreWpf/Views/Core/DynamoView.xaml.cs

            this.dynamoViewModel.RequestReturnFocusToView += OnRequestReturnFocusToView;
            this.dynamoViewModel.Model.WorkspaceSaving += OnWorkspaceSaving;
            this.dynamoViewModel.Model.WorkspaceOpened += OnWorkspaceOpened;
+            this.dynamoViewModel.Model.WorkspaceAdded += OwnWorkspaceAdded;


Typo OnWorkspaceAdded*

reddyashish · 2023-10-13T18:52:59Z

src/DynamoCoreWpf/ViewModels/Core/ShortcutToolbarViewModel.cs

+
+        private void DynamoViewModel_WindowRezised(object sender, System.EventArgs e)
+        {
+            ViewButtonsText = !(bool)sender;


It would be good to make validate that a bool value is passed here, otherwise the casting will throw an exception.

We can use Show/HideText, it is not guaranteed a button either, right? One of the case is a menu item

QilongTang · 2023-10-16T16:06:09Z

src/DynamoCoreWpf/Views/Core/DynamoView.xaml.cs

+            CalculateWindowThreshold();
+        }
+
+        internal void CalculateWindowThreshold()


Comments, what does it do?

QilongTang · 2023-10-16T16:06:14Z

src/DynamoCoreWpf/Views/Core/DynamoView.xaml.cs

+            dynamoViewModel.OnWindowResized(dynamoViewModel.Model.PreferenceSettings.WindowW <= threshold);            
+        }
+
+        internal void CalculateWindowMinWidth()


Comments, what does it do?

QilongTang · 2023-10-16T16:10:07Z

src/DynamoCoreWpf/ViewModels/Core/ShortcutToolbarViewModel.cs


        public ShortcutToolbarViewModel(DynamoViewModel dynamoViewModel)
        {
            NotificationsNumber = 0;
            authManager = dynamoViewModel.Model.AuthenticationManager;
            ValidateWorkSpaceBeforeToExportAsImageCommand = new DelegateCommand(dynamoViewModel.ValidateWorkSpaceBeforeToExportAsImage);
            SignOutCommand = new DelegateCommand(authManager.ToggleLoginState);
-            authManager.LoginStateChanged += (o) => { RaisePropertyChanged(nameof(LoginState)); };
+            authManager.LoginStateChanged += (o) => { RaisePropertyChanged(nameof(LoginState)); };            
+            dynamoViewModel.WindowRezised += DynamoViewModel_WindowRezised;


unsubscribe?

QilongTang · 2023-10-16T16:17:56Z

src/DynamoCoreWpf/ViewModels/Core/ShortcutToolbarViewModel.cs

@@ -70,5 +78,18 @@ public bool IsNotificationsCounterVisible
                return true;
            }
        }
+
+        public bool ViewButtonsText


Please update name and add comments as suggested above

reddyashish · 2023-10-20T14:51:08Z

@jesusalvino It looks like there are failures on this PR. Can you take a look at it once.

reddyashish · 2023-10-20T14:51:08Z

@jesusalvino It looks like there are failures on this PR. Can you take a look at it once.

jesusalvino · 2023-10-20T15:17:11Z

@jesusalvino It looks like there are failures on this PR. Can you take a look at it once.

that's weird, lot of tests failed but most of them passed locally, anyway already started them on the master-15

QilongTang · 2023-10-20T15:48:14Z

src/DynamoCoreWpf/ViewModels/Core/ShortcutToolbarViewModel.cs

@@ -70,5 +81,24 @@ public bool IsNotificationsCounterVisible
                return true;
            }
        }
+
+        public bool ShowMenuItemText


comments please

QilongTang · 2023-10-20T15:48:51Z

src/DynamoCoreWpf/Views/Core/DynamoView.xaml.cs

@@ -101,6 +103,8 @@ public partial class DynamoView : Window, IDisposable
            get { return preferencesWindow; }
        }

+        internal double DefaultMinWidth = 0;


comments please, especially this is defined in DynamoView

QilongTang · 2023-10-20T15:48:56Z

src/DynamoCoreWpf/Views/Core/DynamoView.xaml.cs

@@ -84,6 +84,8 @@ public partial class DynamoView : Window, IDisposable
        private bool isPSSCalledOnViewModelNoCancel = false;
        private readonly DispatcherTimer _workspaceResizeTimer = new DispatcherTimer { Interval = new TimeSpan(0, 0, 0, 0, 500), IsEnabled = false };
        private ViewLoadedParams sharedViewExtensionLoadedParams;
+        private double toolBarRightMenuWidth = 0;
+        private double additionalWidth = 50;


comments please, especially this is defined in DynamoView

QilongTang

Some last comments, as long as regressions are addressed

jesusalvino · 2023-10-20T20:48:11Z

Some last comments, as long as regressions are addressed

Done

Jesus Alfredo Alviño added 2 commits October 12, 2023 17:56

Dealing with the Workspace Tabs, Window Threshold and Toolbar buttons

72a6fe0

Renaming method

fa72ae7

reddyashish reviewed Oct 13, 2023

View reviewed changes

QilongTang reviewed Oct 16, 2023

View reviewed changes

avidit changed the title ~~Dyn 5473 workspace toolbar~~ Dyn-5473 workspace toolbar Oct 16, 2023

Refactoring and adding comments

36d8511

reddyashish approved these changes Oct 19, 2023

View reviewed changes

reddyashish added this to the 3.0 milestone Oct 19, 2023

QilongTang reviewed Oct 20, 2023

View reviewed changes

Dealing the NaN and adding comments

fb6809a

Set the whole width space to deal with the controls properly

22c6d17

QilongTang approved these changes Oct 23, 2023

View reviewed changes

QilongTang merged commit bdc6ba4 into DynamoDS:master Oct 23, 2023
16 of 17 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dyn-5473 workspace toolbar #14483

Dyn-5473 workspace toolbar #14483

jesusalvino commented Oct 12, 2023

jesusalvino commented Oct 13, 2023 •

edited

Loading

reddyashish Oct 13, 2023

reddyashish Oct 13, 2023

reddyashish Oct 13, 2023 •

edited

Loading

QilongTang Oct 16, 2023

QilongTang Oct 16, 2023

QilongTang Oct 16, 2023

QilongTang Oct 16, 2023

QilongTang Oct 16, 2023

reddyashish commented Oct 20, 2023

reddyashish commented Oct 20, 2023

jesusalvino commented Oct 20, 2023

QilongTang Oct 20, 2023

QilongTang Oct 20, 2023

QilongTang Oct 20, 2023

QilongTang left a comment

jesusalvino commented Oct 20, 2023

Dyn-5473 workspace toolbar #14483

Dyn-5473 workspace toolbar #14483

Conversation

jesusalvino commented Oct 12, 2023

Purpose

Declarations

Reviewers

FYIs

jesusalvino commented Oct 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reddyashish Oct 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reddyashish commented Oct 20, 2023

reddyashish commented Oct 20, 2023

jesusalvino commented Oct 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

QilongTang left a comment

Choose a reason for hiding this comment

jesusalvino commented Oct 20, 2023

jesusalvino commented Oct 13, 2023 •

edited

Loading

reddyashish Oct 13, 2023 •

edited

Loading