fix(button/button-group): styles updates

[SisIvanova]

Closes #1107

[AnjiManova]

Fluent
I know that the focus in this PR is not Fluent, but it was mentioned that some changes have also been made there, so I checked it.

Button Group
https://github.com/IgniteUI/igniteui-angular/assets/127843837/662996be-a147-4ebf-b2c7-1c0edfd43af4
as we discussed, the focused border shouldn't stay after interaction with the button. Applicable for Light and Dark mode.

Bootstrap
Same thing as in Fluent.

Material
Same thing as in Fluent and Bootstrap.

Same issue in Buttons for all themes.

Everything else looks good.

[imincheva]

Button Group

• Material: Min-width of the button group button should be 42px;
• Fluent: Min-width of the button group button should be 42px;
• Bootstrap: Min-width of the button group button should be 42px;

[rkaraivanov]

@SisIvanova

Does this PR require the addition of new properties to the button components? Can this be accomplished using only :focus and :focus-visible, or is there something I'm overlooking?

[SisIvanova]

@SisIvanova

Does this PR require the addition of new properties to the button components? Can this be accomplished using only :focus and :focus-visible, or is there something I'm overlooking?

We need this new property to remove the focus styles after clicking on the button which cannot be achieved only using :focus and :focus-visible. The same approach is used in the switch/checkbox/radio components.

[rkaraivanov]

@SisIvanova
Does this PR require the addition of new properties to the button components? Can this be accomplished using only :focus and :focus-visible, or is there something I'm overlooking?

We need this new property to remove the focus styles after clicking on the button which cannot be achieved only using :focus and :focus-visible. The same approach is used in the switch/checkbox/radio components.

I still believe it's feasible to attain the same functionality using only CSS pseudo-selectors, unless my understanding of the specification is incorrect. The introduction of :focus-visible, which allows different styling based on "browser heuristics" — typically the distinction between pointer and keyboard-driven focus — supports this notion.

In my experimentation with the toggle button, it appears to exhibit identical behavior after removing the extra JavaScript properties, the focused DOM element part, and its references in the Fluent style file, along with implementing the subsequent modifications to the Fluent styles:

:host {
    [part='toggle'] {
        min-height: $fluent-flat-btn-size;
        border-width: $group-item-border-thickness;
    }

    [part='toggle']:focus {
        background: var-get($theme, 'item-background');
    }

    [part='toggle']:focus-visible {
        background: var-get($theme, 'item-background');

        &::after {
            content: '';
            position: absolute;
            inset-block-start: $outline-btn-indent;
            inset-inline-start: $outline-btn-indent;
            pointer-events: none;
            width: calc(100% - (#{$outline-btn-indent} * 2));
            height: calc(100% - (#{$outline-btn-indent} * 2));
            box-shadow: 0 0 0 rem(1px) var-get($theme, 'item-focused-border-color');
        }
    }
}

As I'm not well-versed in CSS, I'm uncertain about the ease, scalability, and applicability of this approach to our component library, so I will limit my comments to the JavaScript aspects. We should consider if it's possible to simplify the implementation of these behaviors without relying on additional JavaScript code.

@simeonoff

[simeonoff]

@SisIvanova
Does this PR require the addition of new properties to the button components? Can this be accomplished using only :focus and :focus-visible, or is there something I'm overlooking? @SisIvanova

We need this new property to remove the focus styles after clicking on the button which cannot be achieved only using :focus and :focus-visible. The same approach is used in the switch/checkbox/radio components.

I still believe it's feasible to attain the same functionality using only CSS pseudo-selectors, unless my understanding of the specification is incorrect. The introduction of :focus-visible, which allows different styling based on "browser heuristics" — typically the distinction between pointer and keyboard-driven focus — supports this notion.

In my experimentation with the toggle button, it appears to exhibit identical behavior after removing the extra JavaScript properties, the focused DOM element part, and its references in the Fluent style file, along with implementing the subsequent modifications to the Fluent styles:

:host {
    [part='toggle'] {
        min-height: $fluent-flat-btn-size;
        border-width: $group-item-border-thickness;
    }

    [part='toggle']:focus {
        background: var-get($theme, 'item-background');
    }

    [part='toggle']:focus-visible {
        background: var-get($theme, 'item-background');

        &::after {
            content: '';
            position: absolute;
            inset-block-start: $outline-btn-indent;
            inset-inline-start: $outline-btn-indent;
            pointer-events: none;
            width: calc(100% - (#{$outline-btn-indent} * 2));
            height: calc(100% - (#{$outline-btn-indent} * 2));
            box-shadow: 0 0 0 rem(1px) var-get($theme, 'item-focused-border-color');
        }
    }
}

As I'm not well-versed in CSS, I'm uncertain about the ease, scalability, and applicability of this approach to our component library, so I will limit my comments to the JavaScript aspects. We should consider if it's possible to simplify the implementation of these behaviors without relying on additional JavaScript code.

@simeonoff

Yeah, you are correct in your assessment and we are aware of the focus-visible pseudo selector and we're using it extensively wherever we can. I believe there were some issues that couldn't be overcome by simply using it in the case of the button, but Silvia can expand on that.

[rkaraivanov]

Avoid exposing additional public APIs for internal states when possible. This approach should accomplish the desired behavior without introducing a new public property.

[rkaraivanov]

Suggested change

	@state()
	public focused = false;
	@state()
	protected focused = false;

[rkaraivanov]

Suggested change

	button.focused = false;
	Object.assign(button, { focused: false });

[SisIvanova]

@rkaraivanov We have different focus styles depending on if the button is focused via keyboard or mouse click. If you focus a button using keyboard and then click on it with the mouse, the "keyboard focus styles" should be removed. Here is an example with the suggested CSS code:

Screen.Recording.2024-04-30.at.1.01.12.AM.mov

As you can see, the focus-visible box-shadow remains visible. The same thing goes for the button component.

[AnjiManova]

Button Group

Material & Fluent:

• right and left paddings are 16 for all sizes, they should be 8, 12 and 16 (small, medium and large) ✅ implemented

Bootstrap:

• right and left paddings are 8, 10 and 12 should be, 8, 12 and 16 (small, medium and large) ✅ implemented

Indigo:

• for medium the paddings are correct @andiesm813, could you please provide/validate the small and large size paddings

[AnjiManova]

From our side, everything looks fine, and all comments for Material, Fluent and Bootstrap are implemented.

[andiesm813]

Button Group

Material & Fluent:

• right and left paddings are 16 for all sizes, they should be 8, 12 and 16 (small, medium and large) ✅ implemented

Bootstrap:

• right and left paddings are 8, 10 and 12 should be, 8, 12 and 16 (small, medium and large) ✅ implemented

Indigo:

• for medium the paddings are correct @andiesm813, could you please provide/validate the small and large size paddings

INDIGO THEME

I will just list the correct left and right paddings here:

• Small: 6px
• Medium: 8px ✅
• Large: 10px

Like @AnjiManova said, the medium size is correctly implemented. The other 2 need to be fixed.

[andiesm813]

The comments for Indigo Theme were implemented. So looks good to me! ✅✅✅