rowellx68 / nhs-components

A React implementation of the NHS.UK Frontend library. Compatible with Next.js
https://rowellx68.github.io/nhs-components/
MIT License
5 stars 2 forks source link

ButtonProps type is missing `onClick` prop #89

Open graham-a3 opened 1 week ago

graham-a3 commented 1 week ago

image

Current work around is to explicitly add the onClick prop to the wrapper.

rowellx68 commented 1 week ago

We can define an onClick property in ButtonProps but it will remove the typing that the polymorphic component brings.

From these:

// when we set `as` to `a`
onClick?: React.MouseEventHandler<HTMLAnchorElement> | undefined

// without setting the `as` property
onClick?: React.MouseEventHandler<HTMLButtonElement> | undefined

to this

onClick?: React.MouseEventHandler<HTMLElement> | undefined
rowellx68 commented 1 week ago

The other option is to augment the type like you have. Also, as per mantine's docs regarding wrapping polymorphic components it suggests to do it like so:

import React, { forwardRef, MouseEventHandler } from 'react';
import { Button, ButtonProps } from 'nhsuk-frontend-react';

type WrappedButtonProps = ButtonProps & {
  isLoading?: boolean;
  onClick?: MouseEventHandler<HTMLButtonElement>;
};

export const WrappedButton = forwardRef<HTMLButtonElement, WrappedButtonProps>(
  ({ isLoading, children, ...props }, ref) => (
    <Button {...props} disabled={isLoading} ref={ref}>
      {isLoading ? 'Loading...' : children}
    </Button>
  ),
);