Fix bug where DropDown would lose chivron with reveal

[marcelwgn]

Summary

Fixed a bug where the expan chivron of the DropDownButton would disappear with its reveal style applied.

Motivation

Fixes #70 (comment)

Screenshot

[mdtauk]

Can we fix the hover state for the buttons in Dark Mode turning darker instead of lighter, to match the reveal state? @chigy

[jevansaks]

What was this style/template copied from? ButtonRevealStyle plus some changes?

[marcelwgn]

It is a mix of the ButtonRevealStyle and the DropDownButtonStyle template.

[kaiguo]

I did a diff and looks like it's DropDownButtonStyle + reveal background and border changes + reveal state setters. Looks good to me : )

[chigy]

Can we fix the hover state for the buttons in Dark Mode turning darker instead of lighter, to match the reveal state? @chigy

Dark mode turning darker is current design (the color gets closer to the background). Though not sure why the reveal is not working. It maybe a bug.

[mdtauk]

Can we fix the hover state for the buttons in Dark Mode turning darker instead of lighter, to match the reveal state? @chigy

Dark mode turning darker is current design (the color gets closer to the background). Though not sure why the reveal is not working. It maybe a bug.

@chigy Wouldn't it getting darker, be a pressed state? But lighter as you hover over it? That is what happens in Light Mode. It would also match the Reveal spotlight affect on hover.

[chigy]

@chigy Wouldn't it getting darker, be a pressed state? But lighter as you hover over it? That is what happens in Light Mode. It would also match the Reveal spotlight affect on hover.

@mdtauk , currently, light and dark mode are opposite of each other. On light mode, it goes lighter on hover and darker on press. So it goes darker on hover and lighter on press for dark mode.

[kaiguo]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[kaiguo]

I did a diff and looks like it's DropDownButtonStyle + reveal background and border changes + reveal state setters. Looks good to me : )

[mdtauk]

@chigy Wouldn't it getting darker, be a pressed state? But lighter as you hover over it? That is what happens in Light Mode. It would also match the Reveal spotlight affect on hover.

@mdtauk , currently, light and dark mode are opposite of each other. On light mode, it goes lighter on hover and darker on press. So it goes darker on hover and lighter on press for dark mode.

In Dark Mode the Reveal Hover, and the Default Hover are at odds however.

In my mock ups, I wanted to maintain the Lighten on Hover, and Darken on pressed design language

[kaiguo]

In Dark Mode the Reveal Hover, and the Default Hover are at odds however.

Dark mode accent hover/pressed and default hover/pressed are inconsistent too. They are by design but maybe we should revisit @chigy ?

[marcelwgn]

Dark mode accent hover/pressed and default hover/pressed are inconsistent too. They are by design but maybe we should revisit @chigy ?

I think this whole topic should be discussed over at issue #1126 .

In addition to this, @jevansaks made this comment, where he explained that the reveal effect uses a light brush on both themes as a dark brush was mostly seen as "too gloomy" and "like a storm cloud following me".

[mdtauk]

Dark mode accent hover/pressed and default hover/pressed are inconsistent too. They are by design but maybe we should revisit @chigy ?

I think this whole topic should be discussed over at issue #1126 .

In addition to this, @jevansaks made this comment, where he explained that the reveal effect uses a light brush on both themes as a dark brush was mostly seen as "too gloomy" and "like a storm cloud following me".

@chingucoding Technically that thread is specifically about Reveal. The "problem" is with the Hover State in Dark Mode

[chigy]

In Dark Mode the Reveal Hover, and the Default Hover are at odds however.

Dark mode accent hover/pressed and default hover/pressed are inconsistent too. They are by design but maybe we should revisit @chigy ?

@kaiguo , Can you explain what is inconsistent?

[kaiguo]

@kaiguo Kai Guo FTE , Can you explain what is inconsistent?

Accent button background is lighter on hover and darker on pressed (same as reveal), which is the opposite of the default dark mode button background.

[mdtauk]

In Dark Mode the Reveal Hover, and the Default Hover are at odds however.

Dark mode accent hover/pressed and default hover/pressed are inconsistent too. They are by design but maybe we should revisit @chigy ?

@kaiguo , Can you explain what is inconsistent?

Dark Mode
The Reveal Hover effect Lightens the button with a spotlight and border.
The Reveal Pressed effect lightens the control with an animated light and border.

The Hover state Darkens the button.
The Pressed state Lightens the button

Light Mode
The Reveal Hover effect subtly Lightens the button and has a dark border.
The Reveal Pressed effect subtly lightens the control with an animated light.

The Hover state Lightens the button.
The Pressed state Darkens the button

@chigy You should swap the Hover and Pressed state colours for the button in Dark Mode to match the Light on Hover, Dark on Pressed behaviour of the Accent and Light Mode buttons.

[marcelwgn]

@chingucoding Technically that thread is specifically about Reveal. The "problem" is with the Hover State in Dark Mode

Oh yes I see. Regarding that, I think that the current behavior of the button makes sense as long as the behavior of hover is consistent along the components.

When supporting buttons in light and dark mode you have two possible solutions:
Hover and Pressed have the same effect in both modes (e.g. hover makes button lighter and pressed darker) OR the effect of hover and pressed flip depending on mode. So in light mode hover gets lighter ("color moves towards the white background") and on dark mode the background gets darker ("color moves towards the black background"). I can see why both strategies makes sense but I prefer the latter one.

However the accent style is not consistent and that is a problem. The problem I see is that accent colors may be a bit problematic since they do not perfectly align all the time with the background e.g. dark blue in dark theme feels different to the same dark blue in a light themed app.

In addition to that, keeping the button accessible in all states is a challenge too.

[mdtauk]

Oh yes I see. Regarding that, I think that the current behavior of the button makes sense as long as the behavior of hover is consistent along the components.

When supporting buttons in light and dark mode you have two possible solutions:
Hover and Pressed have the same effect in both modes (e.g. hover makes button lighter and pressed darker) OR the effect of hover and pressed flip depending on mode. So in light mode hover gets lighter ("color moves towards the white background") and on dark mode the background gets darker ("color moves towards the black background"). I can see why both strategies makes sense but I prefer the latter one.

However the accent style is not consistent and that is a problem. The problem I see is that accent colors may be a bit problematic since they do not perfectly align all the time with the background e.g. dark blue in dark theme feels different to the same dark blue in a light themed app.

The Accent Style is the same in Light and Dark modes. In Light Mode the button lightens when you hover, and darkens when you press, The Accent style does the same. It is in the Dark Mode that the notion is flipped.

The reason why this is happening is because the theme colours are a simple inversion, and so you need to swap the Pressed and Hover colour resources around, to make it consistent to the other frameworks and the XAML Light and Accent button styles.

[marcelwgn]

The Accent Style is the same in Light and Dark modes. In Light Mode the button lightens when you hover, and darkens when you press, The Accent style does the same. It is in the Dark Mode that the notion is flipped.
The reason why this is happening is because the theme colours are a simple inversion, and so you need to swap the Pressed and Hover colour resources around, to make it consistent to the other frameworks and the XAML Light and Accent button styles.

I see. If there is no explicit reason for that behavior, I can create a PR if that is something we want to change.

[mdtauk]

@chingucoding Technically that thread is specifically about Reveal. The "problem" is with the Hover State in Dark Mode

Oh yes I see. Regarding that, I think that the current behavior of the button makes sense as long as the behavior of hover is consistent along the components.

When supporting buttons in light and dark mode you have two possible solutions:
Hover and Pressed have the same effect in both modes (e.g. hover makes button lighter and pressed darker) OR the effect of hover and pressed flip depending on mode. So in light mode hover gets lighter ("color moves towards the white background") and on dark mode the background gets darker ("color moves towards the black background"). I can see why both strategies makes sense but I prefer the latter one.

However the accent style is not consistent and that is a problem. The problem I see is that accent colors may be a bit problematic since they do not perfectly align all the time with the background e.g. dark blue in dark theme feels different to the same dark blue in a light themed app.

The Accent Style is the same in Light and Dark modes. In Light Mode the button lightens when you hover, and darkens when you press, The Accent style does the same. It is in the Dark Mode that the notion is flipped.
The reason why this is happening is because the theme colours are a simple inversion, and so you need to swap the Pressed and Hover colour resources around, to make it consistent to the other frameworks and the XAML Light and Accent button styles.

I see. If there is no explicit reason for that behavior, I can create a PR if that is something we want to change.

This is something @chigy would need to put to the Design Team. When the Light Mode's hover state was changed, the Dark Mode was not treated separately, and so we have this inconsistent behaviour, but its not technically a bug

[chigy]

@mdtauk , @chingucoding , @kaiguo there are multiple issues being discussed here so I'm trying to clarify what each of them are and where my thoughts are for each.

1. The GIF of this issue shows reveal not working
2. Hover state of the dark theme button is going darker but folks want it go lighter
3. Hover state of accent color button is going lighter and it is an inconsistency

1. The GIF of this issue shows reveal not working
If this is working, then issue # 2 is less of an issue because the button will still be lighter with reveal light. How come the reveal is not working on that GIF?

2. Hover state of the dark theme button is going darker but folks want it go lighter
This is by design and I believe if reveal is working, it won't be as weird as it looks in GIF.

3. Hover state of accent color button is going lighter and it is an inconsistency
We are using the list control's color behavior for consistency across the system. If we update this, a) we would either have to update the entire system or b) be inconsistent with other controls in the system. Please let me know if you are advocating to a).

[mdtauk]

@chigy I would say that I feel Hover should always brighten or lighten the colour of the control. Pressed would then darken the colour.

Accent has the Light and Dark shades that could be used for this Lightening and Darkening. With the Light mode, you can use a lower transparency for Hover, making it lighten, and then increase the opacity to make it look darker.

This would be reversed for the Dark Theme.

You Increase the opacity to lighten, and decrease to darken.

REST
HOVER
PRESSED

[marcelwgn]

1. The GIF of this issue shows reveal not working
If this is working, then issue # 2 is less of an issue because the button will still be lighter with reveal light. How come the reveal is not working on that GIF?
2. Hover state of the dark theme button is going darker but folks want it go lighter
This is by design and I believe if reveal is working, it won't be as weird as it looks in GIF.

The Reveal effect is the same as for other components. In contrast it looks a bit weird, that is true. However this reveal/hover behaviour of the button has been that way a long time.

So I think if we were to "fix" this, no matter how, we will probably have to rework quite a lot.

[mdtauk]

1. The GIF of this issue shows reveal not working
If this is working, then issue # 2 is less of an issue because the button will still be lighter with reveal light. How come the reveal is not working on that GIF?
2. Hover state of the dark theme button is going darker but folks want it go lighter
This is by design and I believe if reveal is working, it won't be as weird as it looks in GIF.

The Reveal effect is the same as for other components. In contrast it looks a bit weird, that is true. However this reveal/hover behaviour of the button has been that way a long time.

So I think if we were to "fix" this, no matter how, we will probably have to rework quite a lot.

Reveal is being rethought so that will hopefully take all this into account #1126 But the reveal effects should be in alignment with Hover and Pressed States, not totally replace those states.

[chigy]

1. The GIF of this issue shows reveal not working
If this is working, then issue # 2 is less of an issue because the button will still be lighter with reveal light. How come the reveal is not working on that GIF?
2. Hover state of the dark theme button is going darker but folks want it go lighter
This is by design and I believe if reveal is working, it won't be as weird as it looks in GIF.

The Reveal effect is the same as for other components. In contrast it looks a bit weird, that is true. However this reveal/hover behaviour of the button has been that way a long time.

So I think if we were to "fix" this, no matter how, we will probably have to rework quite a lot.

@chingucoding , I am not quite sure what you mean by your response.
What do you mean by "if we fix this?" What are you suggesting we fix here? Other than the reveal not working, there is no issue other than design suggestion from you.

Reveal is being rethought so that will hopefully take all this into account #1126 But the reveal effects should be in alignment with Hover and Pressed States, not totally replace those states.

@mdtauk , Hover and Pressed States are not replacing. They are there and working fine. Reveal is addition. I am not sure what you are suggesting other than your desire for the hover state to go lighter, which I hear and take your suggestion but not something we are looking into changing.

[mdtauk]

Reveal is being rethought so that will hopefully take all this into account #1126 But the reveal effects should be in alignment with Hover and Pressed States, not totally replace those states.

@mdtauk , Hover and Pressed States are not replacing. They are there and working fine. Reveal is addition. I am not sure what you are suggesting other than your desire for the hover state to go lighter, which I hear and take your suggestion but not something we are looking into changing.

@chigy My point about the replacing, is how you could "fix" the fact that Hover goes dark, and Pressed goes like in Dark Mode, which is the opposite to the Accent and Light Mode Buttons. Just swapping the Pressed and Pointer Over colours in the State Triggers, would give the expected result.

Reveal lightens the control with it's hover effect. This matches the behaviour with Light and Accent buttons, but when compared to the Hover State in dark mode, it seems incorrect.

Reveal is undergoing a rethink, but I think the Reveal effects should go on top of the standard Pressed and Pointer Over states. At the moment a Button with a Reveal Style, has totally separate Pointer Over and Pressed designs, to the Default Buttons.

[chigy]

@chigy My point about the replacing, is how you could "fix" the fact that Hover goes dark, and Pressed goes like in Dark Mode, which is the opposite to the Accent and Light Mode Buttons. Just swapping the Pressed and Pointer Over colours in the State Triggers, would give the expected result.

@mdtauk , as mentioned it is not as simple as that because we have a system and changing this behavior would impact other parts of the system and will have cascading effect where we have to touch every single control which we are not ready to do yet. Again, this is the system behavior so it is not broken. It is the current design so nothing to "fix" here.

Reveal lightens the control with it's hover effect. This matches the behaviour with Light and Accent buttons, but when compared to the Hover State in dark mode, it seems incorrect.

Hover state and reveal are independent behavior. In a way, button darkening will make the light shine better. That said, I will take another look at the behavior.

Reveal is undergoing a rethink, but I think the Reveal effects should go on top of the standard Pressed and Pointer Over states.

Yes and it does today.

At the moment a Button with a Reveal Style, has totally separate Pointer Over and Pressed designs, to the Default Buttons.

I do not understand. It is the same as Default Buttons but it has reveal. What is totally separate about? I must not be understanding what you mean...

[mdtauk]

At the moment a Button with a Reveal Style, has totally separate Pointer Over and Pressed designs, to the Default Buttons.

I do not understand. It is the same as Default Buttons but it has reveal. What is totally separate about? I must not be understanding what you mean...

@chigy

If you look at the button colour between the Hover and Pressed with and without Reveal. The Reveal Hover and Pressed styles are using the Rest State button colour, and putting its border and spotlight effect on top of it.

[kaiguo]

The discussions are beyond the purpose of the code changes in this PR. The bug fix itself looks good so I just merged in the code. Discussions can continue 😀

[chigy]

The discussions are beyond the purpose of the code changes in this PR. The bug fix itself looks good so I just merged in the code. Discussions can continue 😀

Actually, I'd like this issue be closed and new specific ones to be opened if the discussion want to continue...

[msft-github-bot]

🎉Microsoft.UI.Xaml v2.2.190917002 has been released which incorporates this pull request.:tada:

Handy links:

• Release Notes
• Official Nuget Release

[msft-github-bot]

🎉Microsoft.UI.Xaml v2.3.191007001-prerelease has been released which incorporates this pull request.:tada:

Handy links:

• Release Notes
• Official Nuget Release