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

DYN-6411 update version check #14583

Merged
merged 10 commits into from
Nov 14, 2023
Merged

DYN-6411 update version check #14583

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
2517c29
update version check
dnenov Nov 8, 2023
4a5229c
Merge branch 'master' into pm-updateversion-fix
dnenov Nov 9, 2023
02063f6
comments
dnenov Nov 9, 2023
6d5462a
Merge branch 'master' into pm-updateversion-fix
dnenov Nov 10, 2023
8c6004b
refactor anonymous _searchPkgsView.Closed event handling
dnenov Nov 13, 2023
40482fd
Merge remote-tracking branch 'upstream/master' into pm-updateversion-fix
dnenov Nov 13, 2023
06bf0af
refactor update
dnenov Nov 13, 2023
8f4776a
Update DynamoView.xaml.cs
dnenov Nov 13, 2023
e6a11ab
Merge remote-tracking branch 'upstream/master' into pm-updateversion-fix
dnenov Nov 14, 2023
9e4e933
test fixes
dnenov Nov 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 31 additions & 23 deletions src/DynamoCoreWpf/Controls/InstalledPackagesControl.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
ItemsSource="{Binding Path=Filters}"
Background="Transparent"
KeyboardNavigation.IsTabStop="False"
VerticalContentAlignment="Center"
Padding="0,5,0,5">
<ItemsControl.ItemsPanel>
<ItemsPanelTemplate>
Expand All @@ -53,17 +54,21 @@
Style="{StaticResource {x:Type ToggleButton}}"
FontSize="10"
FontFamily="Artifakt Element"
FontWeight="Medium"
HorizontalAlignment="Center"
BorderThickness="1,1,1,1"
Background="{StaticResource PrimaryCharcoal300Brush}"
BorderThickness="1"
BorderBrush="Transparent"
UseLayoutRounding="True"
SnapsToDevicePixels="True"
Background="#CCCCCC"
Checked="ToggleButton_OnChecked"
Foreground="{StaticResource ExpanderCaretToggleButtonBackground}">
Foreground="{StaticResource PackageManagerTabBackgroundColor}">
<RadioButton.Resources>
<Style TargetType="Border">
<Setter Property="CornerRadius" Value="5"/>
<Setter Property="CornerRadius" Value="8"/>
<Setter Property="HorizontalAlignment" Value="Stretch"/>
<Setter Property="Margin" Value="5,0,0,5"/>
<Setter Property="Padding" Value="5,0,5,0"/>
<Setter Property="Margin" Value="5,3,0,0"/>
<Setter Property="Padding" Value="5,1,5,0"/>
</Style>
</RadioButton.Resources>
</RadioButton>
Expand Down Expand Up @@ -92,21 +97,20 @@
ItemsPresenter can't be wider then visible part of the window.-->
<Border BorderBrush="{StaticResource PreferencesWindowBackgroundColor}"
BorderThickness="0,0,0,1"
Background="{StaticResource PreferencesWindowBackgroundColor}"
Background="{StaticResource PackageManagerTabBackgroundColor}"
Width="{Binding Path=ActualWidth, RelativeSource={RelativeSource FindAncestor,
AncestorType={x:Type ItemsPresenter}}}">
<Expander Name="PackageHeader"
IsExpanded="{Binding Path=Model.TypesVisibleInManager}"
Style="{StaticResource InstalledPackagesExpanderStyle}">
IsExpanded="{Binding Path=Model.TypesVisibleInManager}"
Style="{StaticResource InstalledPackagesExpanderStyle}">
<Expander.Header>
<Grid x:Name="PackageGrid" >
<DockPanel HorizontalAlignment="Stretch"
>
<DockPanel HorizontalAlignment="Stretch">
<StackPanel HorizontalAlignment="Left"
Orientation="Horizontal"
Margin="10,5,5,7">
<TextBlock Text="{Binding Path=Model.Name}"
FontSize="11"
FontSize="12"
Width="150"
TextTrimming="CharacterEllipsis"
Margin="15,0,10,0"
Expand All @@ -118,29 +122,33 @@
<TextBlock Text="{Binding Path=Model.VersionName}"
MinWidth="70"
FontSize="11"
VerticalAlignment="Center"
Foreground="{StaticResource PreferencesWindowFontColor}"></TextBlock>

</StackPanel>
<StackPanel HorizontalAlignment="Right" Orientation="Horizontal" Width="Auto">
<Grid Height="18"
MaxWidth="150">
<Border Background="DimGray"
HorizontalAlignment="Stretch"
CornerRadius="5"
Margin="5,0,0,0"
Padding="5,0,5,0">
MaxWidth="150"
Margin="10 0">
<Border Background="#CCCCCC"
HorizontalAlignment="Stretch"
CornerRadius="8"
Margin="5,0,0,0"
Padding="5,0,5,0">
<Border.ToolTip>
<ToolTip Content="{Binding Path=PackageLoadStateTooltip}" Style="{StaticResource GenericToolTipLight}"/>
</Border.ToolTip>
<TextBlock Name="PackageStateLabel" Text="{Binding Path=PackageLoadStateText}"
TextTrimming="CharacterEllipsis"
FontSize="10"
FontFamily="Artifakt Element"
FontWeight="Medium"
TextAlignment="Left"
HorizontalAlignment="Center"
Margin="0,3,0,0"
Foreground="{StaticResource PrimaryCharcoal100Brush}"/>
Margin="0,2,0,0"
Foreground="{StaticResource PackageManagerTabBackgroundColor}"/>
</Border>
</Grid>
</StackPanel>
<StackPanel HorizontalAlignment="Right" Orientation="Horizontal" Width="Auto">
<Button Name="MoreButton"
Click="MoreButton_OnClick"
Width="16"
Expand Down Expand Up @@ -195,7 +203,7 @@
</MenuItem.ToolTip>
</MenuItem>
<MenuItem Name="MakeNewVersionButton"
Visibility="{Binding Path=CanPublish, Converter={StaticResource BooleanToVisibilityCollapsedConverter}}"
Visibility="{Binding Path=CanPublishNewVersion, Converter={StaticResource BooleanToVisibilityCollapsedConverter}}"
reddyashish marked this conversation as resolved.
Show resolved Hide resolved
Command="{Binding Path=PublishNewPackageVersionCommand}"
Header="{x:Static p:Resources.InstalledPackageViewContextMenuPublishVersion}">
<MenuItem.ToolTip>
Expand Down
6 changes: 4 additions & 2 deletions src/DynamoCoreWpf/UI/Themes/Modern/DynamoModern.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3683,8 +3683,8 @@
</Setter>
</Style>

<SolidColorBrush x:Key="ExpanderCaretToggleButtonBackground" Color="#4d4d4d" />
<SolidColorBrush x:Key="ExpanderCaretToggleButtonSelectedBackground" Color="#3c3c3c" />
<SolidColorBrush x:Key="ExpanderCaretToggleButtonBackground" Color="#2A2A2A" />
<SolidColorBrush x:Key="ExpanderCaretToggleButtonSelectedBackground" Color="#2F2E30" />

<ControlTemplate x:Key="ExpanderCaretToggleButton" TargetType="{x:Type ToggleButton}">
<Border Name="Border"
Expand Down Expand Up @@ -3775,6 +3775,8 @@


<Style x:Key="InstalledPackagesExpanderStyle" TargetType="Expander">
<Setter Property="UseLayoutRounding" Value="True"/>
<Setter Property="SnapsToDevicePixels" Value="True"/>
<Setter Property="Template">
<Setter.Value>
<!-- Control template for expander -->
Expand Down
2 changes: 1 addition & 1 deletion src/DynamoCoreWpf/ViewModels/PackageManager/PackageManagerSearchViewModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1612,7 +1612,7 @@ internal void PackageManagerViewClose()
internal void Dispose()
{
nonHostFilter?.ForEach(f => f.PropertyChanged -= filter_PropertyChanged);
aTimer.Elapsed -= OnTimedEvent;
if (aTimer != null) aTimer.Elapsed -= OnTimedEvent;

TimedOut = false; // reset the timedout screen
InitialResultsLoaded = false; // reset the loading screen settings
Expand Down
10 changes: 5 additions & 5 deletions src/DynamoCoreWpf/ViewModels/PackageManager/PackageViewModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
using Dynamo.PackageManager;
using Dynamo.Wpf.Properties;
using Dynamo.Wpf.Utilities;
using NotificationObject = Dynamo.Core.NotificationObject;
using Prism.Commands;
using NotificationObject = Dynamo.Core.NotificationObject;

namespace Dynamo.ViewModels
{
Expand Down Expand Up @@ -149,7 +149,7 @@ public bool HasNodeLibraries

public bool HasCustomNodes
{
get { return Model.LoadedCustomNodes.Any(); }
get { return Model.LoadedCustomNodes.Any(); }
}

public bool HasAssemblies
Expand Down Expand Up @@ -183,12 +183,12 @@ public PackageViewModel(DynamoViewModel dynamoViewModel, Package model)

ToggleTypesVisibleInManagerCommand = new DelegateCommand(() => { }, () => true);
GetLatestVersionCommand = new DelegateCommand(() => { }, () => false);
PublishNewPackageVersionCommand = new DelegateCommand(() => ExecuteWithTou(PublishNewPackageVersion), () => CanPublish);
PublishNewPackageVersionCommand = new DelegateCommand(() => ExecuteWithTou(PublishNewPackageVersion), IsOwner);
PublishNewPackageCommand = new DelegateCommand(() => ExecuteWithTou(PublishNewPackage), () => CanPublish);
UninstallCommand = new DelegateCommand(Uninstall, CanUninstall);
UnmarkForUninstallationCommand = new DelegateCommand(UnmarkForUninstallation, CanUnmarkForUninstallation);
LoadCommand = new DelegateCommand(Load, CanLoad);
DeprecateCommand = new DelegateCommand(Deprecate, CanDeprecate);
DeprecateCommand = new DelegateCommand(Deprecate, IsOwner);
UndeprecateCommand = new DelegateCommand(Undeprecate, CanUndeprecate);
GoToRootDirectoryCommand = new DelegateCommand(GoToRootDirectory, () => true);

Expand Down Expand Up @@ -408,7 +408,7 @@ private void Deprecate()
packageManagerClient.Deprecate(Model.Name);
}

private bool CanDeprecate()
private bool IsOwner()
{
if (!CanPublish) return false;
return packageManagerClient.DoesCurrentUserOwnPackage(Model, dynamoModel.AuthenticationManager.Username);
Expand Down
45 changes: 33 additions & 12 deletions src/DynamoCoreWpf/Views/Core/DynamoView.xaml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1308,7 +1308,7 @@ private void DynamoView_Loaded(object sender, EventArgs e)

#region Package manager

dynamoViewModel.RequestPackagePublishDialog += DynamoViewModelRequestRequestPackageManagerPublish;
dynamoViewModel.RequestPackagePublishDialog += DynamoViewModelRequestPackageManager;
dynamoViewModel.RequestPackageManagerSearchDialog += DynamoViewModelRequestShowPackageManagerSearch;
dynamoViewModel.RequestPackageManagerDialog += DynamoViewModelRequestShowPackageManager;

Expand Down Expand Up @@ -1464,23 +1464,37 @@ private void DynamoViewModelRequestAboutWindow(DynamoViewModel model)

private PublishPackageView _pubPkgView;

private void DynamoViewModelRequestRequestPackageManagerPublish(PublishPackageViewModel model)
private void DynamoViewModelRequestPackageManager(PublishPackageViewModel model)
{
var cmd = Analytics.TrackCommandEvent("PublishPackage");
if (_pubPkgView == null)
if (packageManagerWindow == null)
{
_pubPkgView = new PublishPackageView(model)
if (_pkgSearchVM == null)
{
_pkgSearchVM = new PackageManagerSearchViewModel(dynamoViewModel.PackageManagerClientViewModel);
}

if (_pkgVM == null)
{
_pkgVM = new PackageManagerViewModel(dynamoViewModel, _pkgSearchVM);
}

packageManagerWindow = new PackageManagerView(this, _pkgVM)
{
Owner = this,
WindowStartupLocation = WindowStartupLocation.CenterOwner
};
_pubPkgView.Closed += (sender, args) => { _pubPkgView = null; cmd.Dispose(); };
_pubPkgView.Show();

if (_pubPkgView.IsLoaded && IsLoaded) _pubPkgView.Owner = this;
// setting the owner to the packageManagerWindow will centralize promts originating from the Package Manager
dynamoViewModel.Owner = packageManagerWindow;
Copy link
Contributor

Choose a reason for hiding this comment

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

This line does not look correct to me... did I miss anything?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The way I used it and the way I understand it is, whichever more major View 'takes control', gets ownership of this property. The same thing happens in the PreferencesView:

Dynamo/src/DynamoCoreWpf/Views/Menu/PreferencesView.xaml.cs

Line 77 in caa3268

dynViewModel.Owner = this;

The Owner property is used when utilizing the MessageBoxService:

public static MessageBoxResult Show(Window owner,string msg, string title, MessageBoxButton button, MessageBoxImage img)

So when we launch message boxes from inside the DynamoViewModel, we are centralizing them around that Owner.

Another thing it does is to keep the reference of the current window high up in the hierarchy. For example, in one instance, we are calling the UnninstallCommand which is part of the PackageViewModel from deep inside the PacakgeManagerView. The PackageViewModel and the Uninstall method do not have a reference to the PackageManagerView, though, but they do have a reference to the DynamoViewModel. So when prompting the user, we are setting the Owner of the DynamoViewModel, which is the PackageManagerView, so the prompt is correctly centralized.

I did not do a good job of cleaning up this logic, the logic exists from before. Maybe I did not fully understand it myself, this is my current working logic of it.

Copy link
Contributor

Choose a reason for hiding this comment

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

After checking out the code, I think I understand what you are saying. I believe the Owner property was created so DynamoViewModel could have a reference to the view as a convenient way to raise message box with the Owner correctly passed in. But it is not meant to be served as the property indicating current Window, because I do not see any logic to reset that Owner property back to DynamoView. I can foresee a bug that after Preferences panel opened and dragged out of the screen, then closed, new message boxes using such owner will be out of DynamoView.

But thank you for pointing this out, I do think we provided bad examples in code so it's good for this PR but I will file a follow up task to address that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I understand. If you want me, I can try to trace all the moments where we are setting using message boxes and try to remove that need by setting the Onwer directly somehow.


packageManagerWindow.Closed += HandlePackageManagerWindowClosed;
packageManagerWindow.Show();

if (packageManagerWindow.IsLoaded && IsLoaded) packageManagerWindow.Owner = this;
}

_pubPkgView.Focus();
packageManagerWindow.Focus();
packageManagerWindow.Navigate(Wpf.Properties.Resources.PackageManagerPublishTab);
}

private PackageManagerSearchView _searchPkgsView;
Expand Down Expand Up @@ -1940,7 +1954,7 @@ private void WindowClosed(object sender, EventArgs e)
dynamoViewModel.RequestViewOperation -= DynamoViewModelRequestViewOperation;

//PACKAGE MANAGER
dynamoViewModel.RequestPackagePublishDialog -= DynamoViewModelRequestRequestPackageManagerPublish;
dynamoViewModel.RequestPackagePublishDialog -= DynamoViewModelRequestPackageManager;
dynamoViewModel.RequestPackageManagerSearchDialog -= DynamoViewModelRequestShowPackageManagerSearch;

//FUNCTION NAME PROMPT
Expand Down Expand Up @@ -2242,7 +2256,6 @@ private void DynamoViewModelRequestShowPackageManager(object s, EventArgs e)
if (!DisplayTermsOfUseForAcceptance())
return; // Terms of use not accepted.

var cmd = Analytics.TrackCommandEvent("PackageManager");
if (_pkgSearchVM == null)
{
_pkgSearchVM = new PackageManagerSearchViewModel(dynamoViewModel.PackageManagerClientViewModel);
Expand All @@ -2264,7 +2277,7 @@ private void DynamoViewModelRequestShowPackageManager(object s, EventArgs e)
// setting the owner to the packageManagerWindow will centralize promts originating from the Package Manager
dynamoViewModel.Owner = packageManagerWindow;

packageManagerWindow.Closed += (sender, args) => { packageManagerWindow = null; cmd.Dispose(); };
packageManagerWindow.Closed += HandlePackageManagerWindowClosed;
packageManagerWindow.Show();

if (packageManagerWindow.IsLoaded && IsLoaded) packageManagerWindow.Owner = this;
Expand All @@ -2279,6 +2292,14 @@ private void DynamoViewModelRequestShowPackageManager(object s, EventArgs e)
_pkgSearchVM.RefreshAndSearchAsync();
}

private void HandlePackageManagerWindowClosed(object sender, EventArgs e)
{
packageManagerWindow.Closed -= HandlePackageManagerWindowClosed;
packageManagerWindow = null;

var cmd = Analytics.TrackCommandEvent("PackageManager");
cmd.Dispose();
}

internal void EnableEnvironment(bool isEnabled)
{
Expand Down
12 changes: 7 additions & 5 deletions src/DynamoCoreWpf/Views/PackageManager/Controls/PackageManagerPublishControl.xaml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
using Dynamo.PackageManager;
using System;
using System.Diagnostics;
using System.Windows;
using System.Windows.Controls;
using System.Windows.Input;
using System.Windows.Navigation;
using Dynamo.Logging;
using Dynamo.UI;
using Dynamo.ViewModels;
using DynamoUtilities;
using System.Diagnostics;
using Dynamo.Logging;

namespace Dynamo.PackageManager.UI
{
Expand All @@ -35,9 +34,12 @@ private void InitializeContext(object sender, RoutedEventArgs e)

PublishPackageViewModel = this.DataContext as PublishPackageViewModel;

PublishPackageViewModel.PublishSuccess += PackageViewModelOnPublishSuccess;
if(PublishPackageViewModel != null )
{
PublishPackageViewModel.PublishSuccess += PackageViewModelOnPublishSuccess;
PublishPackageViewModel.RequestShowFolderBrowserDialog += OnRequestShowFolderBrowserDialog;
}

PublishPackageViewModel.RequestShowFolderBrowserDialog += OnRequestShowFolderBrowserDialog;
Logging.Analytics.TrackScreenView("PackageManager");
}

Expand Down
7 changes: 5 additions & 2 deletions src/DynamoCoreWpf/Views/PackageManager/PackageManagerView.xaml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,11 @@ public PackageManagerView(DynamoView dynamoView, PackageManagerViewModel package

InitializeComponent();

packageManagerViewModel.PackageSearchViewModel.RequestShowFileDialog += OnRequestShowFileDialog;
packageManagerViewModel.PackageSearchViewModel.PackageManagerClientViewModel.ViewModelOwner = this;
if (packageManagerViewModel != null )
{
packageManagerViewModel.PackageSearchViewModel.RequestShowFileDialog += OnRequestShowFileDialog;
packageManagerViewModel.PackageSearchViewModel.PackageManagerClientViewModel.ViewModelOwner = this;
}

Dynamo.Logging.Analytics.TrackEvent(
Actions.Open,
Expand Down
6 changes: 3 additions & 3 deletions test/DynamoCoreWpfTests/PackageManager/PackageManagerUITests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public void CanOpenPackagePublishDialogAndWindowIsOwned()
var l = new PublishPackageViewModel(ViewModel);
ViewModel.OnRequestPackagePublishDialog(l);

AssertWindowOwnedByDynamoView<PublishPackageView>();
AssertWindowOwnedByDynamoView<PackageManagerView>();
}

[Test, Ignore("Unknown reason")]
Expand All @@ -125,8 +125,8 @@ public void PackagePublishWindowClosesWithDynamo()
var l = new PublishPackageViewModel(ViewModel);
ViewModel.OnRequestPackagePublishDialog(l);

AssertWindowOwnedByDynamoView<PublishPackageView>();
AssertWindowClosedWithDynamoView<PublishPackageView>();
AssertWindowOwnedByDynamoView<PackageManagerView>();
AssertWindowClosedWithDynamoView<PackageManagerView>();

}
#endregion
Expand Down
Loading