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

fix(Accordion): update Accordion styles to match spec #12148

Merged
merged 4 commits into from
Oct 6, 2022

Conversation

tw15egan
Copy link
Member

@tw15egan tw15egan commented Sep 23, 2022

Closes #12141
Closes #12223

Fixes a few style issues with Accordion

Changelog

Changed

  • lg size Accordion is now center aligned
  • Background hover color is now $background-hover, not $layer-hover

Testing / Reviewing

  • Go to the Playground story and set the size to lg, ensure the text and chevron are aligned
  • Hover over the AccordionItem and ensure the token is $background-hover on the ::before element

cc @shixiedesign

@netlify
Copy link

netlify bot commented Sep 23, 2022

Deploy Preview for carbon-components-react ready!

Name Link
🔨 Latest commit 21aa6c3
🔍 Latest deploy log https://app.netlify.com/sites/carbon-components-react/deploys/632dd6ef777fe20009c01f4e
😎 Deploy Preview https://deploy-preview-12148--carbon-components-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Sep 23, 2022

Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit 21aa6c3
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/632dd6efc97a3a0008ca9e78
😎 Deploy Preview https://deploy-preview-12148--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Sep 23, 2022

Deploy Preview for carbon-components-react ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 30d4520
🔍 Latest deploy log https://app.netlify.com/sites/carbon-components-react/deploys/633ef72ebbd6aa0009f4fda4
😎 Deploy Preview https://deploy-preview-12148--carbon-components-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Sep 23, 2022

Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit 30d4520
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/633ef72e8aabf6000908ee62
😎 Deploy Preview https://deploy-preview-12148--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@shixiedesign
Copy link
Contributor

Both issues are fixed! Thank you @tw15egan 🎉 🎉 🎉

@aagonzales
Copy link
Member

aagonzales commented Sep 28, 2022

The only thing about using background-hover is that its transparent and the border dividers now show up with the hover.

image

Where as before they were blended in/hidden.

image

@tw15egan
Copy link
Member Author

@aagonzales, do we just need to update the spec on the website re: background-hover? lg fix can remain in the PR, but if its just a spec change I can PR the website to update the token value

@aagonzales
Copy link
Member

aagonzales commented Sep 28, 2022

Yeah after looking at it a little more I think we should keep it $layer-hover in code (its using layer-hover in both figma and sketch as well) and just update the website to reflect that. @tw15egan

@kodiakhq kodiakhq bot merged commit c1df02a into carbon-design-system:main Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
5 participants