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

Fixes the color of the dropdown arrow in weird themes #13871

Merged
39 commits merged into from
Aug 31, 2022

Conversation

zadjii-msft
Copy link
Member

@zadjii-msft zadjii-msft commented Aug 29, 2022

See also:
https://github.com/microsoft/microsoft-ui-xaml/blob/33732458ede38166f9127675898e4760c82b89bb/dev/SplitButton/SplitButton.xaml#L290-L293

We need to

  • Also set SplitButtonForegroundSecondary cause SplitButton's use that resource separately from the Foreground() property
  • Manually trigger the visual state change, to refresh the brushes. We do something similar in TabBase

This is one of the bullet points in #13725

zadjii-msft and others added 7 commits August 30, 2022 07:07
Co-authored-by: Leonard Hecker <lhecker@microsoft.com>
[11:48 AM] Leonard Hecker
of course not - it's "alpha A inversed"

[11:49 AM] Leonard Hecker
why is that so hard to understand /s lmao
Base automatically changed from dev/migrie/b/13554-new-default-theme to main August 31, 2022 18:32
@DHowett
Copy link
Member

DHowett commented Aug 31, 2022

I am quite scared that this changes fifteen files for what sounds like it shouldn't be too hard!

@DHowett
Copy link
Member

DHowett commented Aug 31, 2022

Oh, I think you need to merge main to comport the history. Otherwise it will continue to show changes that are already on main!

@zadjii-msft
Copy link
Member Author

(diff should be better now)

@zadjii-msft zadjii-msft added this to the Terminal v1.16 milestone Aug 31, 2022
@DHowett
Copy link
Member

DHowett commented Aug 31, 2022

Moved from body of PR (so it didn't clutter up the git commit history with HTML/JSON things):

Tested with these two themes:

json
        {
            "name": "horrible",
            "tab":
            {
                "background": "#00FF00FF",
                "showCloseButton": "never"
            },
            "tabRow":
            {
                "background": "accent",
                "unfocusedBackground": "#008888FF"
            },
            "window":
            {
                "applicationTheme": "light",
                "useMica": false
            }
        },
        {
            "name": "horrible dark",
            "tab":
            {
                "background": "#00FF00FF",
                "showCloseButton": "never"
            },
            "tabRow":
            {
                "background": "accent",
                "unfocusedBackground": "#008888FF"
            },
            "window":
            {
                "applicationTheme": "dark",
                "useMica": false
            }
        },

@@ -3092,12 +3092,24 @@ namespace winrt::TerminalApp::implementation
_newTabButton.Resources().Insert(winrt::box_value(L"SplitButtonBackgroundPointerOver"), backgroundHoverBrush);
_newTabButton.Resources().Insert(winrt::box_value(L"SplitButtonBackgroundPressed"), backgroundPressedBrush);

// Load bearing: The SplitButton uses SplitButtonForegroundSecondary for
// the secondary button, but {TemplateBinding Foreground} for the
Copy link
Member

Choose a reason for hiding this comment

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

yikes can we file that on them

@carlos-zamora
Copy link
Member

@msftbot merge this in 10 minutes

@DHowett DHowett added the AutoMerge Marked for automatic merge by the bot when requirements are met label Aug 31, 2022
@ghost
Copy link

ghost commented Aug 31, 2022

Hello @DHowett!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

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 (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit 36f67d7 into main Aug 31, 2022
@ghost ghost deleted the dev/migrie/b/13725-dropdown-color branch August 31, 2022 21:03
@ghost
Copy link

ghost commented Sep 13, 2022

🎉Windows Terminal Preview v1.16.252 has been released which incorporates this pull request.:tada:

Handy links:

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AutoMerge Marked for automatic merge by the bot when requirements are met
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants