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-6195 - Add tss-react and refactor Button to styled API #8821

Merged
merged 2 commits into from
Feb 25, 2023

Conversation

jaalah-akamai
Copy link
Contributor

@jaalah-akamai jaalah-akamai commented Feb 24, 2023

Description 📝

This refactors the Button component to the new sx and styled API. I also added tss-react so that we can begin migrating the older makeStyles API which will give us better type safety and autocomplete than MUI v4, while providing an almost seamless transition in moving away from the deprecated JSS API.

Major Changes 🔄

  • Added tss-react
  • Added Button.test.tsx for more coverage
  • Refactored Button to use styled API
  • Added sx prop to HelpIcon for one-off styling
  • Added util isPropValid to filter out props that are not valid DOM attributes
  • Created packages/manager/src/styles folder for all future shared keyframes

Preview 📷

Before & After
Screen Shot 2023-02-23 at 11 00 12 PM

How to test 🧪

  1. Start local environments and storybook to ensure Button stayed the same.
  2. Run unit test in root yarn test packages/manager/src/components/Button and observe unit tests pass.
  3. Run E2E tests in root yarn cy:debug

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.

LGTM and I agree with this direction for styles moving forward!

import * as React from 'react';
import Reload from 'src/assets/icons/reload.svg';
import _Button, { ButtonProps } from 'src/components/core/Button';
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like src/components/core/Button isn't used anywhere anymore. Should it be deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's still used in src/components/PromotionalOfferCard/PromotionalOfferCard.tsx and /src/components/Tile/Tile.tsx, but I agree! It will 100% get removed in the larger migration when we get to those files specifically. Good call out 👍

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.

LGTM 🚢

@jaalah-akamai jaalah-akamai merged commit 6b8f2f8 into linode:develop Feb 25, 2023
@jaalah-akamai jaalah-akamai deleted the M3-6195 branch February 25, 2023 04:24
dwiley-akamai pushed a commit to dwiley-akamai/manager that referenced this pull request Mar 6, 2023
hana-akamai added a commit that referenced this pull request Mar 23, 2023
* add user data accordion to linode create

* display accordion if image supports cloud-init

* update linode create payload

* check image for cloud-init

* Add validation warning for user data

Co-authored-by: Hussain Khalil <hussain@sanpilot.co>

* feat: [M3-6149] - Add Cloud Init checkbox to Image Capture/Create flow (#8807)

* Add checkbox to Capture Image form

* Update request to support cloud_init

* Fix controlled component error where checkbox is undefined

* Update image validation and types interface for cloud_init

* (Debugging  an undefined value in createImage request)

* Add cloud_init to redux create fn params to include in request

* Move checkbox state to ImageCreate so value persists across tabs; code cleanup

* Code clean up

* Fix comment referencing cloud-init for naming consistency

* Update CheckBox tooltip: allow interactivity, minor styling fix

* Add final tooltip copy for now

* Revert "Merge branch 'develop' into feature/metadata"

This reverts commit 8be0c73, reversing
changes made to a5e1b25.

* feat: [M3-6150] - Add Cloud-init compatible check box to Image Upload flow (#8800)

* Add Cloud-init compatible check box

* use Checkbox component and reduce space between label and tooltip icon

* add new fileds for image create

* code cleanup

* make cloud_in as optional

* Code cleanup

* Copy update and PR feedback

* code cleanup

* Enable interactive to tooltip

* feat: [M3 6143] - Add User Data Accordion to Linode Create page (#8811)

## Description 📝
Add User Data accordion to the Linode Create flow so that a user can configure their Linode using cloud-init.

## How to test 🧪
Testing Create via Distribution
1. Point to the dev environment (you might also need additional account setup, reach out for more info)
2. Navigate to the Linode create page http://localhost:3000/linodes/create
3. Select a distribution that supports cloud-init
    - Arch Linux, Cent OS 7, and Ubuntu 16.04 LTS have been marked as cloud-init compatible for testing purposes
4. If the distribution supports cloud-init, you should see an `Add User Data` accordion
    - If the distribution doesn't support cloud-init, you should _not_ see the accordion
5. Enter some text (can be anything) into the User Data input field and fill out the rest of the required Linode Create fields
6. Click on Create Linode and observe the POST payload to `/linode/instances`. You should see a metadata field with the user_data object
    - If you did not enter anything into the User Data input, you should _not_ see any metadata field in the payload.
    - You can just block the network request to `/linode/instances` and check the payload w/o actually creating another Linode
7. There are no changes to the API response so you will not see a metadata field returned

Testing Create via Image
- Navigate to the Linode Create Image tab http://localhost:3000/linodes/create?type=Images
- Select an image that supports cloud-init
  - You can create and mark an image as cloud-init compatible in #8807
- Follow steps 4-7 above

* Move user data validation warning

Co-authored-by: Hussain Khalil <hussain@sanpilot.co>

* Update user data validation warning UX copy

Co-authored-by: Hussain Khalil <hussain@sanpilot.co>

* Refactor user data validation event handler

Co-authored-by: Hussain Khalil <hussain@sanpilot.co>

* Slighly refactor user data validation

Co-authored-by: Hussain Khalil <hussain@sanpilot.co>

* Pull in tss-react & related changes from #8821 (#8843)

* feat: [M3-6146] - Add User Data to Linode Rebuild flow (#8850)

* feat: [M3-6145,  M3-6191, M3-6233] - Add User Data to Linode Clone and Backup flow (#8859)

* Functional flow

* add UserDataAccordion.test.tsx

* Fix test

* Feedback + styling fixes and unit test updates

* Add User Data to Linode Clone flow

* Add respective user data warning message.

* code cleanup

* organize UserDataAccordion componet

* Code cleanup

* refactor: combine two conditional statemets to single one.

* Unit test coverage

* code cleanup

* refactor: code cleanup

---------

Co-authored-by: Dajahi Wiley <dwiley@linode.com>

* fix: [M3-6254] - Show warning message for only Linode clone and backups (#8888)

* fix: [M3-6254] show warning message for only Linode clone and backups flow

* PR -feedback

* PR Feedback

* PR Feedback

* Revert "Revert "Merge branch 'develop' into feature/metadata""

This reverts commit 35daaf1.

* feat: [M3-6432] - Add Metadata Feature flag (#8910)

## Description 📝
Feature flag Metadata

## How to test 🧪
Toggle the flag on/off with the dev tools and check that Metadata features display/hide in these flows:

Image Create
    - Capture Image
    - Upload Image

Linode Create via
    - Rebuild
    - Distribution
    - Custom Image
    - Backups
    - Clone

* address feedback

---------

Co-authored-by: Hussain Khalil <hkhalil@akamai.com>
Co-authored-by: Hussain Khalil <hussain@sanpilot.co>
Co-authored-by: Mariah Jacobs <114685994+mjac0bs@users.noreply.github.com>
Co-authored-by: cpathipa <119517080+cpathipa@users.noreply.github.com>
Co-authored-by: Dajahi Wiley <dwiley@linode.com>
Co-authored-by: hkhalil-akamai <122488130+hkhalil-akamai@users.noreply.github.com>
Co-authored-by: Dajahi Wiley <114682940+dwiley-akamai@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants