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

Dev drive insights and pip cache move rebased to changes from main #2486

Merged
merged 6 commits into from
Apr 1, 2024
+1,042 −42
Merged

Dev drive insights and pip cache move rebased to changes from main #2486

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6058ab8
Applied Dev Drive Insights changes on top of Windows Customization fr…
SohamDas2021 Mar 29, 2024
1a0d87f
Addressed PR comments 1
SohamDas2021 Mar 29, 2024
aa482b9
Addressed PR comments
SohamDas2021 Mar 29, 2024
f162c12
Addressed PR comments
SohamDas2021 Apr 1, 2024
a1a730a
More cleanups
SohamDas2021 Apr 1, 2024
54fd0cf
Merge branch 'main' into user/sodas/devdriveinsights_032824
SohamDas2021 Apr 1, 2024
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
3 changes: 3 additions & 0 deletions tools/Customization/DevHome.Customization/Extensions/ServiceExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using System;
using DevHome.Customization.ViewModels;
using DevHome.Customization.ViewModels.Environments;
using DevHome.Customization.Views;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
Expand All @@ -18,6 +20,7 @@ public static IServiceCollection AddWindowsCustomization(this IServiceCollection
services.AddSingleton<DeveloperFileExplorerViewModel>();
services.AddTransient<DeveloperFileExplorerPage>();

services.AddSingleton<OptimizeDevDriveDialogViewModelFactory>(sp => (cacheLocation, environmentVariable) => ActivatorUtilities.CreateInstance<OptimizeDevDriveDialogViewModel>(sp, cacheLocation, environmentVariable));
services.AddSingleton<DevDriveInsightsViewModel>();
services.AddTransient<DevDriveInsightsPage>();

Expand Down
10 changes: 9 additions & 1 deletion tools/Customization/DevHome.Customization/Strings/en-us/Resources.resw
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,10 @@
<resheader name="writer">
<value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
</resheader>
<data name="ChooseDirectoryPromptText" xml:space="preserve">
<value>Choose directory on dev drive...</value>
Copy link
Collaborator

@krschau krschau Apr 1, 2024 •
edited by SohamDas2021
Loading

Choose a reason for hiding this comment

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

Dev Drive should be capitalized, I believe. (Applies to all instances on this page)

Suggested change
<value>Choose directory on dev drive...</value>
<value>Choose directory on Dev Drive...</value>
``` #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is what it says in the Figma.

Copy link
Contributor

Choose a reason for hiding this comment

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

actually @SohamDas2021 it should be like Kristen outlined, If I recall, it was from Marketing who said that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So capitalize all Dev Drive in the text?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, thanks.

<comment>Choose directory on dev drive</comment>
</data>
<data name="DevDriveFreeSizeText" xml:space="preserve">
<value>{0} {1} free</value>
<comment>Dev drive size free</comment>
Expand Down Expand Up @@ -157,6 +161,10 @@
<value>Enable end task in taskbar by right click</value>
<comment>The description for the end task on task bar settings card</comment>
</data>
<data name="ExampleDevDriveLocation" xml:space="preserve">
<value>Example: E:\packages\pip</value>
<comment>Example dev drive location</comment>
</data>
<data name="EndTaskOnTaskBar.Header" xml:space="preserve">
<value>End Task</value>
<comment>The header for the end task on task bar settings card</comment>
Expand All @@ -177,7 +185,7 @@
<value>The environment variable {0} is set to {1}</value>
<comment>Optimized dev drive description</comment>
</data>
<data name="OptimizeDevDriveDialogDescription" xml:space="preserve">
<data name="OptimizeDevDriveDialogDescription.Text" xml:space="preserve">
<value>Contents of {0} will be copied to chosen directory. And {1} will be set to chosen directory.</value>
<comment>Optimize dev drive dialog description</comment>
</data>
Expand Down
44 changes: 7 additions & 37 deletions tools/Customization/DevHome.Customization/ViewModels/DevDriveInsightsViewModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,8 @@
using System.Threading.Tasks;
using CommunityToolkit.Mvvm.ComponentModel;
using CommunityToolkit.Mvvm.Input;
using CommunityToolkit.WinUI.Collections;
using DevHome.Common.Models;
using DevHome.Common.Services;
using DevHome.Customization.Models.Environments;
using DevHome.Customization.ViewModels.Environments;
using DevHome.Customization.Views;
using DevHome.SetupFlow.Services;
Expand All @@ -21,7 +19,7 @@

namespace DevHome.Customization.ViewModels;

public partial class DevDriveInsightsViewModel : SetupPageViewModelBase
public partial class DevDriveInsightsViewModel : ObservableObject
{
private readonly Microsoft.UI.Dispatching.DispatcherQueue _dispatcher;

Expand All @@ -31,6 +29,8 @@ public partial class DevDriveInsightsViewModel : SetupPageViewModelBase

public ObservableCollection<DevDriveOptimizedCardViewModel> DevDriveOptimizedCardCollection { get; private set; } = new();

private readonly OptimizeDevDriveDialogViewModelFactory _optimizeDevDriveDialogViewModelFactory;

[ObservableProperty]
private bool _shouldShowCollectionView;

Expand All @@ -53,50 +53,19 @@ public partial class DevDriveInsightsViewModel : SetupPageViewModelBase

private IEnumerable<IDevDrive> ExistingDevDrives { get; set; } = Enumerable.Empty<IDevDrive>();

public DevDriveInsightsViewModel(
ISetupFlowStringResource stringResource,
SetupFlowViewModel setupflowModel,
SetupFlowOrchestrator orchestrator,
IDevDriveManager devDriveManager,
ToastNotificationService toastNotificationService)
: base(stringResource, orchestrator)
public DevDriveInsightsViewModel(IDevDriveManager devDriveManager, OptimizeDevDriveDialogViewModelFactory optimizeDevDriveDialogViewModelFactory)
{
_optimizeDevDriveDialogViewModelFactory = optimizeDevDriveDialogViewModelFactory;
_dispatcher = Microsoft.UI.Dispatching.DispatcherQueue.GetForCurrentThread();
Copy link
Collaborator

@krschau krschau Apr 1, 2024 •
edited by SohamDas2021
Loading

Choose a reason for hiding this comment

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

If this isn't running on the UI thread, this will return null. Use WindowEx instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Running on UI thread.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It isn't guaranteed that this will run on the UI thread every time. I can forward you the internal thread I started about this. This can return null and will result in a crash.

DevDriveManagerObj = devDriveManager;

_ = this.OnFirstNavigateToAsync();
}

/// <summary>
/// Compares two strings and returns true if they are equal. This method is case insensitive.
/// </summary>
/// <param name="text">First string to use in comparison</param>
/// <param name="text2">Second string to use in comparison</param>
private bool CompareStrings(string text, string text2)
{
return string.Equals(text, text2, StringComparison.OrdinalIgnoreCase);
}

public bool CanEnableSyncButton()
{
return DevDriveLoadingCompleted;
}

[RelayCommand(CanExecute = nameof(CanEnableSyncButton))]
public void SyncDevDrives()
{
GetDevDrives();
}

/// <summary>
/// Make sure we only get the list of DevDrives from the DevDriveManager once when the page is first navigated to.
/// All other times will be through the use of the sync button.
/// </summary>
protected async override Task OnFirstNavigateToAsync()
public void OnFirstNavigateTo()
{
// Do nothing, but we need to override this as the base expects a task to be returned.
await Task.CompletedTask;

GetDevDrives();
GetDevDriveOptimizers();
GetDevDriveOptimizeds();
Expand Down Expand Up @@ -306,6 +275,7 @@ public void UpdateOptimizerListViewModelList()
if (existingPipCacheLocation != null && !CacheInDevDrive(existingPipCacheLocation))
{
var card = new DevDriveOptimizerCardViewModel(
_optimizeDevDriveDialogViewModelFactory,
"Pip cache (Python)",
existingPipCacheLocation,
"D:\\packages" + cacheSubDir /*example location on dev drive to move cache to*/,
Expand Down
2 changes: 1 addition & 1 deletion tools/Customization/DevHome.Customization/ViewModels/Environment/DevDriveCardViewModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public DevDriveCardViewModel(IDevDrive devDrive, IDevDriveManager manager)
_devDriveManager = manager;

DevDriveLabel = devDrive.DriveLabel;
var divider = (ulong)((devDrive.DriveUnitOfMeasure == ByteUnit.TB) ? 1000000000000 : 1000000000);
var divider = (ulong)((devDrive.DriveUnitOfMeasure == ByteUnit.TB) ? 1000_000_000_000 : 1000_000_000);
DevDriveSize = devDrive.DriveSizeInBytes / divider;
DevDriveFreeSize = devDrive.DriveSizeRemainingInBytes / divider;
DevDriveUsedSize = DevDriveSize - DevDriveFreeSize;
Expand Down
28 changes: 0 additions & 28 deletions ...tomization/DevHome.Customization/ViewModels/Environment/DevDriveOptimizedListViewModel.cs
View file
Open in desktop

This file was deleted.

8 changes: 6 additions & 2 deletions ...tomization/DevHome.Customization/ViewModels/Environment/DevDriveOptimizerCardViewModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ namespace DevHome.Customization.ViewModels.Environments;
/// </summary>
public partial class DevDriveOptimizerCardViewModel : ObservableObject
{
public OptimizeDevDriveDialogViewModelFactory OptimizeDevDriveDialogViewModelFactory { get; set; }

public string CacheToBeMoved { get; set; }

public string DevDriveOptimizationSuggestion { get; set; }
Expand All @@ -37,15 +39,17 @@ private async Task OptimizeDevDriveAsync(object sender)
var settingsCard = sender as Button;
if (settingsCard != null)
{
var optimizeDevDriveDialog = new OptimizeDevDriveDialog(CacheToBeMoved, ExistingCacheLocation, EnvironmentVariableToBeSet);
var optimizeDevDriveViewModel = OptimizeDevDriveDialogViewModelFactory(ExistingCacheLocation, EnvironmentVariableToBeSet);
var optimizeDevDriveDialog = new OptimizeDevDriveDialog(optimizeDevDriveViewModel);
optimizeDevDriveDialog.XamlRoot = settingsCard.XamlRoot;
optimizeDevDriveDialog.RequestedTheme = settingsCard.ActualTheme;
Copy link
Collaborator

@krschau krschau Mar 29, 2024 •
edited by SohamDas2021
Loading

Choose a reason for hiding this comment

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

I don't think you want to do this -- it will use the right theme without it, and if the RequestedTheme is Default, then this will cause a bug when the System theme is changed. For example, if RequestedTheme is Default and ActualTheme is Dark, but then you set the system theme to Light, the theme of the dialog will stay Dark #Pending

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without this, I see light theme dialog pop up for a dark themed app.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Does it make a difference if you include the Style on the dialog like I mentioned below? There should be a way we can remove this.

Copy link
Contributor Author

@SohamDas2021 SohamDas2021 Apr 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I have the following:

Style="{StaticResource DefaultContentDialogStyle}" in the dialog.

That is not making any difference- still seeing light theme dialog with dark theme app without setting the RequestedTheme, if that is what you are referring to.

await optimizeDevDriveDialog.ShowAsync();
}
}

public DevDriveOptimizerCardViewModel(string cacheToBeMoved, string existingCacheLocation, string exampleLocationOnDevDrive, string environmentVariableToBeSet)
public DevDriveOptimizerCardViewModel(OptimizeDevDriveDialogViewModelFactory optimizeDevDriveDialogViewModelFactory, string cacheToBeMoved, string existingCacheLocation, string exampleLocationOnDevDrive, string environmentVariableToBeSet)
{
OptimizeDevDriveDialogViewModelFactory = optimizeDevDriveDialogViewModelFactory;
CacheToBeMoved = cacheToBeMoved;
ExistingCacheLocation = existingCacheLocation;
ExampleLocationOnDevDrive = exampleLocationOnDevDrive;
Expand Down
25 changes: 0 additions & 25 deletions ...omization/DevHome.Customization/ViewModels/Environment/DevDriveOptimizersListViewModel.cs
View file
Open in desktop

This file was deleted.

35 changes: 0 additions & 35 deletions tools/Customization/DevHome.Customization/ViewModels/Environment/DevDrivesListViewModel.cs
View file
Open in desktop

This file was deleted.

Loading
Loading