[EuiToggle] Deprecate in favor of documenting aria-pressed

[myasonik]

I'd like to deprecate EuiToggle largely because it's an inaccessible pattern that requires more work to get right but having it bundled in the library gives it a air of authority.

I think it also aligns with EUI's ethos of not providing things that don't have use-cases. So, right now, this is a public API that we have upkeep expectations around even though no one has any reason to use so we can't even say this is the best solution we can offer.

After scouring GitHub, the only usage of it that I could find is EUI itself within EuiButtonToggle so it shouldn't have much (if any) impact on any consumers of EUI.

Given it's lack of usage, I don't think a better time to remove it will come up.

CC @snide @cchaos — thoughts?

[cchaos]

@myasonik Before we just remove the utility altogether, is there no way to fix it?

[myasonik]

I mean, I think the fix is EuiButtonToggle... I'm not quite sure what purpose this is supposed to serve that EuiButtonToggle doesn't.

[cchaos]

When they don't want a <button>... haha

Or really <EuiButton> to be exact. There's no way to make an EuiButtonEmpty a toggle without EuiToggle

[myasonik]

🤔 OK — that's something!

So I wonder if we can solve this a different way? Just thinking out loud, haven't dug into the code to check the practicalities of any of this:

• Maybe we create a prop for EuiButtonToggle where you can pass empty into? You can already pass in isIconOnly which combines EuiButton and EuiButtonIcon. We could either have isEmptyButton=true or buttonType='icon'|'empty'.
• Or, maybe we flip it, and we create a prop for EuiButton, EuiButtonIcon and EuiButtonEmpty where you can pass type='toggle' into?

[myasonik]

That kind of makes me wonder though, why does EuiButtonToggle have "icon" and "regular" combined into one component but then there are separate components for Button and ButtonIcon?

Should there actually be one Button to rule them all?

[cchaos]

@myasonik and I talked this on through. Essentially, to make a <button> behave like a toggle it simply needs the aria-pressed property. All HTML buttons accept this as a property and therefore, all EUI button (EuiButton, EuiButtonEmpty, EuiButtonIcon) all support this toggling option inherently.

Therefore, it makes both the EuiButtonToggle and EuiToggle components moot. Our path forward would be to slowly phase these components out, adding them to our deprecation cycle. In turn, we will be swapping the EuiButtonToggle doc example for an example of how to properly turn any EUI button into a toggle via aria-pressed.

@myasonik will work on the language for this docs section.

[myasonik]

@cchaos So I started writing this up tonight and I hit a decision point:

To deprecate this I wanted to update EUI to not use it so I was updating EuiButtonGroup which uses EuiButtonToggle and it exposed a thing:

EuiToggleButton is the only way we have to make a filled EuiButtonIcon it seems. Should I add a filled state to EuiButtonIcon or should I add an "iconOnly" prop to EuiButton?

I vote I add the prop to EuiButton because then I think we could actually deprecate EuiButtonIcon as well further simplifying our API (imo).

Thoughts?

[cchaos]

Lol, I love the enthusiasm, but we should not continue to consider deprecating core components as a path forward. One of the main reasons we use separate components instead of more+ props is to make it more explicit/semantic to what the consumer is trying to create. Also, once you start compounding all the different permutations of combinations of props on a single component you start running into failures and issues like "if this prop is this, then it also needs that" and it gets very complicated.

Likely the solution is just that we have a custom classname passed to EuiButtonIcon inside of EuiButtonGroup that will set the background colors based on aria-pressed status. I believe that's mostly how it's working right now just likely that we need to check for a different prop.

If you can get the button group working with aria-pressed but using EuiButton and EuiButtonIcon, I can help with the styling.

[myasonik]

I'm good with that being the plan forward for deprecating EuiToggle! Let's at least worry about one thing at a time.

But longer term, I actually find the bifurcation of EuiButton, EuiButtonEmpty, EuiButtonIcon more cumbersome than a single EuiButton that could take a prop and change its rendering.

@chandlerprall @snide Maybe y'all have some thoughts on the longer term strategy?

[snide]

It is much easier to control the stylistic look and patterns of separate components, then try to make one component that can do everything, but be very adaptive with nested prop cascade. I'm willing to have that conversation, but it would require a beverage and a lack of priorities. 🍻

[chandlerprall]

I don't particularly care about the end direction taken. However, I'd like to see an icon button concept that accounts for all of an icon button's use cases: icon with maybe a fill and/or border. I don't think EuiButtonGroup should be applying classes/styles to style the icon button in a specific way, instead of managing via props; doing so would break down some lines that a component should be self-contained, and makes it harder to re-use the concept elsewhere.

[cchaos]

I'd recommend just taking baby steps right now. Let's fix what this issue is truly trying to solve, then we get into creating more states for other components. But currently adding more visual styles to EuiButtonIcon requires some thinking on design implications around the necessity/usages.