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-6418] - MUI v5 Migration - Components > Toggle #8990

Merged

Conversation

bnussman-akamai
Copy link
Member

Description 📝

  • I don't even know if this counts as a react tss migration but a ticket existed for the <Toggle /> but there are no styles applied to it so I just did a clean up and modernization of the component

How to test 🧪

  • Find places in the app that use toggles, verify the toggles still look normal and function normally

@cypress
Copy link

cypress bot commented Apr 12, 2023

1 failed and 1 flaky tests on run #2950 ↗︎

1 145 3 0 Flakiness 1

Details:

migrate `Toggle`
Project: Cloud Manager E2E Commit: 9948219230
Status: Failed Duration: 19:03 💡
Started: Apr 12, 2023 4:11 PM Ended: Apr 12, 2023 4:31 PM
Failed  cypress/e2e/general/smoke-deep-link.spec.ts • 1 failed test

View Output Video

Test Artifacts
smoke - deep link > Go to Profile/Display > by Tab Output Screenshots Video
Flakiness  cypress/e2e/linodes/rescue-linode.spec.ts • 1 flaky test

View Output Video

Test Artifacts
rescue linode > rescue a linode Output Screenshots Video

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@@ -1,2 +1 @@
import Toggle from './Toggle';
export default Toggle;
export { Toggle, ToggleProps } from './Toggle';
Copy link
Contributor

@hana-akamai hana-akamai Apr 12, 2023

Choose a reason for hiding this comment

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

Exporting ToggleProps like this displays an error in the console and the app doesn't render when running locally. I think this is why we import and export interfaces with underscore in other components

image

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh interesting, I'm gonna fix it by separating the exports

export { Toggle } from './Toggle';
export type { ToggleProps } from './Toggle';

Copy link
Member Author

Choose a reason for hiding this comment

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

Another reason I don't want to keep the index files around 😖

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for catching this @hana-linode

@bnussman-akamai bnussman-akamai merged commit f4df80a into linode:develop Apr 13, 2023
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.

4 participants