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

[4.0] Inline Messages/Alerts #25116

Merged
merged 5 commits into from Jun 13, 2019
Merged

Conversation

brianteeman
Copy link
Contributor

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 inline messages/alerts.

Currently we only convey the importance of the text by 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.

Secondly we need to convey the importance of the text to screen readers. The text alone is not always sufficient to do that so this PR adds a sr-only text

This is based on the discussions joomla-projects/custom-elements#99

Note: The icon could have been added just with css but as we were adding the sr-only text I couldnt see any advantage

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 inline messages/alerts.

Currently we only convey the importance of the text by 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.

Secondly we need to convey the importance of the text to screen readers. The text alone is not always sufficient to do that so this PR adds a sr-only text
@ghost ghost changed the title [4.0] Inline Messages/Alerts [a11y] [4.0] Inline Messages/Alerts Jun 4, 2019
@ghost ghost added the a11y Accessibility label Jun 4, 2019
@Quy
Copy link
Contributor

Quy commented Jun 4, 2019

I have tested this item ✅ successfully on 72ed01b


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25116.

brianteeman added a commit to brianteeman/joomla-cms that referenced this pull request 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
@brianteeman
Copy link
Contributor Author

Related PR #25135

@richard67
Copy link
Member

It seems com_tags was forgotten?
Unbenannt

@richard67
Copy link
Member

And com_newsfeeds?
Unbenannt

@brianteeman
Copy link
Contributor Author

@richard67 That looks like an issue with com_patchtester not updating all the files - probably because there are so many. If you check the files changed you can see that neither of these have been forgotten.

image

image

@richard67
Copy link
Member

@brianteeman I think you are right, patchtester was a bit slow with this PR. Is of course your fault, you updated too many files 👅 (joke) . Beside this: I guess dgrammatiko 's comment in PR #25135 allso applies here, right?

@richard67
Copy link
Member

I have tested this item ✅ successfully on 72ed01b


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25116.

@brianteeman
Copy link
Contributor Author

brianteeman commented Jun 9, 2019 via email

@richard67
Copy link
Member

@brianteeman I did not mean that he is right, I only meant that what he says he would say here, too. From my point of view, replacing fontawesome with inline svg or whatever else is subject of another, future PR, so I agree with you.

@brianteeman
Copy link
Contributor Author

@richard67 the decision was made that fontawesome would be supported in the admin iirc - @wilsonge can confirm

@richard67
Copy link
Member

@brianteeman I know, and saw meanwhile that this here is admin only, while other one was site, too. So I tested this here with success. It has 2 good tests now and could be set RTC.

@ghost ghost removed the a11y Accessibility label Jun 10, 2019
@ghost
Copy link

ghost commented Jun 10, 2019

Status "Ready To Commit".

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jun 10, 2019
@roland-d roland-d merged commit 06c01af into joomla:4.0-dev Jun 13, 2019
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jun 13, 2019
@roland-d
Copy link
Contributor

Thank you

@roland-d roland-d added this to the Joomla 4.0 milestone Jun 13, 2019
@brianteeman
Copy link
Contributor Author

Thanks

@brianteeman brianteeman deleted the inline_messages branch June 13, 2019 06:56
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

5 participants