-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
ColorIndicator: Convert component to TypeScript #41587
Conversation
Size Change: +13 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks!
packages/components/src/color-indicator/test/__snapshots__/index.tsx.snap
Show resolved
Hide resolved
Co-authored-by: Lena Morita <lena@jaguchi.com>
export function ColorIndicator( | ||
// ref is omitted until we have `WordPressComponentPropsWithoutRef` or add | ||
// ref forwarding to ColorIndicator. | ||
props: Omit< | ||
WordPressComponentProps< ColorIndicatorProps, 'span', false >, | ||
'ref' | ||
> | ||
) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we forwardRef
here as well? cc @mirka
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me know and I'll update the PR / do a follow-up PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, let's forward! (In this PR or follow-up, whichever is most convenient to you)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've just updated the Migration guidelines to include a point re. this (point 7 / vii
):
Example snippet:
// With `forwardRef`
import type { ForwardedRef } from 'react';
import { forwardRef } from '@wordpress/element';
import type { WordPressComponentProps } from '../ui/context';
import type { ComponentOwnProps } from './types';
function UnforwardedMyComponent(
props: WordPressComponentProps< ComponentOwnProps, 'div', true >,
forwardedRef: ForwardedRef< any >
) { /* ... */ }
/**
* MyComponent JSDoc
*/
export const MyComponent = forwardRef( UnforwardedMyComponent );
export default MyComponent;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Been away for a few days. Will need to either make a follow-up on update this next week.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ForwardRef are now added to this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, thanks! 🚀
What?
Converts the
ColorIndicator
component to TypeScript.Why?
Part of the @wordpress/components's TypeScript migration (#35744).
How?
Refactors the current
ColorIndicator
to TypeScript.Testing Instructions
ColorIndicator
continues to function as expected