Skip to content

refactor: [M3-6282] MUI v5 Migration - SRC > Features > StackScripts pt1#9773

Merged
coliu-akamai merged 17 commits intolinode:developfrom
coliu-akamai:r-m3-6282
Oct 12, 2023
Merged

refactor: [M3-6282] MUI v5 Migration - SRC > Features > StackScripts pt1#9773
coliu-akamai merged 17 commits intolinode:developfrom
coliu-akamai:r-m3-6282

Conversation

@coliu-akamai
Copy link
Contributor

@coliu-akamai coliu-akamai commented Oct 10, 2023

Description 📝

Part 1 of migrating the MUI styling for the StackScripts package. This PR is for the Partials, SelectStackScriptPanel, StackScriptBase, StackScriptCreate and StackScriptForm subpackages.

Changes 🔄

  • Migrated styling from makeStyles to use Styled components and sx
  • Removed React.FC
  • updated imports to be named exports instead of default exports (unless lazily exported / the component had higher order functions applied to it)
  • removed unnecessary index.tsx files

How to test 🧪

There should be no visual changes.

Prerequisites

(How to setup test environment)

  • Set up local host and navigate to the stack scripts section

Verification steps

(How to verify changes)

  • Verify that there are no visual changes on the StackScript landing page
  • Verify that the StackScript create page still looks and functions the same
  • Verify that the StackScript section in the Linode Create flow still looks and functions the same

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

: {
width: '26%',
}),
cursor: 'default !important',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Kept !important in this file since it's needed to maintain the cursor (or padding at line 82)

})
);

// will convert these in part m3-6282 pt2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's currently repeat styling here -- this file will be cleaned up a lot in the next PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Realized I missed some labels for the styled components in this file -- they'll be added in the part 2 of this PR

@coliu-akamai coliu-akamai marked this pull request as ready for review October 10, 2023 16:49
@coliu-akamai coliu-akamai requested a review from a team as a code owner October 10, 2023 16:49
@coliu-akamai coliu-akamai requested review from dwiley-akamai and jaalah-akamai and removed request for a team October 10, 2023 16:49
Copy link
Member

@bnussman-akamai bnussman-akamai left a comment

Choose a reason for hiding this comment

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

Found no style regressions!

@coliu-akamai coliu-akamai added Add'tl Approval Needed Waiting on another approval! and removed Ready for Review labels Oct 11, 2023
Copy link
Contributor

@dwiley-akamai dwiley-akamai left a comment

Choose a reason for hiding this comment

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

Linode creation from StackScript and StackScript creation ✅
Visual parity compared to prod for StackScript sections ✅

The only minor difference I observed, and it's probably not from this PR, was the success notice for StackScript creation:

This branch

Screenshot 2023-10-11 at 4 30 57 PM

Prod

Screenshot 2023-10-11 at 4 30 52 PM

That seems like more of a fix than a regression to me though.

coliu-akamai and others added 3 commits October 11, 2023 17:12
…ckScriptBase.styles.ts

Co-authored-by: Dajahi Wiley <114682940+dwiley-akamai@users.noreply.github.com>
Co-authored-by: Dajahi Wiley <114682940+dwiley-akamai@users.noreply.github.com>
@coliu-akamai coliu-akamai removed the Add'tl Approval Needed Waiting on another approval! label Oct 11, 2023
@coliu-akamai coliu-akamai added the Approved Multiple approvals and ready to merge! label Oct 11, 2023
@coliu-akamai
Copy link
Contributor Author

merging -- confirmed that stackscript related cypress tests pass!

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!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants