Handling display in Links and Buttons

[broccolini]

Hi @shawnbot and @emplums, re our discussion in slack on display and links and buttons, I thought I'd put it in an issue...

Use cases we need to account for:

• making a button, or a button that looks like a link, fill the full width of a container
• switching the width of a button/link from mobile to desktop

Proposed api changes:

• give button width system props
• make sure <Button is="a" /> works properly
• make sure <Button> has display: inline-block; by default so that the width props work (it does in primer css)

This would mean we can do something like:

<Button width={[1, 1, 1/2, 'auto']} />

Let me know what you think and whether I missed any scenarios we need to account for!

[shawnbot]

I'm a bit concerned that this may not cover as many use cases as a responsive display prop would. I don't really have any specific examples to back this up, but could relying on a "fixed" (even if responsive) width rather than intrinsic width from display: block open us up to more weird CSS gotchas?

[emplums]

make sure <Button is="a" /> works properly

Sort of a separate discussion but I am a bit concerned about a11y + the flexibility of our Button and Link components. I'd love to see our React component library nudge users towards using accessible markup & using Buttons / Links in the correct context. From an accessibility standpoint, <button> should be used to interact with content on a page, and <a> should be used to navigate to a new page or new website. To take it a step further, <button>s should be styled like buttons and <a> should be styled like links.

Encouraging users to use <Button is='a'> to get style benefits feels like we'd be steering people in the wrong direction a11y-wise. I'm also wary of even allowing Button's tag to be changed at all - in a perfect world the Button would always render a <button> and the Link would always render a valid <a> with a valid href attribute on it - but I know we need a bit of flexibility in our system! Just food for thought!

[shawnbot]

From an accessibility standpoint, <button> should be used to interact with content on a page, and <a> should be used to navigate to a new page or new website.

I agree, but I think that the use of <button> in forms muddies that distinction a bit, especially in the wacky world of github.com, where many forms do either or both. There are lots of buttons here that would submit and either reload or redirect to a new page when clicked without JS, but which do the AJAX thing and update the current page incrementally with JS.

Taking another step back, I wonder if what we're missing (to inform both this discussion and our component docs) are higher-level guidelines for when a button is really appropriate to use. 🤔

[shawnbot]

Okay, @broccolini and I chatted about this in person and decided move forward with:

• Remove the block prop
• Add a responsive width prop

For the record, I'm skeptical of the usefulness of the width prop, given the rarity of buttons on github.com with custom widths as opposed to display utilities, and I think this:

<Button display={[null, 'block']} />

is easier to reason about and more predictable than:

<Button width={['auto', 1]} />

But I'm down to give this a shot and see how it goes.

[emplums]

There are lots of buttons here that would submit and either reload or redirect to a new page when clicked without JS, but which do the AJAX thing and update the current page incrementally with JS.

True, I believe the rule of thumb for clickable elements in forms is <buttons> though, so it's fine if it navigates to a new page