Adding @Jdlrobson and @Jdrewniak to decide who to assign this to. Whoever works on this can feel free to ping me or the @talk-to-design-systems-team channel with any questions

Off the top of my head, we should test how the button icon classes work with the checkbox hack, i.e. as dropdown toggles. Not sure if thats already handled in the codex CSS

@LGoto: Please keep an active project tag associated to tasks so a task can be found on the workboard etc - thanks.

@AnneT Overall this looks fantastic! I appreciate how icons inside buttons automatically change color depending on the button's action. The language specific icon is also super fancy! Here’s a list of things I’ve noticed using the components

Button

• One discrepancy between the codex and mediawiki.ui button classes is the increased size of icon only buttons on small viewports. This behavior is currently relied on in both Vector and Minerva.
• Related to the issue above, Vector currently makes use of the poorly named .mw-ui-icon-flush-left and .mw-ui-icon-flush-right classes in order to visually align quiet buttons with other parts of our UI. For example in this image, notice how the ellipsis icon inside the button is aligned with the edge of the container:
  

Considering how icon button padding could change depending on the viewport, I think it would be ideal for Codex to provide button 'flush' mixins, or at the very least variables for us to be able to implement those helpers ourselves.

• Are there plans to support button styles on non button elements? I know that this is a big subject of debate especially with links styled as buttons, but I know that this is needed for Vector's dropdown menus, which use a checkbox hack implementation where the <label> element is styled as a button
• Another thing to note about the checkbox hack, core button styles have extra CSS related to button states. Vector also relies on these. The reasoning for these styles is a bit confusing so I can provide more context if needed
• Is it possible for the cdx-button--icon-only class to hide text similar to the deprecated .mw-ui-icon-element class? This would be useful for buttons that can become icon only buttons. i.e. Minerva has buttons that become icon only buttons on lower resolutions (.mw-ui-icon-with-label-desktop). Vector doesn't have anything like this yet, but its possible in the future (i.e. a setting for editors who hate icon only buttons). Functionality like this could be easy to achieve if cdx-button--icon-only hid text, and button mixins were provided
• Small design nitpick: The design spec for the normal button shows 12px of horizontal spacing, but the Codex buttons are implemented with horizontal padding of 12px and a 1px border, effectively giving the button a spacing of 13px. This is inconsistent with the icon only button, which implements the spec of 32 by 32px inclusive of the border. This is an existing issue in the mediawiki.ui button classes as well, but I point it out because it resulted in lots of 1px discrepancies when using the 'flush' classes. Also, should the cdx-button--icon-only apply padding: 0 5px instead of padding: 0 4px? This would give a horizontal spacing of 6px.

Icon

• Are there plans to support icon classes in addition to mixins? Or is the reason for that to avoid including all icon classes in the stylesheet?
• Small design nitpick: I noticed that the icon doesn't look exactly vertically aligned when used inside a button, this is particularly noticable in the icon only button demo. Based off my very quick testing it seems like the vertical-align: middle style could be the culprit.

Would you be ok with hand-authoring the markup for CSS-only Buttons, at least for now while we have no system for generating the markup?

This should be fairly easy to do with our mustache templates. Only concern would be the Icon component which would need classes for each icon that use the mixin and appropriate icon token. Is it intended for us to manually create classes for each icon (i.e. .vector-css-icon--ellipsis)? Would appreciate any advice on how this should be handled.

Thoughts on how we could package these components (the CSS for the button and the icon Less mixin) for your use in a way that's ideal for you?

FYI, I moved this task to code review for another web team member to review my spike in case I missed anything. After that I think this is done, or waiting for the design system team to respond and close this task.

@bwang thanks so much for your thorough review and reply!

In T329776#8731334, @bwang wrote:

• One discrepancy between the codex and mediawiki.ui button classes is the increased size of icon only buttons on small viewports. This behavior is currently relied on in both Vector and Minerva.

Good to know; I've created T333392 for us to evaluate how to support this behavior in Codex

• Related to the issue above, Vector currently makes use of the poorly named .mw-ui-icon-flush-left and .mw-ui-icon-flush-right classes in order to visually align quiet buttons with other parts of our UI. For example in this image, notice how the ellipsis icon inside the button is aligned with the edge of the container:
  

Considering how icon button padding could change depending on the viewport, I think it would be ideal for Codex to provide button 'flush' mixins, or at the very least variables for us to be able to implement those helpers ourselves.

Makes sense! I've added T333393 to explore potential implementation paths.

• Are there plans to support button styles on non button elements? I know that this is a big subject of debate especially with links styled as buttons, but I know that this is needed for Vector's dropdown menus, which use a checkbox hack implementation where the <label> element is styled as a button
• Another thing to note about the checkbox hack, core button styles have extra CSS related to button states. Vector also relies on these. The reasoning for these styles is a bit confusing so I can provide more context if needed

We haven't discussed this much recently, and I know there is debate on the design side specific to links as you mentioned (see T284273). In theory, you could use the CSS-only button classes for any element, although this wouldn't completely work for the checkbox hack unless we added selectors like the ones from mw.ui you linked to for the button states. I think that's a reasonable thing for us to do, and maybe eventually we could even add a ButtonMenu component whose CSS-only version (and Vue-SSR-ed version) is the checkbox hack. I've added T333394 to cover this.

• Is it possible for the cdx-button--icon-only class to hide text similar to the deprecated .mw-ui-icon-element class? This would be useful for buttons that can become icon only buttons. i.e. Minerva has buttons that become icon only buttons on lower resolutions (.mw-ui-icon-with-label-desktop). Vector doesn't have anything like this yet, but its possible in the future (i.e. a setting for editors who hate icon only buttons). Functionality like this could be easy to achieve if cdx-button--icon-only hid text, and button mixins were provided

I'll have to put some more thought into this one...this would work for the CSS-only version, but it's not quite the same with the Vue button, since icons are included as svgs and not as background images. So, hiding text requires wrapping the text inside the button in another element. To get around this issue, we're encouraging people to add an aria-label to icon-only buttons rather than including actual button text.

In terms of dynamically changing from an icon-only button to a regular button depending on viewport size, this might be a case where it's better to handle this at the application level, e.g. something like this:

<template>
    <cdx-button class="my-button">
        <cdx-icon :icon="cdxIconWhatever">
        <span class="my-button__text">Button text</span>
    </cdx-button>
</template>

<style lang="less">
// Get design tokens and mixins from Codex
@import 'mediawiki.skin.variables.less';

.my-button {
    @media screen and ( max-width: @max-width-breakpoint-mobile ) {
        // Style as an icon-only button (this mixin doesn't actually exist yet).
        .cdx-mixin-button-icon-only();

        &__text {
            // Visually hide text (this mixin exists but is not public).
            .cdx-mixin-screen-reader-text();
        }
    }
}
</style>

I'm interested to hear @Volker_E's thoughts here too.

• Small design nitpick: The design spec for the normal button shows 12px of horizontal spacing, but the Codex buttons are implemented with horizontal padding of 12px and a 1px border, effectively giving the button a spacing of 13px. This is inconsistent with the icon only button, which implements the spec of 32 by 32px inclusive of the border. This is an existing issue in the mediawiki.ui button classes as well, but I point it out because it resulted in lots of 1px discrepancies when using the 'flush' classes. Also, should the cdx-button--icon-only apply padding: 0 5px instead of padding: 0 4px? This would give a horizontal spacing of 6px.

For regular buttons, this shouldn't be an issue since we set box-sizing: border-box on buttons—I just tested a button with no text in it, and it was 32px wide as expected. Let me know if I'm missing something here..

For icon buttons, I'm not sure why we're setting a horizontal padding of 4px - the CdxButton has a min-width of 32px, so the padding is kind of misleading here. It seems like it should be 6px. I'll follow up with @Volker_E

• Are there plans to support icon classes in addition to mixins? Or is the reason for that to avoid including all icon classes in the stylesheet?

We don't currently have plans to support icon classes, both for the sake of avoiding a giant file that applies all the possible icon background images, and to offer more flexibility, since with the icon mixin you can apply any color and a few pre-set sizes. That said, if we do find that there's an appetite for icon classes, we could consider supporting a limited set of icons this way.

• Small design nitpick: I noticed that the icon doesn't look exactly vertically aligned when used inside a button, this is particularly noticable in the icon only button demo. Based off my very quick testing it seems like the vertical-align: middle style could be the culprit.

Sadly this is a known and complex issue based on using the same size icon (20px) for multiple font sizes (14 and 16px), see T326900. It keeps me up at night :lolsob: I think we landed on vertical-align: middle for now because it's the least bad at a 14px base font size, which is how most production use of the CdxButton is currently being viewed. If you use the icon component's vertical-align value (text-bottom), at 14px you get this:

If you have further ideas on this one though, please do leave a comment in that task!

Would you be ok with hand-authoring the markup for CSS-only Buttons, at least for now while we have no system for generating the markup?

This should be fairly easy to do with our mustache templates. Only concern would be the Icon component which would need classes for each icon that use the mixin and appropriate icon token. Is it intended for us to manually create classes for each icon (i.e. .vector-css-icon--ellipsis)? Would appreciate any advice on how this should be handled.

The intention is for the icon mixin to be applied for each individual instance of an icon in a feature, so you can set not only the icon image but the color and size as well. That said, you could create a utility class of sorts in your codebase as you've described, if you know you're going to be using the exact same icon specs in multiple places. Does that seem workable, or will this be burdensome in a codebase like Vector or Minerva (or core?)

So, hiding text requires wrapping the text inside the button in another element. To get around this issue, we're encouraging people to add an aria-label to icon-only buttons rather than including actual button text.

Yeah that's true, this use case needs an additional wrapper for the text. Currently Vector wraps our button text for minor spacing adjustments

"link": {
  "text-wrapper": {
    "tag": "span"
  }
},

That said, with the way the Vue button component works with slots, we can easily just add that wrapper on our side if we need it.

In terms of dynamically changing from an icon-only button to a regular button depending on viewport size, this might be a case where it's better to handle this at the application level

That sounds good to me, I agree it makes sense to handle this on an application level if we have button mixins

For regular buttons, this shouldn't be an issue since we set box-sizing: border-box on buttons—I just tested a button with no text in it, and it was 32px wide as expected. Let me know if I'm missing something here.

Ah sorry I wasn't super clear, the issue is the expected padding, not the min-width. As you mentioned with the icon button, the spec says the spacing is 6px so you'd expect the padding to the 6px. But if you actually set the padding to 6px you would end up with a button that's 34px wide because of the 1px border. Which is why I'm guessing the correct padding is 5px.

Similarly for the normal button, the spec says the spacing is 12px, so considering the 1px border the padding should be 11px. Its nitpicky, but this comes up with the flush classes. With the current styles, the flush classes would apply a margin of -13px for the normal buttons and -6px for the icon buttons, which is a bit unintuitive.

We don't currently have plans to support icon classes, both for the sake of avoiding a giant file that applies all the possible icon background images, and to offer more flexibility, since with the icon mixin you can apply any color and a few pre-set sizes.

That makes sense to me, thanks for the context

@AnneT - sending this over to you for signoff. Let us know if there's anything else needed from our side!