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’s aria-expanded attribute never leaves true state. #40841

Open
3 tasks done
mwaibel-go opened this issue Sep 17, 2024 · 3 comments
Open
3 tasks done

Collapse’s aria-expanded attribute never leaves true state. #40841

mwaibel-go opened this issue Sep 17, 2024 · 3 comments
Labels

Comments

@mwaibel-go
Copy link

mwaibel-go commented Sep 17, 2024

Prerequisites

Describe the issue

When a Collapse trigger is clicked, it should update the trigger’s aria-expanded attribute from "true" to "false" and vice versa. The test case shows that only the change from "false" to "true" happens. The attribute never gets updated again when the trigger is clicked again.

Note that the change from "false" to "true" doesn’t happen either if the div.collapse is removed. This might be by design, but I think this is a bug, too. There might be nothing to display at the time the trigger is clicked, but IMHO, the element still got expanded and should reflect that.

The reason I care about this is because I show an icon to display if a collapsible element is currently collapsed. Similar to the demo, I update the icon in CSS based on the trigger’s aria-expanded attribute.

Reduced test cases

https://stackblitz.com/edit/stackblitz-starters-zhsmta?file=index.html

What operating system(s) are you seeing the problem on?

Linux

What browser(s) are you seeing the problem on?

Chrome, Firefox

What version of Bootstrap are you using?

5.3.3

@julien-deramond
Copy link
Member

Thanks for reporting an issue @mwaibel-go
I just looked at your StackBlitz reproducible example.
Maybe I'm wrong, you'll tell me, but It seems that changing the class name from .collapse to anything else actually works (sounds like it works in our docs for instance):

<h1
  aria-expanded="false"
  role="button"
-  data-bs-target="#collapse-demo .collapse"
+  data-bs-target="#collapse-demo #test"
  data-bs-toggle="collapse">
  Collapse trigger
</h1>
<div class="expanded-true">aria-expanded = true</div>
<div class="expanded-false">aria-expanded = false</div>
- <div class="collapse">to be hidden</div>
+ <div class="collapse" id="test">to be hidden</div>

That could be explained by the fact that the .collapse class changes during the animation and the change of states. Targetting with data-bs-target the class .collapse is maybe to avoid, and it would be better to target an id (or another "safe" class name).

@mwaibel-go
Copy link
Author

It seems that changing the class name from .collapse to anything else actually works

You’re right. When I debugged this, I didn’t notice that the .collapse class gets removed from the target elements for the duration of the transition.

What seems to happen is this (quoting collapse.js, ll. 183–191)

    this._element.classList.remove(CLASS_NAME_COLLAPSE, CLASS_NAME_SHOW)

    for (const trigger of this._triggerArray) {
      const element = SelectorEngine.getElementFromSelector(trigger)

      if (element && !this._isShown(element)) {
        this._addAriaAndCollapsedClass([trigger], false)
      }
    }

So now I know how to work around my specific issue, but I still think it’s odd to skip _addAriaAndCollapsedClass when there are no elements to collapse. As I said earlier, the element did get (un)collapsed, even though there’s no visible change in the UI.

First the .collapse class gets removed on the target element. This causes the selectorEngine to not find the element anymore (because the data-bs-target attribute specifies elements with the collapse class). Therefore element is null in this snippet, which causes the _addAriaAndCollapsedClass invocation to be skipped.

@julien-deramond
Copy link
Member

I'm glad the workaround was helpful! I'll keep this issue open so we can explore it further. Thanks for your detailed response 🙏.

@github-project-automation github-project-automation bot moved this to To do in v5.3.4 Sep 18, 2024
@julien-deramond julien-deramond moved this from To do to To analyze in v5.3.4 Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: To analyze
Development

No branches or pull requests

2 participants