Content prefix #6
Conversation
| @@ -39,7 +45,7 @@ Accordion.prototype.findTriggers = function() { | |||
| }; | |||
|
|
|||
| Accordion.prototype.setAria = function(trigger, index) { | |||
| var contentID = 'content-' + index; | |||
| var contentID = this.opts.contentPrefix + '-' + 'content-' + index; | |||
There was a problem hiding this comment.
Do we want the accordion to set IDs for all the trigger elements? What about letting users set button IDs in their markup instead? Then the setAria method can check for an ID, and if it's present, update ARIA attributes. That way, we wouldn't be changing IDs of existing elements, which I think would be a good thing.
There was a problem hiding this comment.
Ok, I update this. I added a check to see if there's already IDs set, and if so, use those. I think it's convenient to not have to worry about setting them and let the library do it programmatically (as in the case of a really long list, like the glossary). But good call on not changing things if they already exist.
- Removing org scope from package name - Fixing failing tests - Check for existing ID on content divs and use that if it exists h/t @jmcarp
|
Thanks for the review @jmcarp . Made the change and updated the readme. |
|
Looks fine to me @noahmanger. I'd merge, but I don't have permission. |
|
Thanks. Merged. |
Adding the ability to set custom prefixes for the IDs of the content divs. This allows you to have more than one accordion instance on a page.