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] Add accordion to theme #212

Merged
merged 6 commits into from Jul 3, 2019

Conversation

davidicus
Copy link
Contributor

This PR adds the carbon accordion component to the gatsby carbon theme. It also adds a page to the example gatsby site included in this repo.

@vercel
Copy link

vercel bot commented Jul 2, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://gatsby-theme-carbon-git-fork-davidicus-feat-accordion.carbon-design-system.now.sh

@davidicus
Copy link
Contributor Author

davidicus commented Jul 2, 2019

Hi @vpicone. I have taken a crack at implementing the Accordion based off your feedback in this issue. I wanted to get started and then get with you to see if you had any more suggestions.

@vercel vercel bot temporarily deployed to staging July 3, 2019 03:28 Inactive
@vpicone
Copy link
Collaborator

vpicone commented Jul 3, 2019

Hey @davidicus thanks so much for the PR it looks great. Just a couple small things.

  1. We need to apply responsive widths to the accordion
.accordion {
  width: 100%;
  @include carbon--breakpoint('md') {
    width: 75%;
  }
  @include carbon--breakpoint('lg') {
    width: 58.33%;
  }
}
  1. We meed to use a bold text for the title like you see here at the brand center implementation. Mike and Sadek had a hand at desiging the implementation you see there, but I think the bold title seems to be the only thing missing.

A couple tips:

you'll need to include these at the top of your sass module to do the breakpoints/type stuff

@import '~carbon-components/scss/globals/scss/vars';
@import '~@carbon/elements/scss/type/type';

Also, if you need to target a class that you don't have control over from a module you can do so with the global keyword

.accordion :global(.bx--accordion__title){
	title styles
}

@davidicus
Copy link
Contributor Author

@vpicone do you want me to exactly mirror the styling from that brand center implementation? If so I think I still need to add some padding on the accordion title and the focus blue state. Let me know and I will add.

@vpicone
Copy link
Collaborator

vpicone commented Jul 3, 2019

Could you elaborate on the focus blue state? I'd rather leave the padding as the Carbond default, they can modify that in the brand center site if they want.

@davidicus
Copy link
Contributor Author

actually nevermind. I didnt think the default had the blue outline when the title is focused but it does

image

And in that case. This should be good to go unless you have any other directions.

@vpicone
Copy link
Collaborator

vpicone commented Jul 3, 2019

@davidicus Looks great! thanks for the contribution, I'll try to cut a release today or tomorrow.

@vpicone vpicone merged commit 7c4b226 into carbon-design-system:master Jul 3, 2019
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.

None yet

2 participants