Skip to content

make accordion class selector more inclusive#1085

Merged
msecret merged 2 commits into
18f-pages-stagingfrom
fix-accordion-selection
Mar 28, 2016
Merged

make accordion class selector more inclusive#1085
msecret merged 2 commits into
18f-pages-stagingfrom
fix-accordion-selection

Conversation

@brendansudol

Copy link
Copy Markdown
Contributor

This PR fixes a bug (#1083) where the accordion was not being initialized in the JS when there was a css class preceding the usa-accordion class (i.e., div class="foo usa-accordion")

Comment thread src/js/start.js Outdated
});

$('[class^=usa-accordion]').each(function() {
$('*[class*=usa-accordion]').each(function() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason for not doing $('.usa-accordion')?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the reason i didn't do that is because there is another class to check for as well, namely usa-accordion-bordered

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I remember now.

I'm of the opinion that $('.usa-accordion,.usa-accordion-bordered') is a better solution. This is because with our CSS methodology, we could have classes like .usa-accordion-child that wouldn't represent the whole accordion, but an element of the accordion.

I don't see us creating that many different accordion types, and I think I'd be expected that if you did, you'd expect to have to add a selector for a new accordion type.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed

@msecret msecret merged commit dd1aea4 into 18f-pages-staging Mar 28, 2016
@maya maya deleted the fix-accordion-selection branch March 28, 2016 21:56
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