Lower the opacity of a BitmapIcon when card is Disabled

[niels9001]

Fixes: #731

PR Type

What kind of change does this PR introduce?
Bugfix

What is the current behavior?

What is the new behavior?

PR Checklist

Please check if your PR fulfills the following requirements:

• Created a feature/dev branch in your fork (vs. submitting directly from a commit on main)
• Based off latest main branch of toolkit
• Tested code with current supported SDKs
• New component
  • Documentation has been added
  • Sample in sample app has been added
  • Analyzers are passing for documentation and samples
  • Icon has been created (if new sample) following the Thumbnail Style Guide and templates
• Tests for the changes have been added (if applicable)
• Header has been added to all new source files
• Contains NO breaking changes

Other information

[AndrewKeepCoding]

Hi @niels9001!

Just a suggestion. To keep it flexible, we could also implement this in the Disabled visual state.
One caveat: headers using symbol icons appears slightly blurry due to the opacity changes...

<x:Double x:Key="SettingsCardHeaderDisabledOpacity">0.6</x:Double>

<Setter Target="PART_HeaderIconPresenterHolder.Opacity" Value="{StaticResource SettingsCardHeaderDisabledOpacity}" />

[niels9001]

@AndrewKeepCoding Fair, but we don't check if it's a bitmapicon in that visual state. And we don't want to change the Foreground AND setting the opacity. And we'd want to keep the Foreground on non-image elements to be consistent with other controls and meeting accessibility requirements (e.g. High Contrast themes).

The alternative would be creating a second disabled state?

[AndrewKeepCoding]

I confirmed that an approach like this works.

<VisualStateGroup x:Name="BitmapIconHeaderStates">
    <VisualState x:Name="EnabledBitmapIconHeader" />
    <VisualState x:Name="DisabledBitmapIconHeader">
        <VisualState.Setters>
            <Setter Target="PART_HeaderIconPresenterHolder.Opacity" Value="{StaticResource SettingsCardHeaderDisabledOpacity}" />
        </VisualState.Setters>
    </VisualState>
</VisualStateGroup>

[niels9001]

<x:Double x:Key="SettingsCardHeaderDisabledOpacity">0.6</x:Double>

Yeah, you're right.. that is a more maintainable option :).

Updated the code, please have a look!

[AndrewKeepCoding]

Nit: we could also clean up a few lines:

- using System.ComponentModel.Design;

- IsEnabledChanged += OnIsEnabledChanged;     
+ IsEnabledChanged += OnIsEnabledChanged;

- //  e.Handled = true;

[AndrewKeepCoding]

My bad I missed this earlier. Since IsEnabled is checked, we should also refresh the visual state here just to be consistent.

private void CheckInitialVisualState()
{
    VisualStateManager.GoToState(this, IsEnabled ? NormalState : DisabledState, true);

    if (GetTemplateChild("ContentAlignmentStates") is VisualStateGroup contentAlignmentStatesGroup)
    {
        contentAlignmentStatesGroup.CurrentStateChanged -= this.ContentAlignmentStates_Changed;
        CheckVerticalSpacingState(contentAlignmentStatesGroup.CurrentState);
        contentAlignmentStatesGroup.CurrentStateChanged += this.ContentAlignmentStates_Changed;
    }

    // The Disabled visual state will only set the right Foreground brush, but for images we need to lower the opacity so it looks disabled.
    if (HeaderIcon is BitmapIcon)
    {
        VisualStateManager.GoToState(this, IsEnabled ? BitmapHeaderIconEnabledState : BitmapHeaderIconDisabledState, true);
    }
}

[michael-hawker]

@niels9001 want to address Andrew's last comment? Then I think we're good (though we need to fix CI as GHA runner updated again breaking us... 😭)

[niels9001]

Ah yes,, thanks @AndrewKeepCoding !

[AndrewKeepCoding]

Looks good to me.🙂

[Arlodotexe]

Tested locally, fix works as intended with no side effects.

[michael-hawker]

Re-running job, intermittent RichSuggestBox failure on WinUI 3. FYI @Arlodotexe #589, as I've been mentioning we should disable these specific tests for now (at least on WinUI 3).