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

Ironsides - Tooltips for the bar items #3107

Merged
merged 2 commits into from
Jun 5, 2024
Merged

Ironsides - Tooltips for the bar items #3107

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bccd0e0
Tooltips for the bar items
timkur Jun 4, 2024
36b2998
PR updates
timkur Jun 4, 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
6 changes: 3 additions & 3 deletions tools/PI/DevHome.PI/BarWindowHorizontal.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
<ColumnDefinition x:Name="RightPaddingColumn" Width="0"/>
</Grid.ColumnDefinitions>

<TextBlock x:Name="TitleTextBlock" Text="DevHome PI" FontSize="10" Margin="10,0,0,0" HorizontalAlignment="Left" Grid.Column="1" VerticalAlignment="Center"/>
<TextBlock x:Uid="Title" FontSize="10" Margin="10,0,0,0" HorizontalAlignment="Left" Grid.Column="1" VerticalAlignment="Center"/>
<StackPanel x:Name="ChromeButtonPanel" Orientation="Horizontal" VerticalAlignment="Center" HorizontalAlignment="Right" Margin="0, 0, 10, 0" Background="Transparent" Grid.Column="1">
<Button
x:Uid="SnapButton"
Expand Down Expand Up @@ -65,15 +65,15 @@
</controls:ProcessSelectionButton>

<!-- Per App controls -->
<Image x:Name="AppIcon" HorizontalAlignment="Center" Source="{x:Bind _viewModel.ApplicationIcon, Mode=OneWay}" Margin="5" Height="20" Width="20" Visibility="{x:Bind _viewModel.AppBarVisibility, Mode=OneWay}"/>
<Image x:Name="AppIcon" HorizontalAlignment="Center" Source="{x:Bind _viewModel.ApplicationIcon, Mode=OneWay}" ToolTipService.ToolTip="{x:Bind _viewModel.ApplicationName, Mode=OneWay}" Margin="5" Height="20" Width="20" Visibility="{x:Bind _viewModel.AppBarVisibility, Mode=OneWay}"/>
<TextBlock x:Uid="AppPID" Text="{x:Bind _viewModel.ApplicationPid, Mode=OneWay}" FontFamily="Segoe UI" FontSize="10" Margin="5" VerticalAlignment="Center" Visibility="{x:Bind _viewModel.AppBarVisibility, Mode=OneWay}"/>
<TextBlock x:Uid="AppCPUUsage" Text="{x:Bind _viewModel.AppCpuUsage, Mode=OneWay}" FontFamily="Segoe UI" FontSize="10" Margin="5" VerticalAlignment="Center" Visibility="{x:Bind _viewModel.AppBarVisibility, Mode=OneWay}"/>

<!-- Per System controls -->
<Button x:Uid="SwitchLayoutButton" x:Name="SwitchLayoutButton" HorizontalAlignment="Center" Click="{x:Bind _viewModel.SwitchLayoutCommand}">
<TextBlock Text="&#xe8b4;"/>
</Button>
<Button x:Name="LargeContentButton" Click="{x:Bind _viewModel.ShowBigWindowCommand}" HorizontalAlignment="Center">
<Button x:Name="LargeContentButton" x:Uid="LargeContentButton" Click="{x:Bind _viewModel.ShowBigWindowCommand}" HorizontalAlignment="Center">
Copy link
Contributor

@dhoehna dhoehna Jun 4, 2024 •
edited by timkur
Loading

Choose a reason for hiding this comment

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

NIT: a more descriptive Id. "LargeContentButton" is ambiguous. #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.

Good point.

<TextBlock Text="&#xe8a1;"/>
</Button>

Expand Down
8 changes: 5 additions & 3 deletions tools/PI/DevHome.PI/BarWindowVertical.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,16 @@
<StackPanel x:Name="ChromeButtonPanelVertical" Orientation="Vertical" HorizontalAlignment="Center" Margin="0, 10" Grid.Row="0">
<TextBlock x:Name="GripTextBlock" Text="&#xe76f;&#xe76f;" FontSize="10" Margin="0" HorizontalAlignment="Center" FontFamily="Segoe Fluent Icons" PointerPressed="Window_PointerPressed" PointerMoved="Window_PointerMoved" PointerReleased="Window_PointerReleased"/>
<Button
x:Uid="SnapButton"
Margin="0,20, 0, 0"
Style="{StaticResource ChromeButton}"
HorizontalAlignment="Center"
IsEnabled="{x:Bind _viewModel.IsSnappingEnabled, Mode=OneWay}"
Click="{x:Bind _viewModel.PerformSnapCommand}" >
<TextBlock Text="{x:Bind _viewModel.CurrentSnapButtonText, Mode=OneWay}"/>
<TextBlock Text="{x:Bind _viewModel.CurrentSnapButtonText, Mode=OneWay}"/>
</Button>
<Button
x:Uid="VerticalCloseButton"
Style="{StaticResource ChromeButton}"
Click="CloseButton_Click"
HorizontalAlignment="Center"
Expand Down Expand Up @@ -68,15 +70,15 @@
</controls:ProcessSelectionButton>

<!-- Per App controls -->
<Image x:Name="AppIcon" HorizontalAlignment="Center" Source="{x:Bind _viewModel.ApplicationIcon, Mode=OneWay}" Margin="5" Height="20" Width="20" Visibility="{x:Bind _viewModel.AppBarVisibility, Mode=OneWay}"/>
<Image x:Name="AppIcon" HorizontalAlignment="Center" Source="{x:Bind _viewModel.ApplicationIcon, Mode=OneWay}" ToolTipService.ToolTip="{x:Bind _viewModel.ApplicationName, Mode=OneWay}" Margin="5" Height="20" Width="20" Visibility="{x:Bind _viewModel.AppBarVisibility, Mode=OneWay}"/>
Copy link
Contributor

@dhoehna dhoehna Jun 4, 2024 •
edited by timkur
Loading

Choose a reason for hiding this comment

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

Please put each property on their own line. #WontFix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just my 2cents... obvious consistency is important, but my vote would be to have all the properties on one line. When I'm trying to see how all of my controls fit into a layout on the page, having a line per control is more readable (to me) than having a single control take up a quarter of the page. I think this borders on personal preference though...

Copy link
Collaborator

Choose a reason for hiding this comment

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

Just to add another two cents🙂... generally, we put everything on one line when it's short enough to fit and split when it's not. Personally, I find that easier to read in an IDE, but I also think splitting long lines makes it easier to review on GitHub. I'm fine with the wontfix, just wanted to add a little context.

<TextBlock x:Uid="AppPID" Text="{x:Bind _viewModel.ApplicationPid, Mode=OneWay}" FontFamily="Segoe UI" FontSize="10" Margin="5" HorizontalAlignment="Center" Visibility="{x:Bind _viewModel.AppBarVisibility, Mode=OneWay}"/>
<TextBlock x:Uid="AppCPUUsage" Text="{x:Bind _viewModel.AppCpuUsage, Mode=OneWay}" FontFamily="Segoe UI" FontSize="10" Margin="5" HorizontalAlignment="Center" Visibility="{x:Bind _viewModel.AppBarVisibility, Mode=OneWay}"/>

<!-- Per System controls -->
<Button x:Uid="SwitchLayoutButton" x:Name="SwitchLayoutButton" Click="{x:Bind _viewModel.SwitchLayoutCommand}" HorizontalAlignment="Center">
<TextBlock Text="&#xe8b4;"/>
</Button>
<Button x:Name="LargeContentButton" Click="{x:Bind _viewModel.ShowBigWindowCommand}" HorizontalAlignment="Center">
<Button x:Name="LargeContentButton" x:Uid="LargeContentButton" Click="{x:Bind _viewModel.ShowBigWindowCommand}" HorizontalAlignment="Center">
<TextBlock Text="&#xe8a1;"/>
</Button>

Expand Down
24 changes: 16 additions & 8 deletions tools/PI/DevHome.PI/Strings/en-us/Resources.resw
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,14 +129,6 @@
<value>Restore</value>
<comment>Tooltip for the restore button in the BarWindow</comment>
</data>
<data name="CloseAllMenuItem.Text" xml:space="preserve">
<value>End PI service</value>
<comment>The label for the close all menu item flyout. This will cause the program to terminate.</comment>
</data>
<data name="CloseAllMenuItem.ToolTipService.ToolTip" xml:space="preserve">
<value>Close the PI window and the service</value>
<comment>The tooltip for the close all menu item flyout. This will cause the program to terminate.</comment>
</data>
<data name="PinMenuItemText" xml:space="preserve">
<value>Pin</value>
<comment>A label for the menuitem to pin a tool on the bar.</comment>
Expand Down Expand Up @@ -849,4 +841,20 @@
<value>Edit or delete existing tools</value>
<comment>Header for the setting item to edit or delete tools from the product.</comment>
</data>
<data name="ProcessChooserButton.ToolTipService.ToolTip" xml:space="preserve">
<value>Select a process to investigate</value>
<comment>Describes using a button in PI's UX to investigate another process running on the system</comment>
</data>
<data name="SnapButton.ToolTipService.ToolTip" xml:space="preserve">
<value>Attach the diagnostic bar to the target application</value>
<comment>Describes using a button in PI's UX</comment>
</data>
<data name="Title.Text" xml:space="preserve">
<value>Dev Home PI</value>
<comment>The product name</comment>
</data>
<data name="VerticalCloseButton.ToolTipService.ToolTip" xml:space="preserve">
<value>Close</value>
<comment>Describes a button that will close PI's window</comment>
</data>
</root>
5 changes: 5 additions & 0 deletions tools/PI/DevHome.PI/ViewModels/BarWindowViewModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ public partial class BarWindowViewModel : ObservableObject
[ObservableProperty]
private Visibility _appBarVisibility = Visibility.Visible;

[ObservableProperty]
private string? _applicationName;
Copy link
Contributor

@dhoehna dhoehna Jun 4, 2024 •
edited by timkur
Loading

Choose a reason for hiding this comment

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

NIT: remove ? and assign string.empty in the constructor. #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.

Ok. Seems to fit the pattern better for the rest of the file. I'm guessing a tooltip of null is different than a tooltip of "", but in theory the user won't have access to the tooltip of an app if we're not attached to an app.


[ObservableProperty]
private int _applicationPid;

Expand Down Expand Up @@ -85,6 +88,7 @@ public BarWindowViewModel()

if (process != null)
{
ApplicationName = process.ProcessName;
ApplicationPid = process.Id;
ApplicationIcon = TargetAppData.Instance.Icon;
}
Expand Down Expand Up @@ -186,6 +190,7 @@ private void TargetApp_PropertyChanged(object? sender, PropertyChangedEventArgs
if (process is not null)
{
ApplicationPid = process.Id;
ApplicationName = process.ProcessName;
}
});
}
Expand Down
Loading