Skip to content

Disabled: Improve TypeScript code after recent refactor to inert #45252

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.

Sign up for GitHub

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

Closed
ciampo wants to merge 2 commits into from
Closed

Disabled: Improve TypeScript code after recent refactor to inert #45252

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
180be7d
Disabled: Improve TypeScript code after recent refactor to inert
ciampo Oct 24, 2022
8d90d4e
Bring back param types
tyxla Oct 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Disabled: Improve TypeScript code after recent refactor to inert
  • Loading branch information
@ciampo
ciampo committed Oct 24, 2022
commit 180be7da3901cc15f2f2fc1d05e8a156ed8a76e7
2 changes: 1 addition & 1 deletion packages/components/src/disabled/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ function Disabled( {
return (
<Provider value={ isDisabled }>
<div
// @ts-ignore Reason: inert is a recent HTML attribute
// @ts-expect-error Reason: inert is a recent HTML attribute
inert={ isDisabled ? 'true' : undefined }
className={
isDisabled
Expand Down
5 changes: 0 additions & 5 deletions packages/components/src/disabled/test/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ describe( 'Disabled', () => {
</Disabled>
);

// @ts-ignore
expect( container.firstChild ).toHaveAttribute( 'inert' );
} );

Expand All @@ -40,12 +39,10 @@ describe( 'Disabled', () => {

const { container, rerender } = render( <MaybeDisable /> );

// @ts-ignore
expect( container.firstChild ).toHaveAttribute( 'inert' );

rerender( <MaybeDisable isDisabled={ false } /> );

// @ts-ignore
expect( container.firstChild ).not.toHaveAttribute( 'inert' );
} );

Expand All @@ -58,12 +55,10 @@ describe( 'Disabled', () => {

const { rerender, container } = render( <MaybeDisable /> );

// @ts-ignore
expect( container.firstChild ).toHaveAttribute( 'inert' );

rerender( <MaybeDisable isDisabled={ false } /> );

// @ts-ignore
expect( container.firstChild ).not.toHaveAttribute( 'inert' );
} );

Expand Down
13 changes: 9 additions & 4 deletions packages/compose/src/hooks/use-disabled/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
/**
* External dependencies
*/
import type { RefCallback } from 'react';

/**
* Internal dependencies
*/
Expand All @@ -11,9 +16,9 @@ import useRefEffect from '../use-ref-effect';
*
* If you can, prefer the use of the inert HTML attribute.
*
* @param {Object} config Configuration object.
* @param {boolean=} config.isDisabled Whether the element should be disabled.
* @return {import('react').RefCallback<HTMLElement>} Element Ref.
* @param config Configuration object.
Copy link
Member

@tyxla tyxla Oct 25, 2022

Choose a reason for hiding this comment

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

We still need the types here, don't we? The docs build script was failing because of missing param types.

I just pushed a commit to restore them, but let me know if this had some special purpose.

Copy link
Contributor Author

@ciampo ciampo Oct 25, 2022

Choose a reason for hiding this comment

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

Nice catch! We usually remove JSDoc types when we add the TypeScript ones, but in this case it looks like they have to be kept around for the docgen — TIL!

* @param config.isDisabled Whether the element should be disabled.
* @return Element Ref.
*
* @example
* ```js
Expand Down Expand Up @@ -77,5 +82,5 @@ export default function useDisabled( {
};
},
[ isDisabledProp ]
);
) as RefCallback< HTMLElement >;
}