-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Command palette and search box visual tweaks #12913
Conversation
Wow I friggin love it. I'm offline for the weekend already, but lemme ping the team on Monday. |
I, too, love this! Holy wow! |
I somewhat prefer the first design, as it is consistent with the search box and what VSCode does. Giving the controls some space affords us some visual distance between the shadow, tab content and search. How does the list look in tab switcher mode? (by default, Ctrl+Tab will trigger it; keep holding down Ctrl to see the switcher) |
Ah, and how does this look in "commandline" mode? If you backspace the implicit |
I think this is waiting on the fix for the tab switcher top gap, right? I'm happy to review it still! I love it so much. |
I fixed the spacing, but there is still a crash happening sometimes when typing and deleting text inside the command palette. It can be easily replicated by typing >, backspace, >. |
I'll try debugging this more later today. Wild that it doesn't even like give a useful stack trace - it just explodes 🤔 |
Oh no. This won't be simple to diagnose. Lemme eye up the diff a bit closer, see if there's something that stands out. |
The issue happens when MinHeight in _filteredActionsView has values from 5 to 13. There's a connection to the negative padding, but I'm not sure why a key combination causes this. |
I suspect it has nothing to do with the key combo, but more about the showing/hiding/showing of certain UI elements. XAML can be finicky like that sometimes. At this point, we might just need to mess with the padding and general spacing to try and work around this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(apparently I had a review in progress).
Mostly, notes on the MinHeight
thing.
<!-- Remove when implementing WinUI 2.6 --> | ||
<Thickness x:Key="FlyoutContentPadding">12</Thickness> | ||
<!-- Shadow that can be used by any control. --> | ||
<ThemeShadow x:Name="SharedShadow" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is so brilliant. This is what our life would be like if we had people who actually knew how to use XAML
CornerRadius="{ThemeResource OverlayCornerRadius}" | ||
Orientation="Horizontal" | ||
Style="{ThemeResource SearchBoxBackground}"> | ||
Shadow="{StaticResource SharedShadow}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a future note to self - if we ever do productize the WinUI Terminal control, we'll need to edit this, but that is DEFINITELY not a concern for now.
@@ -482,6 +388,8 @@ | |||
|
|||
<ListView x:Name="_filteredActionsView" | |||
Grid.Row="2" | |||
MinHeight="8" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose here's a thought - why do we need this MinHeight
at all? Setting it to 0 seems to mitigate the crash, and I don't know what it's needed for. At least, my eyes aren't sensitive enough to see what this changes...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for the persistence debugging that MinHeight crash - that kind of thing would have drove me crazy!
Hello @zadjii-msft! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
Tweaked command palette and search box to better match the Windows 11 Fluent design: * Now styled like a normal flyout, with acrylic, shadow and border * TextBox now uses default style * Tweaked button and key chord designs * Adjusted spacing Fixes #12968. ![searchlight](https://user-images.githubusercontent.com/101892345/163623805-3b860942-7e2e-401e-8739-3a966cf2b4a9.png) ![palettelight](https://user-images.githubusercontent.com/101892345/163623825-b1f187fa-557b-4921-86bc-4040ffd2952f.png) ![searchdark](https://user-images.githubusercontent.com/101892345/163623820-b3b7b138-d9af-4aae-a637-8e48717ddd2c.png) ![palettedark](https://user-images.githubusercontent.com/101892345/163623830-448354e6-6b37-4dbe-8cb4-e9275aa56322.png) (cherry picked from commit b63102f) Service-Card-Id: 81805346 Service-Version: 1.13
🎉 Handy links: |
🎉 Handy links: |
Tweaked command palette and search box to better match the Windows 11 Fluent design:
Fixes #12968.