Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(accessibility): solve WAVE problems #11

Closed
wants to merge 1 commit into from

Conversation

Kristinita
Copy link

@Kristinita Kristinita commented Oct 28, 2020

1. Summary

I fixed hes-gallery errors, that shows web accessibility tools.

2. MCVE

2.1. Data

We can check hes-gallery demo page online use WAVE — web accessibility evaluation tool.

Report.

2.2. Behavior

2.2.1. Before

Behavior before

  • 1X Missing alternative text
  • 2X Empty button
2.2.2. After

Behavior after

No “Missing alternative text” and “Empty button” errors.

I didn’t find any regressions after my changes.

3. Changes

3.1. Alternative text

I added alt attribute with value Default alternative text to img#hg-pic.

  • Missing alternative text:

    Why It Matters

    Each image must have an alt attribute. Without alternative text, the content of an image will not be available to screen reader users or when the image is unavailable.

3.2. Buttons

  • Empty button

    Why It Matters

    When navigating to a button, descriptive text must be presented to screen reader users to indicate the function of the button.

I replaced <button> tag to <a>. Possibly, <button> isn’t the best solution if it isn’t used for submissions in forms. See “When To Use The Button Element” article.

4. Environment

4.1. Minification

I minified hes-gallery.js and hes-gallery.css on these online services:

  1. UglifyES 3.3.9
  2. CSSO 4.1.0

4.2. Testing

  1. Windows 10.0.19041.508 Pro N for Workstations 64-bit EN

  2. Browsers:

    1. Firefox 82.0.1 (64-bit)
    2. Chromium 85.0.4183.102 (Official Build) (64-bit)

Thanks.

@demtario
Copy link
Owner

demtario commented Oct 28, 2020

Hi!
First of all, thanks for your contribution!

But, I'm a bit concerned about changing <button> into <a> tag. <a> tag without href attribute is considered as a placeholder, and I think shouldn't be a replacement for a <button>, which is allowed to be used outside the forms as an action button.

I agree, that empty buttons are a bad idea, but I think, this can be solved by moving icon from CSS directly into button (in html structure), and by adding aria-label and title tag with "Next" or "Previous" text in them [source]. Current solution does probably remove error, but does not solve accessibility issue.

So to summarize, you can change:

<button id='hg-prev'></button>
<button id='hg-next'></button>

into:

<button id="hg-prev" title="Previous" aria-label="Next"> <img src="iconsource" /> </button>
<button id="hg-next" title="Next" aria-label="Previous"> <img src="iconsource" /> </button>

Again, thanks for your contribution!

@Kristinita
Copy link
Author

Type: Objection 💬

1. Summary

Possibly, in our case div, not button or a is the most semantic way. What do you think about it?

2. Argumentation

<a> vs <button> vs <div> vs <input type='button"> question.

Answers:

  • First:

    There's a good article that summarizes the differences nicely.

    In shrt:

    General usage Complex designs Can be disabled
    <button> General button purpose. Used in most cases Yes Yes
    <input type="button"> Usually used inside forms No Yes
    <a> Used to navigate to pages and resources Yes No
    <div> Can be used for clickable area which is not button or link by definition Yes No
  • Second:

    Generally, I try to use the HTML tag in the most "semantic way" and useful way:

    • <a> tag: a is for "anchor", I use it when the button is a link to a content on the page or external content;
    • div tag: this is a simple container. I use it when I have to do simple buttons without any specific job but really custom style (simple animation, transition, open modal, etc). This is the jolly.
    • <button> tag: according with W3, this is the standard tag to create buttons on the page, so use it when you need an action like onClick().
    • <input type="button"> tag: is equal to the button tag, but you need it for form submission, because browser that submit for with <button> can submit different values. I use it generally on form.

Thanks.

@demtario
Copy link
Owner

demtario commented Oct 29, 2020

I would rather stay with <button> option. Element about which we discuss is an arrow to change image in the gallery, so it is a clickable one. In my opinion div is just a container (shouldn't hold any action), and in this case button should do this.

can also be focused by keyboard, which is not simply possible for divs

@Kristinita
Copy link
Author

@demtario ,

1. Remark on a secondary argument

can also be focused by keyboard, which is not simply possible for divs

Possibly, tabindex attribute should help in solving this problem.

2. Pull request status

According to the arguments presented above I think that the button element is not the best idea in this case. I will not add it.

But hes-gallery is your plugin, you can use whatever elements you want.

Thanks.

@demtario
Copy link
Owner

demtario commented Oct 29, 2020

@Kristinita Yea, I know that this could be done by tabindex, but this is not a solution. There is no point of using <div> if we can use <button>.

As you said in your arguments:

<button> tag: according with W3, this is the standard tag to create buttons on the page, so use it when you need an action like onClick().

and in

<div> | Can be used for clickable area which is not button or link by definition

Moving to next/prev button is not a area, it is a button by definition

@Kristinita
Copy link
Author

@demtario , OK, we are waiting for it to be implemented).

Thanks.

@demtario
Copy link
Owner

Ok, so I'm closing this PR and moving our conclusions to the new issue: #13

@demtario demtario closed this Oct 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants