Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ghost-variants should also be offered in normal dimensions #434

Open
SebKranz opened this issue Apr 3, 2024 · 8 comments
Open

ghost-variants should also be offered in normal dimensions #434

SebKranz opened this issue Apr 3, 2024 · 8 comments
Labels
request New feature or request

Comments

@SebKranz
Copy link

SebKranz commented Apr 3, 2024

Ghost buttons are currently optimized for being displayed inline in text, with reduced padding and inverted margins.

However, I often find myself wishing for buttons that appear without borders while still being consistent with other bordered buttons.
From frequently seeing this approach in other interfaces, I believe that this use-case is at least as common as the inline variant.

Screenshot 2024-03-17 at 23 39 29 Screenshot 2024-03-18 at 14 56 36 Screenshot 2024-04-03 at 21 27 21 Screenshot 2024-04-03 at 20 51 43

In all these examples (except maybe 4), I'd say that Radix's approach would have looked worse.

It would be great if we could choose the preferred approach through props:

  • ...either offer both ghost- and inline-ghost-variants
  • ...or decouple the dimensions from the variant and provide a separate prop, inline, that inverts the margins. This would make both the API and implementation more consistent. It would also mean that the inverted margin can be used with bordered variants. Although this may seem weird at first, I can actually think of some cases where you would want a bordered button that grows out of its parent.
@kevinmitch14
Copy link

Yeah, the ghost variants without negative margin would be nice. I have been adding a tiny margin manually to achieve this.

@vladmoroz
Copy link
Contributor

  • ...either offer both ghost- and inline-ghost-variants

This might be interesting and I agree that in some cases it makes sense.

We'll consider a way to do that, for now style={{ margin: 0 }} can be used as a quick workaround to remove the compensation

@vladmoroz vladmoroz added the request New feature or request label Apr 4, 2024
@SebKranz
Copy link
Author

SebKranz commented Apr 4, 2024

for now style={{ margin: 0 }} can be used

Unfortunately, since v3.0 the ghost variant also changes the padding, gap, height and behaviour inside flex-layouts. This makes it difficult to override manually.

Rather than finding all the dimensional edge-cases for rt-variant-ghost, I've found it easier to start from one of the "normal" variants and style manually - although I'm basically fighting the framework in this particular case.

.rt-BaseButton.override-ghost {
  /** reset the default variant */
  box-shadow: none;
  background: none;

  /* copied from https://github.com/radix-ui/themes/blob/c7f02af3fc400dadb293a85ba89e437c2dbba54f/packages/radix-ui-themes/src/components/base-button.css#L321 */
  color: var(--accent-a11);

  &:where(.rt-high-contrast) {
    color: var(--accent-12);
  }
  &:where([data-disabled]) {
    color: var(--gray-a8);
    background-color: var(--gray-a3);
  }

  /* copied from https://github.com/radix-ui/themes/blob/c7f02af3fc400dadb293a85ba89e437c2dbba54f/packages/radix-ui-themes/src/components/base-button.css#L357 */
  @media (hover: hover) {
    &:where(:hover) {
      background-color: var(--accent-a3);
    }
  }
  &:where(:focus-visible) {
    outline: 2px solid var(--focus-8);
    outline-offset: -1px;
  }
  &:where([data-state="open"]) {
    background-color: var(--accent-a3);
  }
  &:where(:active:not([data-state="open"])) {
    background-color: var(--accent-a4);
  }
  &:where([data-disabled]) {
    color: var(--gray-a8);
    background-color: transparent;
  }
}
<IconButton class="override-ghost" />

On a side-note: the prop-system of size, color and variant is awesome and it would be nice if I could somehow introduce custom variants or maybe have an unstyled-variant that allows me to add my own classes while still applying the size and color. This is probably a separate discussion though.

@vladmoroz
Copy link
Contributor

Unfortunately, since v3.0 the ghost variant also changes the padding, gap, height and behaviour inside flex-layouts. This makes it difficult to override manually.

I'm not sure what you mean exactly. Don't think we changed that for 3.0; do you have an example of the issue where margin: 0 doesn't do enough?

@SebKranz
Copy link
Author

SebKranz commented Apr 4, 2024

Don't think we changed that for 3.0

Maybe I'm misremembering, but switching from an older version to 3.0 I found that many of my buttons where misaligned inside flex layouts.

Screenshot 2024-04-04 at 10 47 59 Screenshot 2024-04-04 at 10 49 52

In the source code now, there are many overrides for ghost buttons:

&:where(.rt-variant-ghost) {
box-sizing: content-box;
/* Make sure that the height is not stretched in a Flex/Grid layout */
height: fit-content;
}

@vladmoroz
Copy link
Contributor

vladmoroz commented Apr 4, 2024

In the source code now, there are many overrides for ghost buttons:

height: fit-content; was added 9 months ago, pre 1.0, and it's there to avoid the issue you have on the screenshot with a stretched icon button (which is what any CSS flex layout yields with a default align-items value and children that don't have a fixed height defined)

I can't say more without seeing the exact code that produced that screenshot. Did you give it a go with margin: 0?

On a side-note: the prop-system of size, color and variant is awesome and it would be nice if I could somehow introduce custom variants or maybe have an unstyled-variant that allows me to add my own classes while still applying the size and color. This is probably a separate discussion though.

To be fair, if you need completely different variants beyond simple overrides, I'd recommend to create your own components on top of the CSS variables that Radix Themes provides. This would be the most sturdy way going forward and you can be sure they won't break when the implementation details change in further versions.

@SebKranz
Copy link
Author

SebKranz commented Apr 4, 2024

was added 9 months ago, pre 1.0

You're right, sry. Perhaps the changes was seeing were due to an unrelated change on my end.

Did you give it a go with margin: 0?

I did. The button still is smaller, probably due to these lines:

.rt-IconButton {
&:where(:not(.rt-variant-ghost)) {
height: var(--base-button-height);
width: var(--base-button-height);
}
&:where(.rt-variant-ghost) {
padding: var(--icon-button-ghost-padding);

I'd recommend to create your own components

The reason why I enjoy Radix Themes so much, is that the size, color, radius and variant settings are (mostly) independent from each other. While creating my own component wouldn't be the end of the world (in fact, I am doing that), there are cases where I want to only change the variant, while still benefiting from the variety of other options.

For example, #323 could be solved by this: Keep the TextFields slots, focus-handling, dimensions and color, but bring your own ghost variant.

@SebKranz
Copy link
Author

SebKranz commented Apr 6, 2024

Here's a sandbox of the style that I see most commonly in settings where solid and ghost buttons are mixed. My issue is with how, by default, the hover effect doesn't line up with the dimensions of its siblings.

https://codesandbox.io/p/sandbox/recursing-christian-jq5frh

Screenshot 2024-04-06 at 10 42 22

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
request New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants