philbuchanan / Accordion-Blocks

Gutenberg blocks for creating responsive accordion drop-downs in WordPress.
https://wordpress.org/plugins/accordion-blocks/
44 stars 18 forks source link

Consider Even More Accessible Markup Pattern? #25

Open mrwweb opened 4 years ago

mrwweb commented 4 years ago

I generally like the markup of this plugin which was one reason I chose to use it on a recent project. However, I think it could likely be even better if it used Scott O'hara's recommended markup:

<h2> 
  <button aria-expanded="false">Question goes here</button>
</h2>
<div hidden> <!-- not expanded, so hide the content! -->
  <p>My answer goes here!</p>
</div>

That would use a real button as the interactive element (less likely to have weird issues) and also let the heading keep it's native role and appear in outlines consistently.

I have no idea what's involved in making a change like this given that the block is already live, but I'd love for it to be considered!

philbuchanan commented 4 years ago

The markup changes are relatively trivial with Gutenberg Deprecated Blocks.

The bigger challenge is in dealing with styles that users have already written to override the plugin defaults. By changing the markup, I'll be breaking any styles users have put in place—they would need to make updates to their code to address this.

Ideally I'd like to avoid making breaking changes to the plugin. Happy to hear any suggestions you may have in regards to dealing with this issue.

mrwweb commented 4 years ago

Good to know about the Deprecated Blocks docs. I didn't know about those!

I hear you on the old styles and definitely can see that being an issue. (Even I would have to do that!) I could imagine rolling this out over a few releases:

  1. If needed, put out a release that sets an option marking "legacy" sites.
  2. Release new update that contains block with new markup and renames old block to add "Deprecated" or "Legacy". Ensure that old block can be converted to new block. Don't show Deprecated block version for new installs without legacy option set. Add message to Deprecated Blocks explaining advantage of updating.
  3. Provide a filter to disable Deprecated Version of block for sites that have converted to the new block type.
  4. Eventually disable support for old block if there's ever a breaking change that's too onerous to make (though that seems pretty unlikely).

No idea if this sounds like too much work, but that's roughly how I would approach. Thanks for hearing me out!

LukaszJaro commented 4 years ago

I noticed double focus on tab title when using headings, once on the heading(tabindex="0") and on "a" tag. Right now can use JS to remove the tabindex.

philbuchanan commented 4 years ago

@LukaszJaro I'm not sure what you mean by a tag. If you are using headings for your accordion titles, there is no a tag. Can you share a link that demonstrates the issue?

LukaszJaro commented 4 years ago

NVM, somehow my accordion titles were also links for my categories...FIXED. thanks.

shoelaced commented 2 years ago

I'm all for accessibility but I'd have to update either CSS or JS on probably 15 websites if this changes. What are the issues with role="button" on the heading? Would changing the markup inadvertently change non-default markup? For example I use <h3> for the heading in many cases. Would they change to <h2><button>Text</button></h2>?

If it's necessary to make the change, perhaps inheriting the heading styles over the <button> browser styles could at least minimize the styling issues?

.c-accordion__title button {
    display: block;
    font: inherit;
    text-align: inherit;
    text-decoration: inherit;
    text-shadow: inherit;
    letter-spacing: inherit;
    word-spacing: inherit;
    color: inherit;
    margin: 0;
    padding: 0;
    border: 0;
    background: transparent;
    width: 100%;
    cursor: inherit;
}