FocusableIframe: refactor to TypeScript #53979
Conversation
| /** | ||
| * Internal dependencies | ||
| */ | ||
| import type { FocusableIframeProps } from './types'; | ||
|
|
||
| /** | ||
| * @param {Object} props |
There was a problem hiding this comment.
* @param {Object} props should be some variant of ComponentPropsWithoutRef< 'iframe' > to match my change? Idk enough about JSDoc types for an opinion. There are a few other good ways to do this: https://mortenbarklund.com/blog/react-typescript-props-spread/ that I could switch to.
There was a problem hiding this comment.
We usually remove the types entirely from JSDocs when using TypeScript types. In this case, since the JSDoc comment was just there for adding type information, I'd argue that we can just simply delete it altogether.
mirka
added
[Type] Code Quality
margolisj
force-pushed
the
FocusableIframe
branch
from
d510417
to
13fba3e
Compare
There was a problem hiding this comment.
Thank you for working on this! This PR is good to go after deleting the JSDoc comment block as discussed in https://github.com/WordPress/gutenberg/pull/53979/files#r1306938378
| /** | ||
| * Internal dependencies | ||
| */ | ||
| import type { FocusableIframeProps } from './types'; | ||
|
|
||
| /** | ||
| * @param {Object} props |
There was a problem hiding this comment.
We usually remove the types entirely from JSDocs when using TypeScript types. In this case, since the JSDoc comment was just there for adding type information, I'd argue that we can just simply delete it altogether.
margolisj
force-pushed
the
FocusableIframe
branch
from
6dbd71c
to
9598a11
Compare
|
@ciampo Removed JSDocs and rebased to make sure the changelog was happy with both of these branches making internal sections. |
What?
Why?
How?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast