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

[TextareaAutosize] Convert to TypeScript #34751

Closed
wants to merge 4 commits into from

Conversation

paco9595
Copy link

@paco9595 paco9595 commented Oct 13, 2022

I update the TextAreaAutoSize to typeScript

Closes #34721

@mui-bot
Copy link

mui-bot commented Oct 13, 2022

Details of bundle changes

Generated by 🚫 dangerJS against d446d32

@michaldudak
Copy link
Member

There are some issues with the build. Could you please verify your imports?

@paco9595 paco9595 closed this Oct 15, 2022
@paco9595 paco9595 reopened this Oct 15, 2022
@paco9595 paco9595 changed the title Text area auto size [TextAreaAutoSize] to typescript Oct 15, 2022
@zannager zannager added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Oct 17, 2022
@michaldudak michaldudak changed the title [TextAreaAutoSize] to typescript [TextareaAutosize] Convert to TypeScript Oct 17, 2022
@oliviertassinari oliviertassinari removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Oct 17, 2022
@zannager zannager added the component: TextareaAutosize The React component. label Oct 18, 2022
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 20, 2022
'themeStyleOverrides',
'themeVariants',
],
} as any),
Copy link
Member

Choose a reason for hiding this comment

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

Casting to any should be used as a last resort. In this case, it should be possible to correct the code so it's not needed.

) {
const lineHeight = typeof lineHeightArg === 'function' ? lineHeightArg : () => lineHeightArg;

getComputedStyleStub[input] = getComputedStyle;
getComputedStyleStub[input as any] = getComputedStyle;
Copy link
Member

Choose a reason for hiding this comment

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

Something is not right here. An HTMLInputElement shouldn't be used as an index.

Comment on lines +74 to +76
const input: HTMLInputElement = container.querySelector(
'textarea[aria-hidden=null]',
) as HTMLInputElement;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const input: HTMLInputElement = container.querySelector(
'textarea[aria-hidden=null]',
) as HTMLInputElement;
const input: HTMLInputElement = container.querySelector('textarea[aria-hidden=null]')!;

const shadow = container.querySelector('textarea[aria-hidden=true]');
expect(input.style).to.have.property('height', '');
expect(input.style).to.have.property('overflow', '');

setLayout(input, shadow, {
setLayout(input as HTMLInputElement, shadow as HTMLTextAreaElement, {
Copy link
Member

Choose a reason for hiding this comment

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

No need to cast input as it has the correct type.
Also, make sure that shadow has the correct type to avoid casting.

placeholder?: string;
onChange?: (e: React.FormEvent<HTMLTextAreaElement>) => void;
maxRows?: number | undefined;
minRows: number | undefined;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
minRows: number | undefined;
minRows?: number | undefined;

@@ -85,12 +96,14 @@ describe('<TextareaAutosize />', () => {

it('should update when uncontrolled', () => {
const handleChange = spy();
const { container } = render(<TextareaAutosize onChange={handleChange} />);
const input = container.querySelector('textarea[aria-hidden=null]');
const { container } = render(<TextareaAutosize minRows={1} onChange={handleChange} />);
Copy link
Member

Choose a reason for hiding this comment

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

minRows shouldn't be mandatory

@@ -0,0 +1,257 @@
import * as React from 'react';
Copy link
Member

Choose a reason for hiding this comment

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

There are too many changes in this file for git to treat it as a rename. It would be best to specify the new types in the TextareaAutosize.types.ts, to minimize the number of changes done here, so the history is preserved.
It would also make it easier to review the code.

@@ -0,0 +1 @@
export { default } from './TextareaAutoSize';
Copy link
Member

Choose a reason for hiding this comment

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

The newly added types should also be exported.

@sai6855
Copy link
Contributor

sai6855 commented Jan 18, 2023

@paco9595 can I work on this issue (#34721) as there is no recent activity in this pr

@michaldudak
Copy link
Member

Closing due to inactivity in favor of #35862

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: TextareaAutosize The React component. PR: out-of-date The pull request has merge conflicts and can't be merged typescript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TextareaAutosize][base] Convert code to TypeScript
6 participants