fix: consistent button height

The height of primary/secondary/link buttons is inconsistent, because some
have borders and some don't. Secondary buttons get smaller when they're
in-progress or disabled, because the border is removed.

This modifies the default y padding to account for borders, and change
the secondary button to always have a border.

I tried making all the buttons have border (to match the current bg where
applicable) but that was more change, changed the styling slightly, and I
couldn't find a good way to match border opacity for the link button.
The multiple ? is ugly, but far shorter than that alternative.

Tab height unchanged (31h).

Before:
- link/primary/disabled secondary: 29.5h
- secondary: 31.5h (29.5 when disabled)
- danger: 33.5h

After: all 29.5h

Fixes #6304.

Signed-off-by: Tim deBoer <git@tdeboer.ca>
This commit is contained in:
Tim deBoer 2024-03-07 14:25:56 -05:00 committed by Florent BENOIT
parent d29c5f3404
commit 46f23dcf88
2 changed files with 21 additions and 2 deletions

View file

@ -30,6 +30,8 @@ test('Check primary button styling', async () => {
const button = screen.getByRole('button');
expect(button).toBeInTheDocument();
expect(button).toHaveClass('bg-purple-600');
expect(button).toHaveClass('border-none');
expect(button).toHaveClass('py-[5px]');
expect(button).toHaveClass('text-[13px]');
expect(button).toHaveClass('text-white');
});
@ -41,6 +43,7 @@ test('Check disabled/in-progress primary button styling', async () => {
const button = screen.getByRole('button');
expect(button).toBeInTheDocument();
expect(button).toHaveClass('bg-charcoal-50');
expect(button).toHaveClass('py-[5px]');
expect(button).toHaveClass('text-[13px]');
});
@ -61,6 +64,8 @@ test('Check secondary button styling', async () => {
const button = screen.getByRole('button');
expect(button).toBeInTheDocument();
expect(button).toHaveClass('border-gray-200');
expect(button).toHaveClass('border-[1px]');
expect(button).toHaveClass('py-[4px]');
expect(button).toHaveClass('text-[13px]');
expect(button).toHaveClass('text-white');
});
@ -72,6 +77,9 @@ test('Check danger button styling', async () => {
const button = screen.getByRole('button');
expect(button).toBeInTheDocument();
expect(button).toHaveClass('border-red-600');
expect(button).toHaveClass('border-2');
expect(button).toHaveClass('px-4');
expect(button).toHaveClass('py-[3px]');
expect(button).toHaveClass('bg-charcoal-700');
expect(button).toHaveClass('text-[13px]');
expect(button).toHaveClass('text-white');
@ -83,6 +91,10 @@ test('Check disabled/in-progress secondary button styling', async () => {
// check for a few elements of the styling
const button = screen.getByRole('button');
expect(button).toBeInTheDocument();
expect(button).toHaveClass('border-charcoal-50');
expect(button).toHaveClass('border-[1px]');
expect(button).toHaveClass('px-4');
expect(button).toHaveClass('py-[4px]');
expect(button).toHaveClass('bg-charcoal-50');
expect(button).toHaveClass('text-[13px]');
});
@ -94,6 +106,8 @@ test('Check link button styling', async () => {
const button = screen.getByRole('button');
expect(button).toBeInTheDocument();
expect(button).toHaveClass('border-none');
expect(button).toHaveClass('px-4');
expect(button).toHaveClass('py-[5px]');
expect(button).toHaveClass('hover:bg-white');
expect(button).toHaveClass('hover:bg-opacity-10');
expect(button).toHaveClass('text-[13px]');
@ -106,6 +120,8 @@ test('Check disabled/in-progress link button styling', async () => {
// check for a few elements of the styling
const button = screen.getByRole('button');
expect(button).toBeInTheDocument();
expect(button).toHaveClass('px-4');
expect(button).toHaveClass('py-[5px]');
expect(button).toHaveClass('text-charcoal-50');
expect(button).toHaveClass('text-[13px]');
});

View file

@ -15,7 +15,8 @@ export let selected: boolean | undefined = undefined;
$: if (selected !== undefined && type !== 'tab') {
console.error('property selected can be used with type=tab only');
}
export let padding: string = type !== 'tab' ? 'px-4 py-[5px]' : 'px-4 pb-1';
export let padding: string =
'px-4 ' + (type === 'tab' ? 'pb-1' : type === 'secondary' ? 'py-[4px]' : type === 'danger' ? 'py-[3px]' : 'py-[5px]');
let iconType: string | undefined = undefined;
@ -30,8 +31,10 @@ onMount(() => {
let classes = '';
$: {
if (disabled || inProgress) {
if (type === 'primary' || type === 'secondary') {
if (type === 'primary') {
classes = 'bg-charcoal-50';
} else if (type === 'secondary') {
classes = 'border-[1px] border-charcoal-50 bg-charcoal-50';
} else if (type === 'danger') {
classes = 'border-2 border-gray-700 bg-charcoal-800';
} else {