Contextual quick edit toggle should be a <a role="button"> not a <a> because it tracks on/off state

Isn't this a bug in contextual.module, really? We should adhere to the standard set forth by contextual.module, so we can't have one <button> if everything else is <a> (because that'd break styling)?

@Wim Well the others go somewhere else, this one doesn't. So its more a semantic issue, introduced by edit.module ?

#2: That's true, but my argument still stands.

Can't we just add the ARIA role="button"? I was hoping to submit a patch, but I'm not finding the right space via grepping through the code.

https://developer.mozilla.org/en-US/docs/Accessibility/ARIA/ARIA_Techniq...

@mgifford: if you're still up to it, the relevant code is in two places:

edit.js:

    loadMissingEditors(function () {
      var $links = $(contextualLink.el).find('.contextual-links');
      var contextualLinkView = new Drupal.edit.ContextualLinkView($.extend({
        el: $('<li class="quick-edit"><a href=""></a></li>').prependTo($links),
        model: entityModel,
        appModel: Drupal.edit.app.model
      }, options));
      entityModel.set('contextualLinkView', contextualLinkView);
    });

and

ContextualLinkView.js (where you just need to update the selectors).

Besides that, you may need to add to edit.module's CSS or change contextual.module's CSS to be less specific.

Thanks @Wim.

I'm on a bus at the moment, so this will definitely need a review, but it should be good to get the conversation moving on this again.

Useful in light of:
http://coding.smashingmagazine.com/2013/08/20/semantic-css-with-intellig...

Okay, so we're moving everything to be <button>s. I agree with that (see #1). But it does mean all of the contextual links rendering should be updated as well to render buttons, not anchors. So: NW.

+++ b/core/modules/contextual/css/contextual.theme.css
@@ -110,8 +110,8 @@
+.contextual-region .contextual .contextual-links  button:hover {

Two spaces where there should be one :)

I'm not taking on "all of the contextual links rendering should be updated as well to render buttons, not anchors." just yet, but I think I took care of the other stuff. Well, I think it still could arguably be in edit.module since it touches core/modules/edit/js/edit.js

Plus it needed a re-roll.

How many anchors are there with the contextual links module anyways?

New patch, previous patch actually broke the quick edit functionality.
Added some css to make the button look and behave like the other contextual links.

[EDIT]
Oh yeah, and I added the aria-pressed behavior as well.

#10: Lovely, thanks for pushing this forward! Sadly, it does break styling though :(

Furthermore, I noticed that there are subtle problems now with keyboard navigation; we have pretty advanced/complex logic to deal with tabbing away from an individual contextual link, to decide when to open or close the contextual links "menu", and by using <button> instead of <a>, that breaks things.

So, switching from <a> to <a role="button"> instead of <button>. Much simpler, and achieves the same goal.

Sorry, this needs a small re-roll after #2089397: Double "Quick edit" contextual link.

Plain old reroll

Thanks! :)

Accessibility review was performed in #12.

Committed and pushed to 8.x. Thanks!