-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
fix(type): add default type feature flag #3742
fix(type): add default type feature flag #3742
Conversation
Deploy preview for the-carbon-components ready! Built with commit 02139bf https://deploy-preview-3742--the-carbon-components.netlify.com |
Deploy preview for carbon-elements ready! Built with commit 02139bf |
Deploy preview for carbon-components-react ready! Built with commit 02139bf https://deploy-preview-3742--carbon-components-react.netlify.com |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 - Thanks @jnm2377! BTW strong
thing is intentional, right?
@@ -199,3 +199,9 @@ | |||
@include carbon--icons; | |||
} | |||
} | |||
|
|||
@include exports('css--default-type') { | |||
@if variable-exists(css--default-type) == false or $css--default-type==true { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style nit - Probably spaces around ==
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Assuming moving the strong declaration from default type because thats where it was before this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me, and seems the snapshots need updates to pass CI
@alisonjoseph yes, the strong declaration was in the css reset before this change, so I left it in there. @emyarod will update the snapshots 👍 |
Ref #3648
Changed