Added option to use button tag as accordion toggle

[philbuchanan]

@CarlyGerard I added the option to use <button> tags as the accordion toggles. I'd appreciate if you could take a look.

I ended up re-writing the entire JS to separate the controller (the thing the user uses to trigger an accordion) from the accordion title html tag.

[CarlyGerard]

Hi @philbuchanan, semantically looks great, thank you! One thing I'm noticing is that when an accordion with a button is clicked the first time, the height of the accordion content is only set to 1px and has an overflow: hidden. Once it's closed, then the height of the accordion content is calculated correctly, and opens as it should when the button is clicked a second time. It looks like when a button is first clicked, nothing becomes visible.

At first I wondered if maybe my outdated code was doing something, but our site theme is on master so it doesn't have any of that accordion code. I'm a bit confused by why the content is hidden on first click, would appreciate your thoughts on that.

[philbuchanan]

I'm not seeing what you are describing. The content should never have overflow: hidden applied. As soon as I click the button, the content opens as expected. No need to click the button twice.

Perhaps you have some other code as part of your theme, or another plugin that is interfering?

I'm testing on fresh WP install using Twenty Nineteen theme.

[CarlyGerard]

Could be, I'll keep looking into it.

[CarlyGerard]

Okay I found it, it was human error. Once I found the mistype it was functioning as expected. Sorry for any confusion there! Mostly works now that I found my mistake.

I did notice that the aria-hidden on the accordion content doesn't toggle from true to false as expected. I took a look and I think line 170 needs to change from item.controller.next()... to item.content.attr.... It looks like item.controller is the button element when applied, and the next() function won't work since there's no element after the button when wrapped in the heading.

I'm also noticing that when opening the accordion using Enter or Space, it closes up again. Triple checked my code this time and the structure should be good, so I'm not sure why the item immediately closes on keypress after opening, but not on mouse click. Are you also noticing this, or no?

[philbuchanan]

I think I've fixed both issue.

You were right about the item.content.attr. I must have missed that one.

The open/immediately close issue was due to me doubling up on the events that fired.

[CarlyGerard]

Thanks for looking into that @philbuchanan. I was trying to figure out the open/close issue but wasn't able to figure out exactly where the issue was. I pulled down your changes, but was still noticing the open/close events still firing too quick, so I think adding a "else, return" statement to the corresponding if statements should stop the event from firing mutliple times:

if (!settings.usebuttons) { // 13 = Return, 32 = Space if ((code === 13) || (code === 32)) { $(this).click(); } } else { return; }

However, I'm finding that destroys the Esc functionality, but that's less critical IMO than making sure Enter/Space events aren't firing twice on one click.

[philbuchanan]

Whoops. Forgot to push the updated minified JS (I as working with define('SCRIPT_DEBUG', true);. That should fix it.

[CarlyGerard]

Awesome, sounds and works great to me. Thank you so much for working on making that valid and a real button. Having this plugin updated is a huge accessibility win and will be so helpful to us. I so appreciate your quick reviews and work on this.

[philbuchanan]

Version 2.4.0 of the plugin has been pushed to plugin repo.