feat(button): add option to render as tag

[AdnoC]

Required for https://github.com/dequelabs/walnut/pull/3320

[github-actions]

Preview branch generated at https://tag-as-button.d1gko6en628vir.amplifyapp.com

[straker]

Should add an example of the Tag use on the button component page

[straker]

LGTM

add docs

[thuey]

We should provide a visual indicator to sighted users on hover that the tag button is interactive. Right now, there is no indication that the tag button is interactive.

If the hover state is not already defined, it would be good to work with @awpearlm on it.

added style

[thuey]

Rather than introduce a new class name, can we just leverage button.Tag like we are doing for button.Link?

[scurker]

I could go either way on this, although we should be consistent on both. I think we can reutilize the styles for .Tag but it would be nice to specifically target our buttons with .Button--variant classname. If we want to use .Button--tag, I think .Button--link should exist as well.

[thuey]

@scurker My personal preference would be .Button.Tag, since it still targets our buttons without introducing another classname that needs to be included any time you want to do className="Button Tag". However, like you said, I wanted to be consistent with what we had w/o introducing a breaking change (button.Link -> .Button.Link) 🤷

[thuey]

Thoughts?

[scurker]

@thuey The problem is css specificity, and is counter to our guidelines:

For modifiers, such as variants or states, these values would be delimited by two dashes --

.Calendar--variant-large {
  font-size: var(--text-size-large);
}

.Calendar--variant-small {
  font-size: var(--text-size-small);
}

[scurker]

I'm suggesting something completely different that doesn't introduce a breaking change:

• Use both button.Tag and button.Button--tag. .Tag inherits the styles from .Tag and .Button--tag keeps our conventions in place (and implementing the same pattern for .Link).

This allows someone to target either all tags (with .Tag) or just button tag variants (with .Button--tag) while keeping specificity simple. Any styles that are specific to just .Button--tag should exist on the variant class, not .Button.Tag.

[thuey]

Gotcha. That works for me 👍

[thuey]

Actually, that still doesn't answer the question of what happens to the current button.Link styles:

cauldron/packages/styles/button.css

Lines 44 to 46 in 62c9172

	button.Link {
	cursor: pointer;
	}

cauldron/packages/styles/button.css

Lines 231 to 237 in 62c9172

	.cauldron--theme-dark button.Link {
	color: var(--accent-light);
	}
	.cauldron--theme-dark button.Link:hover {
	color: var(--white);
	}

If we change those to button.Button--link, that is a breaking change. If we leave them as-is, that breaks consistency.

[scurker]

Since this is an unrelated change to the proposed work, I don't have an issue with this being a breaking change as part of a separate ticket so here's my suggestion:

• Create a separate issue to change the .Link class to .Button--link
• Make the change here for .Button--tag as a feat

[thuey]

Here's the ticket: #681

Sorry for the confusion @AdnoC. We will want to do Button--tag after all

[scurker]

I could go either way on this, although we should be consistent on both. I think we can reutilize the styles for .Tag but it would be nice to specifically target our buttons with .Button--variant classname. If we want to use .Button--tag, I think .Button--link should exist as well.

[scurker]

Why position: relative?

[AdnoC]

That is needed for hover to work. Since the :before element has position: absolute.

Changed to use button.Tag selector

[github-actions]

Preview branch generated at https://tag-as-button.d1gko6en628vir.amplifyapp.com