facebook / facebook/astryx

Button hard-codes a fixed height per size, silently overriding theme paddingBlock overrides (theming a taller button requires non-obvious height: auto)

Open
#3,379 1 comment 0 reactions 0 assignees View on GitHub
bug component
Dominant language
TypeScript
Stars
13k
Forks
1.1k
Avg merge
1d 15h
Merged PRs (30d)
690

Description

## Summary

Increasing a Button's height via a theme `paddingBlock` override **silently does nothing**. The base Button hard-codes a fixed `height` per size, and under `box-sizing: border-box` that fixed height absorbs any padding you add in the theme. There's no error and no visual change — the theme override just appears ignored. The (non-obvious) fix is to also set `height: auto` in the theme override.

This is a theming trap: "make this button taller" should be a one-line `paddingBlock` change in the theme, and instead it fails invisibly until you go read the component's compiled StyleX.

**Versions:** `@astryxdesign/core@0.1.2`, `@astryxdesign/cli@0.1.2`

## Cause

`dist/Button/Button.d.ts`:
```ts
declare const sizeStyles: Readonly<{
readonly sm: { readonly height: /* 28px */ };
readonly md: { readonly height: /* 32px */ };
readonly lg: { readonly height: /* 36px */ };
}>;
export type ButtonSize = keyof typeof sizeStyles;
```

So each size applies a fixed `height`. A theme override like:
```ts
button: {
'size:lg': { paddingBlock: '20px', paddingInline: '30px' },
}
```
generates `.astryx-button.lg { padding-block: 20px }` — which wins for padding, but the button stays **36px tall** because the fixed `height` is still in effect and border-box means the padding just eats into those 36px. Result: no visible change.

## Workaround

Release the height in the same override:
```ts
button: {
'size:lg': { height: 'auto', paddingBlock: '20px', paddingInline: '30px' },
}
```
Now padding drives the height and the button grows as expected.

## Suggestions

- Prefer `min-height` over a fixed `height` for size styles, or let padding drive height, so theme padding overrides compose naturally.
- Failing that, document prominently (in the `theme` docs / component override examples) that resizing a Button via padding requires `height: 'auto'`.
- Consider a dev warning when a component-override sets `paddingBlock`/`paddingInline` on a size that also carries a fixed `height`.

## Related

- Sizes aren't theme-extensible the way variants are: `ButtonSize = keyof typeof sizeStyles` is a closed object, whereas variants use the augmentable `ButtonVariantMap` (#3371). So you can't add a "jumbo" size via the theme — only tune existing ones, or swizzle. The fixed-height issue above compounds this: the sizes you *do* have don't respond to the most natural theming lever.
- Part of the broader theme-authoring DX in #3375 and adoption traps in #3374.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.