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

TextArea Re-sizing Issue #6924

Merged
merged 5 commits into from Aug 31, 2023
Merged

TextArea Re-sizing Issue #6924

merged 5 commits into from Aug 31, 2023

Conversation

beahackman
Copy link
Collaborator

What does this PR do?

Added a max-width=100% property to the TextArea component to address re-sizing issue and ran prettier.

Where should the reviewer start?

StyleTextArea.js file is where the change has been done.

What testing has been done on this PR?

Ran locally on storybook.

How should this be manually tested?

Do Jest tests follow these best practices?

  • screen is used for querying.
  • The correct query is used. (Refer to this list of queries)
  • userEvent is used in place of fireEvent.
  • asFragment() is used for snapshot testing.

Any background context you want to provide?

What are the relevant issues?

#6919

Screenshots (if appropriate)

Do the grommet docs need to be updated?

Should this PR be mentioned in the release notes?

Is this change backwards compatible or is it a breaking change?

Yes

Copy link
Collaborator

@MikeKingdom MikeKingdom left a comment

Choose a reason for hiding this comment

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

The change looks good.
You should revert the yarn.lock change though. You shouldn't need to updated yarn.lock in this case

Copy link
Collaborator

@MikeKingdom MikeKingdom left a comment

Choose a reason for hiding this comment

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

Much better. And it looks good on the storybook.

You'll also need to run yarn test-update and include the updated snapshots since the new max-width style is showing up there and causing tests to fail.

Copy link
Collaborator

@MikeKingdom MikeKingdom left a comment

Choose a reason for hiding this comment

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

Awesome. Thanks!

Copy link
Collaborator

@jcfilben jcfilben left a comment

Choose a reason for hiding this comment

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

Looks good!

@jcfilben jcfilben merged commit 28550e8 into grommet:master Aug 31, 2023
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants