Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Button: Improve disabled-related prop descriptions #57864

Merged
merged 1 commit into from
Jan 16, 2024
Merged

Conversation

mirka
Copy link
Member

@mirka mirka commented Jan 15, 2024

What?

In Button, adds some clarity in the prop descriptions for __experimentalIsFocusable and disabled.

Why?

Some of the wording was a bit confusing.

@mirka mirka added [Type] Developer Documentation Documentation for developers [Package] Components /packages/components labels Jan 15, 2024
@mirka mirka requested a review from a team January 15, 2024 23:20
@mirka mirka self-assigned this Jan 15, 2024
@mirka mirka requested a review from ajitbohra as a code owner January 15, 2024 23:20
Copy link
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Thanks!

*/
disabled?: boolean;
};

type AnchorProps = {
/**
* Whether the button is disabled.
* If `true`, this will force a `button` element to be rendered.
*
* If `true`, this will force a `button` element to be rendered, even when an `href` is given.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this even be true, given that the type is false? Feels like this entire line is redundant here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. In theory, yes it seems a bit wrong. In reality, the props types/descriptions for the <a> case and the <button> case are union-ed into a polymorphic type, and in most scenarios where this description can be shown to a developer I think this extra clarification is always relevant. For example, in the Storybook props table, or in IntelliSense (including parts of a codebase that are not type checked).

Basically even in an instance like <Button href="foo" disabled={ false }>, it's still useful to know that flipping the disabled to true would force the Button into a <button> element.

@mirka mirka merged commit f478dfe into trunk Jan 16, 2024
60 of 61 checks passed
@mirka mirka deleted the button-type-docs branch January 16, 2024 23:37
@github-actions github-actions bot added this to the Gutenberg 17.6 milestone Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components [Type] Developer Documentation Documentation for developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants