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

Collapse documentation doesn't mention anything about the class "collapsed" #28648

Open
KaranLala opened this issue Apr 12, 2019 · 7 comments
Open

Comments

@KaranLala
Copy link

The class "collapsed" is added to the trigger element when the target element is hidden, but this feature isn't mentioned anywhere in the collapse documentation. Is this a formally recognized feature?

I'm sure many others, like me, would like to change their triggers style when showing/hiding the target element, and it's not very clear that it's straightforward to do this purely with CSS because of the collapsed class. It would be helpful to add a few words about this to confirm (or deny) that this is a feature that can and should be used.

@ysds
Copy link
Member

ysds commented May 6, 2019

It seems that this class isn't used in Bootstrap's CSS and JS either (our JS just toggle this class).

I think we need to discuss whether to leave this JS processing.

At least, It is possible to apply styles using the aria-expanded="false" selector instead of using the collapsed class.

@minaelee
Copy link

The collapsed class still exists in Bootstrap 4.5's collapse.js:

const CLASS_NAME_COLLAPSED  = 'collapsed'
          if (!$elem.hasClass(CLASS_NAME_SHOW)) {
            $(trigger).addClass(CLASS_NAME_COLLAPSED)
              .attr('aria-expanded', false)
          }

It is used in the Accordion example in Bootstrap 4.0 and 4.5 documentation - see the buttons in https://getbootstrap.com/docs/4.5/components/collapse/#accordion-example

In testing the Accordion example, the collapsed class does set aria-expanded to false on collapsed elements, and when un-collapsed, it is removed and aria-expanded is set to true. So it appears to be behaving as expected.

However, there is documentation on setting aria-expanded at https://getbootstrap.com/docs/4.5/components/collapse/#accessibility which makes no mention of the existence of the collapsed class, despite its entire point of existence being to set that attribute.

Can collapsed be added to the documentation, or if there is some reason it is not being mentioned (e.g. being deprecated, buggy behavior, etc), can it be removed from the Accordion example to reduce confusion? Thank you.

@minaelee
Copy link

I've checked the Bootstrap 5 documentation as well, and this issue occurs again there. In Bootstrap 5, Accordion has its own documentation page under Components (rather than a subsection in the Collapse page) and both examples use collapsed without providing any definition of it. Neither is it mentioned in the Collapse page.

https://v5.getbootstrap.com/docs/5.0/components/accordion/
https://v5.getbootstrap.com/docs/5.0/components/collapse

@mdo
Copy link
Member

mdo commented Jan 12, 2021

Near as I can tell, .collapsed does nothing in v5, possibly nothing in v4. Can anyone on @twbs/js-review confirm? If so, can we drop it from v5?

@ffoodd
Copy link
Member

ffoodd commented Jan 12, 2021

As far as I recall, we sometimes use it to negate styles: :not(.collapsed). But we may use [aria-expanded] instead.

@XhmikosR
Copy link
Member

It's used in the collapse component to signal the collapse is done showing.

@GeoSot
Copy link
Member

GeoSot commented Apr 8, 2021

In js code, .collapsed is toggled exactly the same time with [aria-expanded].

Do we still want to check if it can be omitted?

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

8 participants