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.

Sign up for GitHub

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

Jump to bottom

chore(styles): deprecate misc features for v10 #1916

Merged
merged 4 commits into from
Mar 6, 2019
Merged

chore(styles): deprecate misc features for v10 #1916

merged 4 commits into from
Mar 6, 2019

Conversation

asudoh
Copy link
Contributor

@asudoh asudoh commented Feb 26, 2019

Refs #1626.

Changelog

New

  • _deprecate.scss file to contain only @mixin deprecate - Having it in _helper-mixin.scss has caused import loop.
  • @deprecated annotations for several variables/mixins/functions
  • Deprecate message for deprecated mixins

Testing / Reviewing

Testing should make sure no component is broken.

@asudoh asudoh requested a review from jnm2377 February 26, 2019 06:56
@asudoh
Copy link
Contributor Author

asudoh commented Feb 26, 2019

Questions primary for @joshblack:

Thanks!

@netlify
Copy link

netlify bot commented Feb 26, 2019 •
edited
Loading

Deploy preview for the-carbon-components ready!

Built with commit 8bcbac3

https://deploy-preview-1916--the-carbon-components.netlify.com

@joshblack
Copy link
Contributor

Does carbon-elements have "type reset" feature? (e.g. Applying .bx--type-alpha to <h1>)

Not currently, wasn't sure what we wanted to map over to the different headings. cc @jeanservaas do you think we should do this by default?

Does cabron-elements have a corresponding function to @function padding()?

No, this is no longer applicable to the new grid work.

Does cabron-elements have a corresponding feature to @mixin grid-container?

Would most likely correspond to these: https://github.com/IBM/carbon-elements/blob/master/packages/grid/scss/grid.scss#L14-L24

Does cabron-elements have a corresponding feature to @mixin font-smoothing?

Does cabron-elements have a corresponding feature to @mixin letter-spacing?

No, mostly because these mixins are no longer used because of type styles 👍

How do we want to handle color reset for <body> found in _css--body.scss in v10?

Good question, I think this is as intended right now but cc @aagonzales to confirm that we want $text-01 as the default color and ui-02 as the default background color?

Is carbon--spacing() is the right migration path for $spacing-*/$layout-*?

This needs to be resolved, I think. cc @jeanservaas what did we end up doing for the spacing scale?

@asudoh
Copy link
Contributor Author

asudoh commented Feb 27, 2019

Thanks @joshblack for your response! Added some additional @deprecated annotations where you provided the migration path.

@jeanservaas @aagonzales Great to have your feedback!

@asudoh asudoh mentioned this pull request Feb 28, 2019
Closed
@aagonzales
Copy link
Member

Q: How do we want to handle color reset for found in _css--body.scss in v10?
Josh: Good question, I think this is as intended right now but cc @aagonzales to confirm that we want $text-01 as the default color and ui-02 as the default background color?

Yes, except the default background should be $ui-background

@jeanservaas
Copy link
Collaborator

Hey @joshblack sorry for the delay:

Does carbon-elements have "type reset" feature? (e.g. Applying .bx--type-alpha to <h1>)

Not currently, wasn't sure what we wanted to map over to the different headings. cc @jeanservaas do you think we should do this by default?

No, we don't want to do this, that's the whole point of the semantic structure that it's purposely not tied to the H classes

@jeanservaas what did we end up doing for the spacing scale?

We ditched the t-shirt sizes for the spacing scale, the tokens are correct as you see them on the site right now $spacing-01... $layout-01 etc.

@joshblack
Copy link
Contributor

Sounds good @jeanservaas 👍 Will track implementation in: https://github.com/IBM/carbon-elements/issues/389

@asudoh
chore(styles): updated body reset for v10
5f8b542 
Also removed type reset from v10.
@asudoh asudoh requested a review from a team March 6, 2019 06:47
@asudoh
Copy link
Contributor Author

asudoh commented Mar 6, 2019

Thanks @aagonzales and @jeanservaas for the response!

@IBM/carbon-developers Could you proceed with code review? Please use "Diff settings" - "Hide whitespace changes" given some files have change in indent. Thanks!

dakahn
dakahn approved these changes Mar 6, 2019
View reviewed changes
Copy link
Contributor

@dakahn dakahn left a comment

Choose a reason for hiding this comment

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

🐬 Great! Thank you so much 🐬

jnm2377
jnm2377 approved these changes Mar 6, 2019
View reviewed changes
Copy link
Contributor

@jnm2377 jnm2377 left a comment

Choose a reason for hiding this comment

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

Looks great!

@jnm2377 jnm2377 merged commit e16b80d into carbon-design-system:master Mar 6, 2019
@asudoh asudoh deleted the deprecate-sass branch March 6, 2019 22:20
@carbon-bot
Copy link
Contributor

🎉 This PR is included in version 9.83.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@asudoh asudoh mentioned this pull request Mar 14, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dakahn dakahn dakahn approved these changes

@jnm2377 jnm2377 jnm2377 approved these changes

@lovemecomputer lovemecomputer lovemecomputer approved these changes

Assignees
No one assigned
Labels
status: ready for review 👀
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@asudoh @joshblack @aagonzales @jeanservaas @carbon-bot @lovemecomputer @jnm2377 @dakahn @tw15egan