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-flush and unique id #61

Closed
wants to merge 1 commit into from
Closed

accordion-flush and unique id #61

wants to merge 1 commit into from

Conversation

JimiRecard
Copy link
Contributor

This adds the possibility for accordion-flush (see https://getbootstrap.com/docs/5.0/components/accordion/#flush) and fixes the unique id for accordions.

The unique id is important for the behavior of the panels. If the id is not unique, we cannot set a proper data-bs-parent and get a JS error

This adds the possibility for accordion-flush (see https://getbootstrap.com/docs/5.0/components/accordion/#flush) and fixes the unique id for accordions.

The unique id is important for the behavior of the panels. If the id is not unique, we cannot set a proper data-bs-parent and get a JS error
@smithdc1
Copy link
Member

Same comments for this PR.

I thought I had the accordions working? 🤔 I'll take a closer look when the tests are updated as that'll make it much easier to check 🙂.

@JimiRecard
Copy link
Contributor Author

JimiRecard commented Jun 11, 2021

It's defintly working at first glance, but the JS error is there, and the panel needs a double click to work.

I could use some help with the test. The issue is that the unique id generated contains a random number. Got any idea how I should address that on the expected HTML?

EDIT

Nevermind, I think I got it. If we add random.seed(0) we can fix the output. I'll try to fix the tests

EDIT 2

Also, I think the tests won't work if I try the PRs one at a time. I'll try to make a single PR and close the two I've opened.

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.

2 participants