NikolayRys / Likely

The social sharing buttons that aren’t shabby
ISC License
394 stars 61 forks source link

Code in the example of accessibility settings do not make Likely accessible #161

Closed baradusov closed 1 year ago

baradusov commented 4 years ago

Codepen example with all changes.

Adding tabindex, role and aria-label to current recommended layout with divs

<div class="likely">
    <div class="facebook" tabindex="0" role="link" aria-label="Share on Facebook">Share</div>
    <div class="twitter" tabindex="0" role="link" aria-label="Tweet on Twitter">Tweet</div>
    <div class="vkontakte" tabindex="0" role="link" aria-label="Share on Vkontakte">Share</div>
    <div class="pinterest" tabindex="0" role="link" aria-label="Pin on Pinterest">Pin</div>
    <div class="odnoklassniki" tabindex="0" role="link" aria-label="Like on Odnoklassniki">Like</div>
    <div class="telegram" tabindex="0" role="link" aria-label="Send on Telegram">Send</div>
    <div class="linkedin" tabindex="0" role="link" aria-label="Share on LinkedIn">Share</div>
    <div class="whatsapp" tabindex="0" role="link" aria-label="Send on WhatsApp">Share</div>
</div>

only making buttons accessible for navigating with Tab, but they ignore Space or Enter pressing, so modal window doesn't open.

Because people mostly copy the default layout I suggest to change main recommended layout in "Setup" section to

<div class="likely">
    <button class="facebook" aria-label="Share on Facebook">Share</button>
    <button class="twitter"  aria-label="Tweet on Twitter">Tweet</button>
    <button class="vkontakte" aria-label="Share on Vkontakte">Share</button>
    <button class="pinterest" aria-label="Pin on Pinterest">Pin</button>
    <button class="odnoklassniki" aria-label="Like on Odnoklassniki">Like</button>
    <button class="telegram" aria-label="Send on Telegram">Send</button>
    <button class="linkedin" aria-label="Share on LinkedIn">Share</button>
    <button class="whatsapp" aria-label="Send on WhatsApp">Share</button>
</div>

or at least make changes in section about accessibility settings.

Adding button will break styles, so will have to change index.styl a little: from

.likely {
  &__widget {
    display: inline-block;
    position: relative;
    white-space: nowrap;
  }
  ...
}

to

.likely {
  &__widget {
    display: inline-block;
    border: none; // remove button's border
    position: relative;
    white-space: nowrap;
    appearance: none; // reset platform specific styles
    -webkit-appearance: none;
  }
  ...
}

I think this little changes make LIkely a little bit better.

We can also remove browser's default outline and make fancier one. Remove outline:

&__widget {
    display: inline-block;
    border: none;
    position: relative;
    white-space: nowrap;
    appearance: none; // reset platform specific styles
    -webkit-appearance: none;
    outline: 0; // remove outline
 }

Change colorize function:

&:focus {
  box-shadow: 0 0 0 2px alpha(color,0.7); // adding custom outline that matches brand color
}

Codepen example with all changes. If it is acceptable changes I could make PR.

pepelsbey commented 4 years ago

Thank you for raising this question I was thinking to ask for a while. But why don’t we go further? I believe all the buttons could be replaced with links:

<div class="likely">
    <a href="" class="facebook" aria-label="Share on Facebook">Share</a>
    <a href="" class="twitter"  aria-label="Tweet on Twitter">Tweet</a>
    <a href="" class="vkontakte" aria-label="Share on Vkontakte">Share</a>
    <a href="" class="pinterest" aria-label="Pin on Pinterest">Pin</a>
    <a href="" class="odnoklassniki" aria-label="Like on Odnoklassniki">Like</a>
    <a href="" class="telegram" aria-label="Send on Telegram">Send</a>
    <a href="" class="linkedin" aria-label="Share on LinkedIn">Share</a>
    <a href="" class="whatsapp" aria-label="Send on WhatsApp">Share</a>
</div>
pepelsbey commented 4 years ago

Why links? Because that’s what really happening when you click those things. It’s just a link opening in another tab. Yes, it needs to be opened in a certain way, maybe as a pop-up window, maybe with some extra GET-parameters you need to get from the current page, but it’s still a link. And it could look and even function almost the same even without JS, while it’s loading or in case when it’s blocked or broken.

baradusov commented 4 years ago

@pepelsbey Thanks for the additional suggestions! I doubted <button> is the right choice, but decided to use it, because my thought was that it is a pop-up window first. But after your clarification I agree that <a> fit in better here.

pepelsbey commented 4 years ago

Actually I’ve never seen Likely’s pop-ups before I wrote this issue because my browser on macOS is always in full screen split mode, so to me it’s just a target=_blank thing.

NikolayRys commented 4 years ago

Thank you for the suggestion, guys, we'll consider it for the upcoming version.

pepelsbey commented 4 years ago

@NikolayRys, thanks! Please let me know once you have some code to test or even the idea how to approach this. I’ll be glad to help!

NikolayRys commented 1 year ago

Switched to <a> tags in 3.0, please update your version: https://github.com/NikolayRys/Likely/releases/tag/v3.0.0