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

Jump to bottom

[PowerRename] Show UI info if item cannot be renamed #19934

Merged
merged 11 commits into from
Aug 24, 2022
Merged

[PowerRename] Show UI info if item cannot be renamed #19934

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b9291f3
PowerRename cleanup
stefansjfw Aug 10, 2022
a1571cf
Extract ExplorerItem as a UserControl
stefansjfw Aug 11, 2022
fb4c8e5
Add VisualStateManager
stefansjfw Aug 12, 2022
39c9a6e
UI fixes
niels9001 Aug 12, 2022
6a57af9
Implement error UI logic
stefansjfw Aug 12, 2022
946459e
Merge remote-tracking branch 'origin/main' into stefan/powerrename_wip
stefansjfw Aug 16, 2022
0e3ef87
Update src/modules/powerrename/lib/PowerRenameManager.cpp
stefansjfw Aug 17, 2022
6276b6c
Update src/modules/powerrename/lib/PowerRenameManager.cpp
stefansjfw Aug 17, 2022
1942532
Folder max path is 247
stefansjfw Aug 17, 2022
660a647
Merge branch 'stefan/powerrename_wip' of github.com:microsoft/PowerTo…
stefansjfw Aug 17, 2022
4a55f22
Implement State() properly
stefansjfw Aug 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions src/modules/powerrename/PowerRenameUILib/ExplorerItem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ namespace winrt::PowerRenameUI::implementation
{
m_highlight = visibility;
m_propertyChanged(*this, Microsoft::UI::Xaml::Data::PropertyChangedEventArgs{ L"Highlight" });
VisualStateManager::GoToState(*this, L"Highlight", false);
}
}
}
Expand Down
149 changes: 102 additions & 47 deletions src/modules/powerrename/PowerRenameUILib/ExplorerItem.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,69 +2,124 @@
x:Class="PowerRenameUI.ExplorerItem"
xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
xmlns:local="using:PowerRenameUI"
xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
xmlns:local="using:PowerRenameUI"
xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
mc:Ignorable="d">

<Grid Height="28" Padding="0,0,0,0">
<Grid.ColumnDefinitions>
<ColumnDefinition Width="*" />
<ColumnDefinition Width="4" />
<!-- Some spacing in between -->
<ColumnDefinition Width="*" />
<ColumnDefinition Width="Auto" />
<!-- Error tooltip -->
</Grid.ColumnDefinitions>

<Border
Name="HighlightBorder"
Grid.ColumnSpan="4"
Margin="0,0,0,-1"
HorizontalAlignment="Stretch"
VerticalAlignment="Stretch"
Background="Transparent" />

<Grid
Grid.Column="0"
Margin="10,4,0,4"
HorizontalAlignment="Left">
<Grid.ColumnDefinitions>
<ColumnDefinition MinWidth="{x:Bind Indentation}" />
<ColumnDefinition Width="Auto" />
<ColumnDefinition Width="Auto" />
<ColumnDefinition Width="Auto" />
</Grid.ColumnDefinitions>


<CheckBox
Name="{x:Bind IdStr}"
Grid.Column="1"
Width="28"
Height="28"
MinWidth="0"
AutomationProperties.HelpText="{x:Bind Renamed}"
AutomationProperties.Name="{x:Bind Original}"
Content=""
IsChecked="{x:Bind Checked, Mode=TwoWay}"
IsTabStop="True"
TabIndex="0"
XYFocusKeyboardNavigation="Enabled" />
<Image
Grid.Column="2"
Width="16"
Margin="4,0,0,0"
HorizontalAlignment="Left"
Source="{x:Bind ImagePath}" />
<TextBlock
Grid.Column="3"
Margin="6,0,4,0"
VerticalAlignment="Center"
FontSize="12"
Foreground="{ThemeResource TextFillColorSecondaryBrush}"
Text="{x:Bind Original, Mode=OneWay}"
TextTrimming="CharacterEllipsis" />
</Grid>
<TextBlock
Grid.Column="2"
Margin="0,0,4,0"
VerticalAlignment="Center"
FontSize="12"
FontWeight="SemiBold"
Foreground="{ThemeResource AccentTextFillColorPrimaryBrush}"
Text="{x:Bind Renamed, Mode=OneWay}"
TextTrimming="CharacterEllipsis" />

<Border
x:Name="ErrorIcon"
Grid.Column="3"
Width="12"
Height="12"
CornerRadius="12"
Margin="4,0,8,0"
Background="{ThemeResource SystemFillColorCriticalBrush}"
Visibility="Collapsed">
<TextBlock
HorizontalAlignment="Center"
VerticalAlignment="Center"
FontFamily="{ThemeResource SymbolThemeFontFamily}"
Foreground="White"
FontSize="8"
Text="&#xE10A;" />
Comment on lines +78 to +93
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we use glyph icons with the official brushes and symbols for the error icon in the muxc:InfoBar? This would fix the colors and make the x centered in the circle.

PowerToys/src/settings-ui/Settings.UI/Views/PowerLauncherPage.xaml

Line 284 in 24f4405

<!-- Error badge -->

Copy link
Collaborator

@Jay-o-Way Jay-o-Way Aug 16, 2022 •
edited

Choose a reason for hiding this comment

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

@niels9001 E10A is not allowed. You should know this by now. Please remember our agreement.
And please never hard-code a font color.

Copy link
Collaborator

@htcfreek htcfreek Aug 17, 2022 •
edited

Choose a reason for hiding this comment

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

@stefansjfw
Did you see my review?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, that's Q for @niels9001 :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

@niels9001
Here is a list of all infobar theme resources: https://docs.microsoft.com/en-us/windows/winui/api/microsoft.ui.xaml.controls.infobar?view=winui-2.8#all-theme-resources

Copy link
Collaborator

@Jay-o-Way Jay-o-Way Aug 18, 2022 •
edited

Choose a reason for hiding this comment

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

@stefansjfw and @niels9001 - Using a <border> to achieve a colored background for an icon? Don't take it the wrong way, but I expect better from you guys (!) (try to find a compliment in there...)

Have a look at https://docs.microsoft.com/windows/apps/design/style/segoe-fluent-icons-font#layering-and-mirroring and let's use

<Grid>
    Icon="InfoBarIconBackgroundGlyph" Foreground="InfoBarErrorSeverityIconBackground" // red color
    Icon="InfoBarErrorIconGlyph Foreground="InfoBarErrorSeverityIconForeground"
</Grid>

Copy link
Collaborator

@htcfreek htcfreek Aug 18, 2022 •
edited

Choose a reason for hiding this comment

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

@Jay-o-Way
I agree with and that's why I wrote it.

And btw. we can use the background of the error bar (InfoBarErrorSeverityBackgroundBrush) as background for the table row. Then we are in line with system colors.

Copy link
Contributor

Choose a reason for hiding this comment

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

@niels9001
Here is a list of all infobar theme resources: https://docs.microsoft.com/en-us/windows/winui/api/microsoft.ui.xaml.controls.infobar?view=winui-2.8#all-theme-resources

The InfoBar brushes inherit other brushes... the one we use, so this is fine. See WinUI repo.

Copy link
Collaborator

Choose a reason for hiding this comment

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

For the brushes I am okay with what we have. But can we use the X-Glyph-Resource from info bar. The current implementation you did isn't centered on the red background and this looks not good. (See screenshots in PR description.)

<ToolTipService.ToolTip>
<TextBlock
x:Name="ErrorMessageTxt"
Text="Error message goes here"
TextWrapping="Wrap" />
</ToolTipService.ToolTip>
</Border>



<VisualStateManager.VisualStateGroups>
<VisualStateGroup x:Name="CommonStates">
<VisualState x:Name="Normal"/>
<VisualState x:Name="Normal" />
<VisualState x:Name="Highlight">
<VisualState.Setters>
<Setter Target="HighlightBorder.Opacity" Value="0.1" />
<Setter Target="HighlightBorder.BorderBrush" Value="{ThemeResource AccentTextFillColorPrimaryBrush}" />
<Setter Target="HighlightBorder.Background" Value="{ThemeResource AccentTextFillColorPrimaryBrush}" />
</VisualState.Setters>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Correct me if I'm mistaken, but I don't see a Icon visibility = none here. Isn't that needed to hide it when it's back to normal?

Copy link
Contributor

@niels9001 niels9001 Aug 19, 2022 •
edited

Choose a reason for hiding this comment

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

No, the closing tag indicates that the Normal state falls back to the default states defined in the XAML - in this case, the icon is already collapsed.

EDIT: Oh, going from Error -> Highlight state? @stefansjfw Is that a valid scenario or will it go through the Normal state first at all times? If not we might want to add the collapsing of the Error icon in the Highlight state as @Jay-o-Way mentioned.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Error -> Highlight state works as expeected

</VisualState>
<VisualState x:Name="Error">
<VisualState.Setters>
<Setter Target="highlight_border.Opacity" Value="1"/>
<Setter Target="highlight_border.BorderBrush" Value="{ThemeResource SystemFillColorCritical}"/>
<Setter Target="highlight_border.Background" Value="{ThemeResource SystemFillColorCriticalBackgroundBrush}"/>
<Setter Target="HighlightBorder.Opacity" Value="1" />
<Setter Target="HighlightBorder.BorderBrush" Value="{ThemeResource SystemFillColorCriticalBackgroundBrush}" />
<Setter Target="HighlightBorder.Background" Value="{ThemeResource SystemFillColorCriticalBackgroundBrush}" />
<Setter Target="ErrorIcon.Visibility" Value="Visible" />
</VisualState.Setters>
</VisualState>
</VisualStateGroup>
</VisualStateManager.VisualStateGroups>

<Border Name="highlight_border" HorizontalAlignment="Stretch" Grid.ColumnSpan="2" BorderThickness="0,0,0,1" Margin="0,0,0,-1" BorderBrush="{ThemeResource SystemAccentColor}" VerticalAlignment="Stretch" Background="{ThemeResource AccentTextFillColorPrimaryBrush}" Opacity="0.1" Visibility="{x:Bind Highlight, Mode=OneWay}" />

<Grid Grid.Column="0" HorizontalAlignment="Left" Margin="10,4,0,4">
<Grid.ColumnDefinitions>
<ColumnDefinition MinWidth="{x:Bind Indentation}"/>
<ColumnDefinition Width="Auto"/>
<ColumnDefinition Width="Auto"/>
<ColumnDefinition Width="Auto"/>
</Grid.ColumnDefinitions>


<CheckBox TabIndex="0"
MinWidth="0"
Width="28"
Height="28"
Grid.Column="1"
XYFocusKeyboardNavigation="Enabled"
IsTabStop="True"
Content=""
Name="{x:Bind IdStr}"
AutomationProperties.Name="{x:Bind Original}"
AutomationProperties.HelpText="{x:Bind Renamed}"
IsChecked="{x:Bind Checked, Mode=TwoWay}" />
<Image Width="16"
Grid.Column="2"
Margin="4,0,0,0"
Source="{x:Bind ImagePath}"
HorizontalAlignment="Left" />
<TextBlock Margin="6,0,0,0"
Grid.Column="3"
Text="{x:Bind Original, Mode=OneWay}"
Foreground="{ThemeResource TextFillColorSecondaryBrush}"
VerticalAlignment="Center"
FontSize="12" />
</Grid>
<TextBlock Text="{x:Bind Renamed, Mode=OneWay}"
Grid.Column="1"
FontWeight="Bold"
Foreground="{ThemeResource AccentTextFillColorPrimaryBrush}"
VerticalAlignment="Center"
FontSize="12" />
</Grid>
</UserControl>