Update docs around the disabled prop and aria-disabled attribute#2946
Update docs around the disabled prop and aria-disabled attribute#2946beaesguerra wants to merge 4 commits intomainfrom
disabled prop and aria-disabled attribute#2946Conversation
…tting of the aria-disabled attribute
🦋 Changeset detectedLatest commit: 520313a The changes in this PR will be included in the next version bump. This PR includes changesets to release 18 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
npm Snapshot: Published🎉 Good news!! We've packaged up the latest commit from this PR (df3275c) and published all packages with changesets to npm. You can install the packages in ./dev/tools/deploy_wonder_blocks.js --tag="PR2946"Packages can also be installed manually by running: pnpm add @khanacademy/wonder-blocks-<package-name>@PR2946 |
…for the `disabled` props to include details about `aria-disabled` being set internally to keep an element focusable while in a disabled state
|
Size Change: 0 B Total Size: 115 kB ℹ️ View Unchanged
|
A new build was pushed to Chromatic! 🚀https://5e1bf4b385e3fb0020b7073c-mioovvuvwt.chromatic.com/ Chromatic results:
|
| }, | ||
| }, | ||
| disabled: { | ||
| description: "Whether the cell is disabled.", |
There was a problem hiding this comment.
When I tried removing the description in the argtypes, the autodocs didn't include the jsdocs for the prop. I updated argtypes descriptions in these cases!
| }, | ||
| disabled: { | ||
| control: {type: "boolean"}, | ||
| description: "Whether or not the action item is disabled.", |
There was a problem hiding this comment.
There are some cases where we were able to remove the description from the argtypes so that autodocs would pull the docs from the prop type instead!
There was a problem hiding this comment.
yeah, it's tricky to find the root cause for this odd behavior. So far, I've noticed that jsdocs are not included automatically when props:
a) Are imported from a different file (e.g. type Props = AriaProps & CommonPropsFromAnotherFile)
b) Are using ReadOnly: type Props = ReadOnly<{...}>).
c) Are defined with discriminated union types (e.g. Clickable's type defs).
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
jandrade
left a comment
jandrade
left a comment
There was a problem hiding this comment.
LGTM! Thanks a lot for improving the docs around this! we really needed to have more consistent docs 🚀
| }, | ||
| disabled: { | ||
| control: {type: "boolean"}, | ||
| description: "Whether or not the action item is disabled.", |
There was a problem hiding this comment.
yeah, it's tricky to find the root cause for this odd behavior. So far, I've noticed that jsdocs are not included automatically when props:
a) Are imported from a different file (e.g. type Props = AriaProps & CommonPropsFromAnotherFile)
b) Are using ReadOnly: type Props = ReadOnly<{...}>).
c) Are defined with discriminated union types (e.g. Clickable's type defs).
There was a problem hiding this comment.
suggestion: We probably don't need to bump a new version as the JSDocs are not included as part of the bundle created for consumers. This could be changed to create an empty changeset instead.
Here's an example of how a WB generated bundle looks like:
https://www.npmjs.com/package/@khanacademy/wonder-blocks-breadcrumbs?activeTab=code
There was a problem hiding this comment.
Oh interesting, I thought we needed new versions released for the packages where we changed the component jsdocs, so that IDEs can use the updated docs for intellisense features! What do you think?
For the npm snapshot release for this PR, I see the updated jsdocs is included in the ts file (@khanacademy/wonder-blocks-button/dist/util/button.types.d.ts): https://www.npmjs.com/package/@khanacademy/wonder-blocks-button/v/0.0.0-PR2946-20260128221406?activeTab=code
There was a problem hiding this comment.
Great point! this makes total sense :)
3 participants
Summary:
Updating existing docs around the
disabledprop across the components in the design system. We include details around how we will set thearia-disabledattribute internally so that the element remains focusable and is included in the tab order.While going through the components that support a
disabledprop, I found that the Checkbox and Radio components currently still use thedisabledattribute instead ofaria-disabled. I created WB-2218 for this!Note: I also updated the prop docs descriptions defined in argTypes. I first tried to remove them so we could use the js docs for the props type, however, in the remaining cases, these descriptions wouldn't be auto-populated
Issue: WB-2204
Test plan:
Review updated docs make sense and
disabledprops across components are updated to explain thearia-disabledbehaviour