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

Kds 1754 migrate toggle switch field guidance block scene to kaio #4062

Conversation

JakePitman
Copy link
Contributor

Why

Part of migrating legacy components to KAIO

What

Migrates the following legacy packages to packages/compoents:

  • ToggleSwitchField
  • GuidanceBlock
  • Scene

@JakePitman JakePitman requested a review from a team as a code owner September 12, 2023 22:47
@changeset-bot
Copy link

changeset-bot bot commented Sep 12, 2023

⚠️ No Changeset found

Latest commit: e6ef939

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@JakePitman JakePitman force-pushed the KDS-1754-migrate-toggle-switch-field-guidance-block-scene-to-kaio branch from 50b8fe0 to 9543580 Compare September 12, 2023 22:52
.cancel {
@include button-reset;

composes: interactiveIconWrapper from "~@kaizen/component-library/components/Icon/Icon.module.scss";
Copy link
Contributor

Choose a reason for hiding this comment

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

We want to remove these cause they aren't the best supported feature in our bundling tools, and generally confusing.

Winter actually migrated these styles over to components/src/Notification/utils/_styles.scss

Maybe best to move the styles from Notification to components/styles so both components can extend them. I think you want to extend %ca-notification_cancel here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You want all styles from components/src/Notification/utils/_styles.scss moved over & shared? Or just %ca-notification__cancel?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, now that I take a second look, maybe they belong with Icon...? Icon/styles/_styles.scss, thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

Just realized slack isn't a great place for keeping info so re-sharing here to capture as food for thought when we loop back here in a health week:

Without having dug too deep into it, just looking at the functionality of the shared styles, it feels like we should try to use a component like IconButton instead of extending (or composing) a class. I'm assuming the older pattern of Guidance block and Menu may be a factor as to why this was done in the pas

@gyfchong
Copy link
Contributor

You'll need to follow the plop template for all the components. I recommend running plop first, then copying over code into the template so we don't miss things like the stickersheets and .mdx story files

automationId?: string
}

export const FieldGroup = ({
Copy link
Contributor

Choose a reason for hiding this comment

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

Just flagging that this has already been migrated

automationId?: string
}

export const Label = ({
Copy link
Contributor

Choose a reason for hiding this comment

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

Just flagging that this has already been migrated

@gyfchong
Copy link
Contributor

gyfchong commented Nov 6, 2023

Broke this into multiple PRs, closing.

@gyfchong gyfchong closed this Nov 6, 2023
@gyfchong gyfchong deleted the KDS-1754-migrate-toggle-switch-field-guidance-block-scene-to-kaio branch November 6, 2023 03:58
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.

3 participants