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

refactor: [M3-8652] - TextField Component Cleanup - Part 1 #11134

Merged
merged 7 commits into from
Oct 31, 2024

Conversation

jaalah-akamai
Copy link
Contributor

@jaalah-akamai jaalah-akamai commented Oct 22, 2024

Description 📝

Goal is to eventually move this to linode/ui package, but a little cleanup was necessary.

Changes 🔄

  • Moved docs to storybook
  • Removed makeStyles for sx prop
  • Wrapped onChange handler in a useCallback for perf
  • Made a few things more DRY

Target release date 🗓️

N/A

Preview 📷

Should be no visual changes

How to test 🧪

Verification steps

  • Verify you can still create a linode and other resources without issue
  • Verify there are no visual changes
  • Verify scroll error still works

As an Author I have considered 🤔

Check all that apply

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/Improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

@jaalah-akamai jaalah-akamai self-assigned this Oct 22, 2024
@jaalah-akamai jaalah-akamai changed the title refactor: [M3-8652] - TextField component cleanup before migration refactor: [M3-8652] - TextField Component Cleanup - Part 1 Oct 22, 2024
@jaalah-akamai jaalah-akamai marked this pull request as ready for review October 22, 2024 01:41
@jaalah-akamai jaalah-akamai requested a review from a team as a code owner October 22, 2024 01:41
@jaalah-akamai jaalah-akamai requested review from cpathipa and hkhalil-akamai and removed request for a team October 22, 2024 01:41
}
};
},
[min, max, type, onChange]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only changes here were to wrap this in a useCallback

{helperText}
</FormHelperText>
)}
{helperText && helperTextPosition === 'bottom' && (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cleaned up this condition by setting a default for the helperTextPosition

@@ -17,7 +71,6 @@ export const Default: Story = {
noMarginTop: true,
placeholder: 'Placeholder',
},
render: (args) => <TextField {...args} />,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

render wasn't necessary for variants

Copy link
Contributor

@hkhalil-akamai hkhalil-akamai left a comment

Choose a reason for hiding this comment

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

Changes look good! Just one visual change from develop to review.

packages/manager/src/components/TextField.tsx Show resolved Hide resolved
packages/manager/src/components/TextField.tsx Outdated Show resolved Hide resolved
@jaalah-akamai jaalah-akamai added the Add'tl Approval Needed Waiting on another approval! label Oct 28, 2024
Copy link

github-actions bot commented Oct 28, 2024

Coverage Report:
Base Coverage: 87.23%
Current Coverage: 87.22%

@cpathipa cpathipa added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Oct 29, 2024
@jaalah-akamai
Copy link
Contributor Author

Flaky DB unit test:
Screenshot 2024-10-31 at 11 48 15 AM

@jaalah-akamai jaalah-akamai merged commit 08b752d into linode:develop Oct 31, 2024
22 of 23 checks passed
Copy link

cypress bot commented Oct 31, 2024

Cloud Manager E2E    Run #6763

Run Properties:  status check passed Passed #6763  •  git commit 08b752d44f: refactor: [M3-8652] - TextField Component Cleanup - Part 1 (#11134)
Project Cloud Manager E2E
Branch Review develop
Run status status check passed Passed #6763
Run duration 26m 47s
Commit git commit 08b752d44f: refactor: [M3-8652] - TextField Component Cleanup - Part 1 (#11134)
Committer Jaalah Ramos
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 1
Tests that did not run due to a developer annotating a test with .skip  Pending 2
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 445
View all changes introduced in this branch ↗︎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge! Modularization
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants