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

Accordion inside accordion not working properly V10 #4768

Closed
prince737 opened this issue Nov 25, 2019 · 8 comments
Closed

Accordion inside accordion not working properly V10 #4768

prince737 opened this issue Nov 25, 2019 · 8 comments

Comments

@prince737
Copy link

prince737 commented Nov 25, 2019

Environment

Mac os Catalina 10.15.1
Chrome 78.0.3904.97
carbon-components 10.7.4
carbon-components-react 7.7.4

Description

Upon expanding the parent accordion the child accordions open in expanded form and click on their header does not work.

Steps to reproduce the issue

  1. Create an Accordion
  2. Add another accordion with some Items in the AccordionItem of this Accordion
  3. Expand the Parent Accordion

Note: The same code works fine for version 9.57.0 of carbon-components and 6.115.1 of carbon-components-react.

Sandbox

https://codesandbox.io/s/polished-wildflower-y3yps
Change dependencies version to 9.57.0 for carbon-components and 6.115.1 for carbon-components-react and change the CSS link version to 9.57.0 to see the desired working model.

EDIT: Modified CSS in the sandbox to achieve the desired result but it would be better if it ships out of the box with the same. Especially since it was already supported in the previous version.

@prince737 prince737 changed the title Accordion inside accordion not working properly Accordion inside accordion not working properly V10 Nov 25, 2019
@asudoh
Copy link
Contributor

asudoh commented Nov 25, 2019

Hi 👋 thank you for reporting! Nested accordion is not supported, though - More discussion can be found at #2230.

@davidicus
Copy link
Contributor

@asudoh @joshblack can we reopen this issue to provide this functionality. Our dev thinks it would be a pretty trivial style change to be able to support this capability. If either of the options below are acceptable we would be willing to open up a PR. Please advise.

Comment from issue #855 of our addons repo:

This problem is isolated to nested accordions. Root cause is that the BEM class 'bx--accordion__item--active' (meaning open) from the parent accordion is affecting the elements in any child accordions. This makes sense since we only have default styles and the 'active' modifier, no modifier for 'inactive'.

I think there is an easy solution which should be safe to implement. We can add a bx--accordion__item--inactive class that cancels out the parent styles from 'bx--accordion__item--active' , e.g

.#{$prefix}--accordion__item--inactive {  
  .#{$prefix}--accordion__content {
    display: none;
  }

  .#{$prefix}--accordion__arrow {
    transform: rotate(90deg);
  }
}

I don't see any reason that such a fix should not be implemented in the carbon library. If they don't agree we can probably wrap the component and use the 'onHeadingClick' callback to set such style in our wrapped component.

Another option for a pure carbon library fix could be to have the 'bx--accordion__item--active' only affect direct descendent by introducing the child combinator, e.g.

.bx--accordion__item--active > .#{$prefix}--accordion__content {...}
.bx--accordion__item--active > .#{$prefix}--accordion__arrow {...}

this wouldn't require any js changes but is perhaps less BEM compatible.

@davidicus
Copy link
Contributor

This is one of our designs where we needed nested accordion.
image

@asudoh
Copy link
Contributor

asudoh commented Jan 31, 2020

@carbon-design-system/design Any thoughts on how the best design would be for above use case? Thanks!

@asudoh asudoh reopened this Jan 31, 2020
@designertyler
Copy link
Contributor

@asudoh This is from the slack discussion. I spoke with @joshblack and @aagonzales and we're aligned on the design recommendation to find ways to avoid a nested accordion.

The accordion is meant show/hide sections of content on a page, but it sometimes gets used as a replacement for navigation.

In the posted example would the page still make sense if all the information was expanded and shown at once? If yes, we could move the pump inspection headers and summary out of the accordion and only have the instructions, history, parts in the accordions. If the information doesn’t make sense when everything is expanded on the same page, it would probably be better to have the pump inspections link to dedicated pages for that content.

@asudoh
Copy link
Contributor

asudoh commented Feb 3, 2020

Thanks @designertyler! Closing for now given the discussion, but can re-open once if any other possible valid use-cases come up.

@gptt916
Copy link
Contributor

gptt916 commented Sep 25, 2020

Hi @asudoh, one use case is on our faq page:
image

Where each faq topic is nested under a collection:

Collection 1:
    Faq topic 1:
        Faq content
    Faq topic 2
Collection 2
Collection 3

... etc

@chrisconnors-ibm
Copy link
Contributor

The position is that generally, nested accordions should be avoided. That doesn't mean there might not be a use case where a team might "need" it. However, the accordion component in the Core/Common package is not going to be expanded to accommodate this feature. There's many reasons why we do not want to encourage this pattern, across the business, by incorporating it into this component, meant to be used by everyone at IBM, everywhere.

@davidicus > Our dev thinks it would be a pretty trivial style change to be able to support this capability. If either of the options below are acceptable we would be willing to open up a PR.

In this case, your team is able to create your own "nested accordion" component, extending or modifying the Core/Common one, to satisfy the use case for your business. You are also able to have it included in the component index, if you think the applications or use cases extend beyond your business' needs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants