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.

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

[Registry Preview] Fixes in for the command bar in XAML #25266

Merged
merged 4 commits into from Apr 11, 2023

Conversation

randyrants
Copy link
Collaborator

@randyrants randyrants commented Apr 7, 2023

Summary of the Pull Request

XAML fixes for the transparency on the command bar and unified radial corners.
Changes shortcuts for Save and Save As buttons.
Moved to FontIcon for icon which should work on systems that don't have the specific Symbol font. Also picks up support for the original icon that was required for Edit 😄

PR Checklist

Detailed Description of the Pull Request / Additional comments

Changed the background property for the command bar from transparent to one of the theme brushes, so that the overflow from the ... button had an opaque background.

Also added radial corner settings to the TextBox and the DataGrid, to match the rest of the UX, changed the keyboard accelerators for Save and Save As to be more "normal" and updated the method used to show an icon.

Validation Steps Performed

Ran the window several times in different sizes as well as in light and dark theme modes.

Change the toolbar to have a color, rather than transparent
Added radial corners to the textbox and gridview.
Changing short  cuts for Save and Save As
@randyrants randyrants changed the title [Registry Preview] Fix for the transparent background on the command bar [Registry Preview] Fixes in for the command bar in XAML Apr 7, 2023
A wise engineer once said "save the file, fool, before committing it to the repro."
@Jay-o-Way
Copy link
Collaborator

Why is IsTabStop="False" explicitly declared? This makes it impossible to use the keyboard :(

@Jay-o-Way
Copy link
Collaborator

Jay-o-Way commented Apr 8, 2023

  • Line 218: StackPanel {Image + Text} <TextBlock Padding="5,0,0,0" -> you could also use <StackPanel Spacing=""
  • multiple occasions: Please do not use FontSize="14". It can lead to unsharp text or icons. Best to stick to either defaults, or use static resources if needed.
  • Line 303: CornerRadius is still hardcoded

Copy link
Collaborator

@Jay-o-Way Jay-o-Way left a comment

Choose a reason for hiding this comment

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

couple of things please :)

@@ -209,6 +226,7 @@
Grid.RowSpan="2"
Grid.Column="2"
Copy link
Collaborator

@Jay-o-Way Jay-o-Way Apr 8, 2023

Choose a reason for hiding this comment

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

RowSpan="2" should be oblsolete now, am I right?

Comment on lines 57 to 58
BorderBrush="Transparent"
BorderThickness="0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why declare these in the first place? :)

@Jay-o-Way Jay-o-Way self-requested a review April 8, 2023 16:43
Copy link
Collaborator

@Jay-o-Way Jay-o-Way left a comment

Choose a reason for hiding this comment

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

x

@jaimecbernardo jaimecbernardo added the Hot Fix Items we will product an out-of-band release for label Apr 11, 2023
Copy link
Collaborator

@SeraphimaZykova SeraphimaZykova left a comment

Choose a reason for hiding this comment

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

Referenced problems for that PR seems to be fixed, but changes @Jay-o-Way proposed also make sense. So I believe we can merge it but open an issue with those comments, so they don't get lost later.

@niels9001
Copy link
Contributor

@SeraphimaZykova Going to merge this in to avoid conflicts, will address the mentioned comments with a PR to fix: #25406

@niels9001 niels9001 merged commit 02349f1 into microsoft:main Apr 11, 2023
7 checks passed
@randyrants randyrants deleted the xamlFixes branch April 15, 2023 01:56
BLM16 pushed a commit to BLM16/PowerToys that referenced this pull request Jun 22, 2023
)

* Update MainWindow.xaml

Change the toolbar to have a color, rather than transparent
Added radial corners to the textbox and gridview.

* Update MainWindow.xaml

Changing short  cuts for Save and Save As

* Update MainWindow.xaml

A wise engineer once said "save the file, fool, before committing it to the repro."

* Moved to FontIcons
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Hot Fix Items we will product an out-of-band release for
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants