Skip to content

Commit

Permalink
feat(Button): remove default type="button" (#2472)
Browse files Browse the repository at this point in the history
When we set this as a default, it is also added to the component it
composes to.
`a` in a normal thing to change button to, which would lead to 
`<a href="*" type="button">min lenke som ikkje er knapp</a>`

`type="button"` also deviates from the standard type of a `button`.

I think we should remove our default, and keep the standard type.
We will have to make sure to update components such as `Pagination` to
set `type="button"`

I did a workaround for this in `PaginationButton`, as this has the same
problem, but we need to make sure all buttons there won't submit a form.
We could also rely on our consumers to add this here, but that feels
off, since it is in a pagination context, and not a "styled button"
context.

---------

Co-authored-by: Eirik Backer <[email protected]>
  • Loading branch information
Barsnes and eirikbacker authored Sep 20, 2024
1 parent a0fe525 commit dacf6f0
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 3 deletions.
5 changes: 5 additions & 0 deletions .changeset/curvy-oranges-notice.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@digdir/designsystemet-react": patch
---

Button: Remove `type` when `asChild={true}`
5 changes: 5 additions & 0 deletions packages/react/src/components/Button/Button.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@ describe('Button', () => {
await act(async () => await user.click(screen.getByRole('button')));
expect(fn).toHaveBeenCalled();
});

it('should not have type attribute when asChild is true', () => {
render({ asChild: true, children: <a href='#'>Link</a> });
expect(screen.getByRole('link')).not.toHaveAttribute('type');
});
});

const render = (props?: ButtonProps) => renderRtl(<Button {...props} />);
12 changes: 9 additions & 3 deletions packages/react/src/components/Button/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,12 @@ export type ButtonProps = {
* @default false
*/
asChild?: boolean;
} & ButtonHTMLAttributes<HTMLButtonElement>;
/**
* Specify the type of button. Unset when `asChild` is true
* @default 'button'
*/
type?: ButtonHTMLAttributes<HTMLButtonElement>['type'];
} & Omit<ButtonHTMLAttributes<HTMLButtonElement>, 'type'>;

/**
* Button used for interaction
Expand All @@ -50,7 +55,6 @@ export const Button = forwardRef<HTMLButtonElement, ButtonProps>(
icon = false,
loading = false,
size = 'md',
type = 'button',
variant = 'primary',
...rest
},
Expand All @@ -70,7 +74,9 @@ export const Button = forwardRef<HTMLButtonElement, ButtonProps>(
data-size={size}
data-variant={variant}
ref={ref}
type={type}
/* don't set type when we use `asChild` */
{...(asChild ? { asChild: true } : { type: 'button' })}
/* if consumers set type, our default does not set anything, as `rest` contains this */
{...rest}
>
{loading === true ? (
Expand Down

0 comments on commit dacf6f0

Please sign in to comment.