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

joomla-alert [a11y] #99

Closed
brianteeman opened this issue Sep 18, 2018 · 39 comments
Closed

joomla-alert [a11y] #99

brianteeman opened this issue Sep 18, 2018 · 39 comments

Comments

@brianteeman
Copy link
Contributor

In some places we are passing an icon to the alert eg

<joomla-alert type="info">
	<span class="icon-info" aria-hidden="true"></span> <?php echo Text::_($fieldSet->description); ?>
</joomla-alert>

But in most places we don't and we rely on the colour of the alert to convey a meaning

For colour blind users this colour has no meaning and so we should always include either a title OR another visual indicator such as an icon.

To avoid rewriting joomla to pass an icon everytime we can simply do it with some css here and remove the calls to the icon (in the few places they exist) from the code.

Before

image

After

image

Example code

https://codepen.io/brianteeman/pen/zJMNGK

@brianteeman
Copy link
Contributor Author

tagging @wilsonge as I am not sure anyone is looking at this repo

@brianteeman
Copy link
Contributor Author

I can easily do a pr for this if agreed and then someone will have to do a release

@ylahav
Copy link

ylahav commented Sep 20, 2018

I will take the "challenge" and work on it. Or you (@brianteeman) want to work on it and I will learn to do the release.
BTW - The ability to put the icon should be "configurable".
For example - something like this:
<joomla-alert type="info" icon="xxx"> <span class="icon-info" aria-hidden="true"></span> <?php echo Text::_($fieldSet->description); ?> </joomla-alert>

The keyword "icon" can be as a stand alone name or with predefined names (like "info" etc.) or the name / path of an image...

@brianteeman
Copy link
Contributor Author

No you don't have to write the code or do the release all you have to do is to confirm or deny that it is an a11y improvement.

You can't do the release anyway because you don't have the access.

@brianteeman
Copy link
Contributor Author

I don't understand the "configurable" statement. It is all CSS so it is already configured and like all CSS can be overridden.

@C-Lodder
Copy link
Contributor

@brianteeman Think he means icon: true/false

@ylahav
Copy link

ylahav commented Sep 20, 2018

@brianteeman Think he means icon: true/false

Yes ... if we use custom elements I think it should be as much as possible configurable BUT keep it simple.
I don't think any editor who use it in his (her) content should deal with CSS

@ylahav
Copy link

ylahav commented Sep 20, 2018

BTW - to your first question about A11y - as a blind color it is essential. I am not sure if it is expressed in the standard BUT the standard is focusing on blind people...

@brianteeman
Copy link
Contributor Author

NO - that would be wrong as it would be creating non-accessible content. If a user wants to do that then they can create css or replace the element. I am not going to be creating code that does that

BUT the standard is focusing on blind people...

That is complete garbage

@ylahav
Copy link

ylahav commented Sep 20, 2018

I will not argue with you ... you know everything better than other - enjoy!

@brianteeman
Copy link
Contributor Author

It is really sad that as accessibility team leader you think that the accessibility standards are focussed only for the blind.

@ylahav
Copy link

ylahav commented Sep 20, 2018

That not what I am saying - BUT regarding blind-color only few remarks....
and it is not only what is in the standard BUT also what is implemented and tested. BUT Brian, please, don't start an argue - lets try to work as a team and not fight....

@brianteeman
Copy link
Contributor Author

I am not fighting - maybe your english let you down. As for the standards please see
https://www.w3.org/WAI/WCAG21/quickref/?versions=2.0#use-of-color

It is a basic principle of accessible design to make sure that colors are not your only method of conveying important information.

@ylahav
Copy link

ylahav commented Sep 20, 2018

thanks

@brianteeman
Copy link
Contributor Author

@C-Lodder do you have time to check my css and convert it to less? I am still learning less.

@C-Lodder
Copy link
Contributor

@brianteeman - Yeah sure. Ping me when you're ready

@brianteeman
Copy link
Contributor Author

From my perspective the code on the codepen is as good as I will be able to make it.

@C-Lodder
Copy link
Contributor

@brianteeman

Just below: https://github.com/joomla-projects/custom-elements/blob/master/src/scss/_variables.scss#L61

add the following:

$alert-icons: (
  success: url("data:image/svg+xml,%3Csvg width='1792' height='1792' viewBox='0 0 1792 1792' xmlns='http://www.w3.org/2000/svg'%3E%3Cpath d='M1299 813l-422 422q-19 19-45 19t-45-19l-294-294q-19-19-19-45t19-45l102-102q19-19 45-19t45 19l147 147 275-275q19-19 45-19t45 19l102 102q19 19 19 45t-19 45zm141 83q0-148-73-273t-198-198-273-73-273 73-198 198-73 273 73 273 198 198 273 73 273-73 198-198 73-273zm224 0q0 209-103 385.5t-279.5 279.5-385.5 103-385.5-103-279.5-279.5-103-385.5 103-385.5 279.5-279.5 385.5-103 385.5 103 279.5 279.5 103 385.5z'/%3E%3C/svg%3E"),
  info: url("data:image/svg+xml,%3Csvg width='1792' height='1792' viewBox='0 0 1792 1792' xmlns='http://www.w3.org/2000/svg'%3E%3Cpath d='M1216 1344v128q0 26-19 45t-45 19h-512q-26 0-45-19t-19-45v-128q0-26 19-45t45-19h64v-384h-64q-26 0-45-19t-19-45v-128q0-26 19-45t45-19h384q26 0 45 19t19 45v576h64q26 0 45 19t19 45zm-128-1152v192q0 26-19 45t-45 19h-256q-26 0-45-19t-19-45v-192q0-26 19-45t45-19h256q26 0 45 19t19 45z'/%3E%3C/svg%3E"),
  warning: url("data:image/svg+xml,%3Csvg width='1792' height='1792' viewBox='0 0 1792 1792' xmlns='http://www.w3.org/2000/svg'%3E%3Cpath d='M896 128q209 0 385.5 103t279.5 279.5 103 385.5-103 385.5-279.5 279.5-385.5 103-385.5-103-279.5-279.5-103-385.5 103-385.5 279.5-279.5 385.5-103zm128 1247v-190q0-14-9-23.5t-22-9.5h-192q-13 0-23 10t-10 23v190q0 13 10 23t23 10h192q13 0 22-9.5t9-23.5zm-2-344l18-621q0-12-10-18-10-8-24-8h-220q-14 0-24 8-10 6-10 18l17 621q0 10 10 17.5t24 7.5h185q14 0 23.5-7.5t10.5-17.5z'/%3E%3C/svg%3E"),
  danger: url("data:image/svg+xml,%3Csvg width='1792' height='1792' viewBox='0 0 1792 1792' xmlns='http://www.w3.org/2000/svg'%3E%3Cpath d='M1024 1375v-190q0-14-9.5-23.5t-22.5-9.5h-192q-13 0-22.5 9.5t-9.5 23.5v190q0 14 9.5 23.5t22.5 9.5h192q13 0 22.5-9.5t9.5-23.5zm-2-374l18-459q0-12-10-19-13-11-24-11h-220q-11 0-24 11-10 7-10 21l17 457q0 10 10 16.5t24 6.5h185q14 0 23.5-6.5t10.5-16.5zm-14-934l768 1408q35 63-2 126-17 29-46.5 46t-63.5 17h-1536q-34 0-63.5-17t-46.5-46q-37-63-2-126l768-1408q17-31 47-49t65-18 65 18 47 49z'/%3E%3C/svg%3E")
);

These are the encoded icon SVG's


Then replace the this mixin: https://github.com/joomla-projects/custom-elements/blob/master/src/scss/alert/alert.scss#L41

with this:

@mixin alert-variant($background, $border, $color) {
  color: $color;
  background-color: $background;
  border-color: $border;

  hr {
    border-top-color: darken($border, 5%);
  }

  .alert-link {
    color: darken($color, 10%);
  }

  &::before{
    content: "";
    height: 1em;
    width: 1em;
    display: inline-block;
    background-size: 100%;
	margin-right: .5em;
  }
}

Then finally in the same file at the very bottom, add the following which will loop though the array of icons:

@each $color, $value in $alert-icons {
  joomla-alert[type="#{$color}"] {
    background-image: $color;
  }
}

I haven't tested this at all, so if the loop doesn't work, then try replacing background-image: $color; with background-image: $value;

@brianteeman
Copy link
Contributor Author

it was $value ;)

Thanks for the help

@brianteeman
Copy link
Contributor Author

dam hit a js problem

We seem to have two different uses of the element

When it is a "pop-up" and when it is "inline"

When it is a "pop-up" there is a block in core.js that adds an h4 to the element eg <h4>Error</h4>

But I cannot find what is the condition in the code that triggers that.

What this means is that the css I have is great for when there is no title BUT when there is a title that css is no good. :(

image

image

any suggestions?

@C-Lodder
Copy link
Contributor

@brianteeman Can you send me the HTML markup from the browser inspector for both alerts in the screenshot above?

@C-Lodder
Copy link
Contributor

Don't worry, got it from our fork where the docs actually work.

The issue is that the <h4> tag is a block element and thus will by default be 100% width.

You have a few options here.

  1. Seeing as the core.js is hardcoding the title within a <h4> tag, you can use CSS to absolutely position the icon and add some left padding to the <h4>.

  2. Rewrite the JS in a way that is allows the user to insert their own icon (as an option), whilst providing fallbacks. This would create a span element with a class.

  3. Again, rewrite the JS but instead it creates a span tag and the class is based on the type of alert
    For example: joomla-alert-icon-$type
    You will then of course need to add some styling. Either the same as option 1, or use flexbox.

@brianteeman
Copy link
Contributor Author

Were you able to see what the condition was that decided if core.js should add the h4 tag

@zwiastunsw
Copy link

Thank you, @brianteeman , for raising this issue. Of course, you are right. Color should not be the only way to convey information. Adding icons is a good solution. Thanks to this, the visually impaired and those who do not differentiate between colors will know the meaning of the messages.
Is this solution sufficient? No, because the information will not be passed on to users of screen readers. But it can be easily solved. It is enough to add text visible only for the screen read, eg.
<span class="sr-only">Info</span>
<span class="sr-only">Success</span>, etc.

@brianteeman
Copy link
Contributor Author

Doesn't this code already do that
<joomla-alert type="warning" role="alert" class="joomla-alert--show">No Matching Results</joomla-alert>

I presumed that type=warning role=alert was sufficient as I was told this had already been approved by a11y tea

@brianteeman
Copy link
Contributor Author

If that is not correct then once I find out the condition for the current js it should be easy to make it so that there is either a visible or a sr-only class on the h4

@C-Lodder
Copy link
Contributor

C-Lodder commented Sep 21, 2018

Brian, the h4 depends on whether or not the "title" has been passed

@brianteeman
Copy link
Contributor Author

that's what I thought but I couldnt see where the title was being passed from - i thought that it
might be in layouts\joomla\system\message.php but not sure now - need coffee and a clear head.

@zwiastunsw
Copy link

I presumed that type=warning role=alert was sufficient as I was told this had already been approved by a11y tea

No, these attributes do not inform the user about the type of alert.
You have suggested improving accessibility with the help of icons. This is not necessary for accessibility, because the information conveys the text of the alert. But it is good for accessibility! And since we have an icon for visually impaired people, the text should be hidden for blind people.

@brianteeman
Copy link
Contributor Author

My mistake for believing that when I was told these custom elements had all been approved by the a11y team that it wasn't true :(

I will work out a way to do it once I get my head around the javascript. I dont want to have to update hundred of instances of the php code

@zwiastunsw
Copy link

@brianteeman - it is not necessary, but it would be good :)
And one more thing:
It is not a good idea to mark alerts with h4 header. Headers should be nested correctly. Rigid marking will cause them not to nest correctly. Moreover, if an alert appears in a new window, marking it with h4 header is completely unjustified. My suggestion: Distinguish the title of the alert in bold. It is enough.

@brianteeman
Copy link
Contributor Author

it is necessary because when there is no title the colour is still being used to convey a meaning and you should never rely on colour alone

@zwiastunsw
Copy link

Of course, you are right. However, in this case, apart from the color, we also have the content of the message. And it is the content that contains the meaning. Color is an additional information. The information is not given only in color.

@brianteeman
Copy link
Contributor Author

that depends if you are cognitively able to understand the text of the message. WCAG 2.1 is much more strict on cogntivie ability

@zwiastunsw
Copy link

Full agreement.

@brianteeman
Copy link
Contributor Author

as soon as i work out the js I will ping you again - I have taken note of the comment about the h4 as well - thanks

@brianteeman
Copy link
Contributor Author

I am closing this - the alerts need a massive rewrite as they are being used completely wrong. It's not if this code is accessible or not but it is where it is being used in the cms. A modal alert is a style it is not necessarily a role=alert - they are not the same thing at all

@brianteeman
Copy link
Contributor Author

re-opening issue

@brianteeman brianteeman reopened this Jun 1, 2019
brianteeman added a commit to brianteeman/joomla-cms that referenced this issue Jun 6, 2019
In J4 we currently have two types of alert. One uses a custom element and is a "popup" the other is a regular div or "inline". This PR is to improve the accessibility of the popup messages/alerts.

Currently we make use of color which conveys no meaning for a color blind user. Accessibility golden rule is to never rely on just one visual indicator so this PR adds an icon. This also makes the popup alerts consisten with the inline alerts updated joomla#25116

Finally the title of the popup used an H4. @zwiastunsw requested here joomla-projects/custom-elements#99 (comment) that this was not good for accessibility as the headings may not be nested correctly. So I have changed the H4 to a class=alert-heading (I have not customised the style of that class as it would be a waste of time as the admin template would change it) It can always be styled later

This is based on the discussions joomla-projects/custom-elements#99
@richard67
Copy link

@brianteeman Will you make the PR with changes suggested by C-Lodder, or shall someone else do that?

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

No branches or pull requests

5 participants