joomla / install-from-web-server

Repository holding the component powering the Install from Web server.
9 stars 31 forks source link

Two icons on some buttons #56

Closed NikitaEmberi closed 3 years ago

NikitaEmberi commented 3 years ago

Issue #30018 in joomla-cms v4 before: image after : image

drmenzelit commented 3 years ago

It doesn't make sense to let the empty <span aria-hidden="true"></span> tags here. They are only placeholders for the icons.

NikitaEmberi commented 3 years ago

ok @drmenzelit ma'am something like this would do right? image As explained here: https://github.com/joomla/joomla-cms/issues/30018#issuecomment-802104022 the solution for the problem would be different. But @infogra768 sir suggests something different https://github.com/joomla/joomla-cms/issues/30018#issuecomment-801730769. I am confused what to implement between the two suggested solutions.

NikitaEmberi commented 3 years ago

So shall I proceed with this one by removing element completely?

drmenzelit commented 3 years ago

For my understanding is more than only removing icons. We have to be sure the link is accessible. https://www.sitepoint.com/15-rules-making-accessible-links/ (Rule 8) https://codersblock.com/blog/external-links-new-tabs-and-accessibility/ I'm not sure if all necessary changes can be done in this repository or in combination with changes in Joomla core.

NikitaEmberi commented 3 years ago

For my understanding is more than only removing icons. We have to be sure the link is accessible. https://www.sitepoint.com/15-rules-making-accessible-links/ (Rule 8) https://codersblock.com/blog/external-links-new-tabs-and-accessibility/ I'm not sure if all necessary changes can be done in this repository or in combination with changes in Joomla core.

@drmenzelit Ma'am after going through the links provided, I came up with this solution:- Remove <span class="icon-download" aria-hidden="true"></span> and replace it with...

<div hidden>
  <span id="new-window-0">Opens in a new window tab</span>
</div>
<a target="_blank"  aria-describedby="new-window-0" rel="noopener noreferrer" class="transcode install btn btn-success" href="www.google.com"> Download</a>

This alerts users using screen reader that the link will open in new tab. and output remains same as shown in https://github.com/joomla/install-from-web-server/pull/56#issue-595486429. References: -https://medium.com/@svinkle/why-let-someone-know-when-a-link-opens-a-new-window-8699d20ed3b1. -Demo (when tested with screen reader chrome extension, I found that it works fine) Please do let me know if I should go ahead with this and make a PR.

drmenzelit commented 3 years ago

You can make the changes and we will test

drmenzelit commented 3 years ago

The problem has been solved in another PR https://github.com/joomla/joomla-cms/pull/32851 Thank you for your contribution @NikitaEmberi