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

feat(styles): add accordion styles and tests #8735

Merged

Conversation

joshblack
Copy link
Contributor

Part of #8555

This converts our accordion styles to Sass modules and also updates some of the existing packages to make it work. There were some challenges that came up where it would be great to get feedback on:

  • Should the _theme.scss module export all theme members, or just colors?
  • How do we want to handle namespaces? In other words, when should we use @use 'module-name' versus use 'module-name' as * to bring members in to scope

Changelog

New

  • Add _tokens.scss file to themes to map variables like $interactive-01 over to their CSS Custom Property variant
  • Add accordion component scss folder

Changed

  • Update utilities to make the namespace import less awkward
    • For example, @include button-reset.button-reset() becomes @include button-reset.reset()

Removed

Testing / Reviewing

  • Make sure the test for accordion passes in styles (which means the styles can compile and be configured)

@joshblack joshblack requested a review from a team as a code owner May 21, 2021 16:36
@joshblack joshblack requested review from emyarod and dakahn May 21, 2021 16:36
@joshblack
Copy link
Contributor Author

cc @tw15egan would love your thoughts here 👀

@netlify
Copy link

netlify bot commented May 21, 2021

Deploy Preview for carbon-elements ready!

Built with commit 9b2c079

https://deploy-preview-8735--carbon-elements.netlify.app

@netlify
Copy link

netlify bot commented May 21, 2021

Deploy Preview for carbon-components-react ready!

Built without sensitive environment variables with commit 9b2c079

https://deploy-preview-8735--carbon-components-react.netlify.app

Copy link
Member

@emyarod emyarod left a 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 I believe a couple of snapshots will need to be updated

@kodiakhq kodiakhq bot merged commit d00d999 into carbon-design-system:main May 24, 2021
@jnm2377 jnm2377 mentioned this pull request May 24, 2021
22 tasks
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.

3 participants