[Buttons]: Squircle looks bad on our plain/plain-faint buttons with a background hover

[fallaciousreasoning]

@AlanBreck in our recent updates to the button shapes, I don't think we should've tweaked the background shape of the plain/plain-faint buttons as it makes a bunch of our existing buttons look weird:

Hovering over the close button for Brave News:

These ones are fine, because they're filled:

Close button on the customize dialog:

[fallaciousreasoning]

Also, not sure if it's related but this button looks like it has too much padding to me:

Here's the same button in Beta:

cc @aguscruiz

[aguscruiz]

Those close buttons would be good candidates for the padding and hoverless buttons, I think. Those variants that are icon ondly and don't have a background change. What do you think?

As for the padding on that "Join cube" button, yeah, I can see it. The padding on these normal buttons have been the subject of long debates until we reached this final version. But it may be time to adjust a bit

[aguscruiz]

That close button though, yeah I think we should use the "icon-only" variant to solve it. But that's just me

[AlanBreck]

@fallaciousreasoning, from what I'm seeing, everything described above is to spec, according to the Figma doc.

I was going to say the same thing that @aguscruiz just said. All of the instances above where the hover background looks off seem like they ought to be icon-only buttons (or fab).

[fallaciousreasoning]

I think we need to be pretty careful about changing the buttons going forward - they're used in a lot of places, and it kinda defeats the point of having a design system if we need to go and audit all the places things are used when we update Nala.

Like, I think my problem is that these were fine, we updated Nala, and now they look weird - it makes bumping Nala in brave-core pretty scary, because I don't know what I'm going to break

[fallaciousreasoning]

Could we maybe get rid of the additional padding on the plain and plain-faint variants? It definitely makes things look weird, and as I understand it, that's what we'd go to if we had a button in a block of text.

This is a plain-faint button. It used to align nicely with the edge but no longer does.

Hover is fine, I think, but maybe a bit too loud? Previously, this was used basically as a link button but it doesn't really work for that now.

[AlanBreck]

I think another problem that this highlights is the need to deprecate fab and introduce a more logical name for icon only buttons. At least for the examples you mentioned, I think many wouldn't have caused an issue if they were icon only buttons, but fab just doesn't jump out as "oh yeah, that's what I should use in this case" when looking at Storybook.

[fallaciousreasoning]

One other thing I noticed is that the filled and outline buttons now have slightly different heights (I think because of the border). This makes a common use case (changing between the two states, depending on what the button is going to do) more complicated because the height changes

[AlanBreck]

One other thing I noticed is that the filled and outline buttons now have slightly different heights (I think because of the border). This makes a common use case (changing between the two states, depending on what the button is going to do) more complicated because the height changes

Yep. I noticed the same thing. I believe it'll be a simple fix of adding a default border of solid 1px transparent to all buttons. Specifically changing this line:

leo/src/components/button/button.svelte

Line 145 in 7dc6a04

border: solid var(--border-width, 0px) var(--border-color, transparent);

- border: solid var(--border-width, 0px) var(--border-color, transparent);
+ border: solid var(--border-width, 1px) var(--border-color, transparent);