ui icon indicating copy to clipboard operation
ui copied to clipboard

fix(button): fix prop types for button

Open Birkbjo opened this issue 1 year ago • 2 comments

Description

I don't think we should extend HTML-elements directly. In some cases like tabIndex we're actually using a conflicting type, which causes type-errors in our types. tabIndex should really be a number (that's what the html type is), but that would be breaking due to our prop-types etc.

So instead of ButtonProps extending the HTMLButtonElement, I'm using type intersection with the ComponentPropsWithoutRef<'button'> without the keys for ButtonProps, to avoid conflicts. I also split this in two types types because eg. export interface ButtonProps extends Omit<React.ComponentPropsWithoutRef<'button'>, keyof ButtonProps>) wouldn't work because of circular references.

See https://github.com/typescript-cheatsheets/react/blob/main/docs/advanced/patterns_by_usecase.md#wrappingmirroring, and expand "Why not ComponentProps or IntrinsicElements or [Element]HTMLAttributes or HTMLProps or HTMLAttributes?" for why we're using ComponentPropsWithoutRef, as opposed to the other prop-types.

Birkbjo avatar Apr 15 '24 15:04 Birkbjo

🚀 Deployed on https://pr-1473--dhis2-ui.netlify.app

dhis2-bot avatar Apr 15 '24 15:04 dhis2-bot

Passing run #3401 ↗︎

0 584 0 0 Flakiness 0

Details:

Merge pull request #1473 from dhis2/fix/button-prop-types
Project: ui Commit: 46f302c931
Status: Passed Duration: 08:53 💡
Started: May 27, 2024 1:27 PM Ended: May 27, 2024 1:36 PM

Review all test suite changes for PR #1473 ↗︎

cypress[bot] avatar Apr 15 '24 16:04 cypress[bot]

:tada: This PR is included in version 9.4.9 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

dhis2-bot avatar May 27 '24 13:05 dhis2-bot