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

The accordion does not work with "any" element as the documentation states. Here is a fix. #7231

Closed
wants to merge 1 commit into from
Closed

Conversation

evanlask
Copy link
Contributor

The accordion does not work with "any" element as the documentation states it requires the use of ul and li. This is a quick fix to that issue.

This “li” selector assumes the use of ul and li to create an accordion however the documentation indicates any elements “should” work.

Switching to “.accordion-item” allows for the use of any element.

however the documentation indicates any elements “should” work.

Switching to “.accordion-item” allows for the use of any element.
@zurbchris
Copy link
Contributor

Quite right you are. Simple enough fix, however, we either need to use a state class i.e. .is-accordion-item or set this as an option i.e. this.options.tabClass. In it makes more sense to use the state class, since it won't interfere with styling, but targeting the elements to apply the class too becomes an issue, since the js doesn't necessarily know what to look for.

@evanlask
Copy link
Contributor Author

evanlask commented Dec 1, 2015

Wouldn't a state class be reserved to indicate a states such as .is-active, .is-open or .is-closed?

.accordion-item os more of a "what" am I kind of class.

Would the reasoning be because .accordion-item might have styles attached to it? As far as I can tell it does not?

@zurbchris
Copy link
Contributor

Sorry for the delay in getting back to you. After talking this over with the Foundation team, it's been decided to move more to the [data-attributes] for JS and classes for styles. Which means, to keep this non-breaking, we need to leave in the check for li's but add in the option to use data-accordion-item to the markup and use that as a target. We'll be doing this for a couple other plugins as well, in the near-ish future. If you update your code to reflect that selector with a conditional statement for maintaining li support, I would love to pull it in before a patch tomorrow.

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