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

Enable the user to unpin/pin an external tool on the bar #2972

Merged
merged 3 commits into from
May 22, 2024
Merged

Enable the user to unpin/pin an external tool on the bar #2972

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3c629bd
Enable the user to unpin/pin an external tool on the bar
andreww-msft May 20, 2024
a8399fb
Updates per feedback
andreww-msft May 20, 2024
3adcb43
Made FilteredExternalTools a ReadOnlyObservableCollection
andreww-msft May 21, 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
9 changes: 3 additions & 6 deletions tools/PI/DevHome.PI/BarWindow.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,13 +93,10 @@
<!-- External tools -->
<StackPanel Orientation="Horizontal">
<StackPanel.ContextFlyout>
<!-- TODO Re-enable this when we have Click handler implementation.-->
<MenuFlyout x:Name="ToolContextMenu">
<MenuFlyoutItem
x:Uid="UnPinMenuItem"
x:Name="UnPinMenuItem"
Click="UnPinMenuItem_Click"
IsEnabled="False">
x:Name="PinUnpinMenuItem"
Click="PinUnpinMenuItem_Click">
<MenuFlyoutItem.Icon>
<FontIcon Glyph="&#xE77A;"/>
</MenuFlyoutItem.Icon>
Expand Down Expand Up @@ -131,7 +128,7 @@

<ItemsRepeater
x:Name="ExternalToolsRepeater"
ItemsSource="{x:Bind helpers:ExternalToolsHelper.Instance.ExternalTools, Mode=OneWay}"
ItemsSource="{x:Bind helpers:ExternalToolsHelper.Instance.FilteredExternalTools, Mode=OneWay}"
Layout="{StaticResource ExternalToolsHorizontalLayout}">
<DataTemplate x:DataType="helpers:ExternalTool">
<Button
Expand Down
66 changes: 38 additions & 28 deletions tools/PI/DevHome.PI/BarWindow.xaml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
using Microsoft.UI.Windowing;
using Microsoft.UI.Xaml;
using Microsoft.UI.Xaml.Controls;
using Microsoft.UI.Xaml.Data;
using Microsoft.UI.Xaml.Input;
using Windows.UI.WindowManagement;
using Windows.Win32;
Expand All @@ -36,6 +37,8 @@ public partial class BarWindow : WindowEx, INotifyPropertyChanged
private readonly Settings settings = Settings.Default;
private readonly string errorTitleText = CommonHelper.GetLocalizedString("ToolLaunchErrorTitle");
private readonly string errorMessageText = CommonHelper.GetLocalizedString("ToolLaunchErrorMessage");
private readonly string pinMenuItemText = CommonHelper.GetLocalizedString("PinMenuItemText");
private readonly string unpinMenuItemText = CommonHelper.GetLocalizedString("UnpinMenuItemText");
private readonly BarWindowViewModel viewModel = new();

// Constants that control window sizes
Expand Down Expand Up @@ -138,12 +141,10 @@ private bool IsMaximized
private readonly WINEVENTPROC winPositionEventDelegate;
private readonly WINEVENTPROC winFocusEventDelegate;

private Button? selectedExternalToolButton;
private MenuFlyoutItem? selectedExternalToolMenuItem;

private HWINEVENTHOOK positionEventHook;
private HWINEVENTHOOK focusEventHook;

private ExternalTool? selectedExternalTool;
private INotifyCollectionChanged? externalTools;

internal static HWND ThisHwnd { get; private set; }
Expand Down Expand Up @@ -234,14 +235,15 @@ private void InitializeExternalTools()
{
ExternalToolsHelper.Instance.Init();

foreach (var item in ExternalToolsHelper.Instance.ExternalTools)
ExternalToolsMenu.Items.Clear();
foreach (var item in ExternalToolsHelper.Instance.AllExternalTools)
{
CreateMenuItemFromTool(item);
}

// We have to cast to INotifyCollectionChanged explicitly because the CollectionChanged
// event in ReadOnlyObservableCollection is protected.
externalTools = ExternalToolsHelper.Instance.ExternalTools;
externalTools = ExternalToolsHelper.Instance.AllExternalTools;
externalTools.CollectionChanged += ExternalTools_CollectionChanged;
}

Expand Down Expand Up @@ -281,7 +283,7 @@ private void CreateMenuItemFromTool(ExternalTool item)
Icon = item.MenuIcon,
};
menuItem.Click += ExternalToolMenuItem_Click;
menuItem.RightTapped += MenuItem_RightTapped;
menuItem.RightTapped += ExternalToolMenuItem_RightTapped;
ExternalToolsMenu.Items.Add(menuItem);

// You can't databind to MenuFlyoutItem, and the ExternalTool icon image is generated asynchronously,
Expand All @@ -308,12 +310,21 @@ private void ManageExternalToolsButton_Click(object sender, RoutedEventArgs e)
settingsTool.Show();
}

private void MenuItem_RightTapped(object sender, RightTappedRoutedEventArgs e)
private void ExternalToolMenuItem_RightTapped(object sender, RightTappedRoutedEventArgs e)
{
var menuItem = sender as MenuFlyoutItem;
if (menuItem is not null)
{
selectedExternalToolMenuItem = menuItem;
selectedExternalTool = (ExternalTool)menuItem.Tag;
if (selectedExternalTool.IsPinned)
{
PinUnpinMenuItem.Text = unpinMenuItemText;
}
else
{
PinUnpinMenuItem.Text = pinMenuItemText;
}

ToolContextMenu.ShowAt(menuItem, e.GetPosition(menuItem));
}
}
Expand Down Expand Up @@ -357,36 +368,35 @@ private void InvokeTool(ExternalTool tool, int? id, HWND hWnd)

private void ExternalToolButton_PointerPressed(object sender, PointerRoutedEventArgs e)
{
selectedExternalToolButton = (Button)sender;
if (sender is Button clickedButton)
{
selectedExternalTool = (ExternalTool)clickedButton.Tag;
if (selectedExternalTool.IsPinned)
{
PinUnpinMenuItem.Text = unpinMenuItemText;
}
else
{
PinUnpinMenuItem.Text = pinMenuItemText;
}
}
}

private void UnPinMenuItem_Click(object sender, RoutedEventArgs e)
private void PinUnpinMenuItem_Click(object sender, RoutedEventArgs e)
{
// TODO Unpin the tool from the bar, but don't remove it from the collection,
// so it stays registered, and it stays in the ExternalTools menu.
if (selectedExternalToolButton is not null)
{
}
else if (selectedExternalToolMenuItem is not null)
// Pin or unpin the tool on the bar.
if (selectedExternalTool is not null)
{
selectedExternalTool.IsPinned = !selectedExternalTool.IsPinned;
}
}

private void UnregisterMenuItem_Click(object sender, RoutedEventArgs e)
{
if (selectedExternalToolButton is not null)
if (selectedExternalTool is not null)
{
if (selectedExternalToolButton.Tag is ExternalTool tool)
{
ExternalToolsHelper.Instance.RemoveExternalTool(tool);
}
}
else if (selectedExternalToolMenuItem is not null)
{
if (selectedExternalToolMenuItem.Tag is ExternalTool tool)
{
ExternalToolsHelper.Instance.RemoveExternalTool(tool);
}
ExternalToolsHelper.Instance.RemoveExternalTool(selectedExternalTool);
selectedExternalTool = null;
}
}

Expand Down
20 changes: 17 additions & 3 deletions tools/PI/DevHome.PI/Helpers/ExternalTool.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,19 @@ public string OtherArgs
{
get; private set;
}


private bool isPinned;

public bool IsPinned
{
get => isPinned;
set
{
isPinned = value;
OnPropertyChanged(nameof(IsPinned));
}
}
krschau marked this conversation as resolved.
Show resolved Hide resolved

[JsonIgnore]
private SoftwareBitmapSource? _toolIcon;
Copy link
Contributor

Choose a reason for hiding this comment

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

_toolIcon

nit: can we use camelCase?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes that would be my preferred style, but I'm seeing the leading underscore in other parts of DevHome. We should make an explicit style choice across the board, and do a code cleanup operation at some point.


Expand Down Expand Up @@ -81,13 +93,15 @@ public ExternalTool(
string executable,
ExternalToolArgType argtype,
string argprefix = "",
string otherArgs = "")
string otherArgs = "",
bool isPinned = false)
{
Name = name;
Executable = executable;
ArgType = argtype;
ArgPrefix = argprefix;
OtherArgs = otherArgs;
OtherArgs = otherArgs;
IsPinned = isPinned;

ID = Guid.NewGuid().ToString();

Expand Down
Loading
Loading