Redmine

Daniel Ritz wrote:

Replaces all uses of image_tag() to render icons with icon-* definitions. This makes is possible to replace all these icons with Font-based icons, e.g. FontAwesome.

Could you explain more details?

Toshi MARUYAMA wrote:

Daniel Ritz wrote:

Replaces all uses of image_tag() to render icons with icon-* definitions. This makes is possible to replace all these icons with Font-based icons, e.g. FontAwesome.

Could you explain more details?

Currently, in most places where an icon is shown, it's done with CSS classes, e.g. setting class="icon icon-edit" to a <span> or <a> or whatever. This makes it very easy in themes to replace those .png based icons with font-based icons like FontAwesome. However not all icons displayed in the GUI use CSS. Some rely on simple <img> tags, mostly added with the helper function image_tag().

This patch changes all these icons to use CSS classes as well instead of the <img> tags. Icons that did not have a icon-<name> CSS class defined already are added (11 I think). Where necessary, test cases are adapted as well.

Also, this introduces the CSS class icon-only". It is like icon, but does not add additional padding on the right side since no text is expected after this. This is mostly cosmetic. I did it so the look of certain pages does not change, e.g. the sort icons (top, up, down, bottom) in Administration -> Issue statuses.

With this patch, almost all icons in Redmine can be replaced with font-based icons. The only exception being the "search" icon in some input fields. They are done with CSS as background, but there's no replacement for it with font-based icons w/o an additional tag to wrap the input field. But that's for another patch :)

I agree, this would be very useful for theme developers!

Removing the link content and only relying on the icon, which is added via CSS, severely hampers accessibility. To make life easier for screen readers and their users, I think the link description should be made available in a hidden span, e.g. using a technique described in this document.

Only relying on the title attribute might not be sufficient.

Current user agents and assistive technology provide no feedback to the user when links have title attribute content available.

https://www.w3.org/TR/2013/NOTE-WCAG20-TECHS-20130905/H33.html

However, in most cases, the current icons were "bundled" with a label like this:

The label should then be enough as a description for the CSS icon attached to it, don't you think so?

You're right. If the link still contains descriptive text, everything is fine.

But if there's an "empty" link with an icon, next to a text-only link pointing to the same target, then I would say that should be changed to a single link containing the icon and the descriptive text.

Gregor Schmidt wrote:

However, in most cases, the current icons were "bundled" with a label like this:

The label should then be enough as a description for the CSS icon attached to it, don't you think so?

You're right. If the link still contains descriptive text, everything is fine.

But if there's an "empty" link with an icon, next to a text-only link pointing to the same target, then I would say that should be changed to a single link containing the icon and the descriptive text.

There should be no such thing as an empty icon link followed by a text-only link with the same target. There are links with icon/text and links with icon only. This patched changed only the latter not to use <img> tags. I'm pretty sure this is no change wrt accessibility. Your suggested hidden span is relatively easy to implement on top of what we have now. It however means checking all icon links. The new icon-only are few and easy to find, but there are others without the "icon-only" class so they need to be checked manually. I can have a look over the weekend if time permits...

Daniel Lopez: That sounds great.

Also sorry for mentioning it not in my first comment: Thanks for the effort and braveness to create such a big patch in the first place. The changes are a great improvement with respect to extensibility and page speed.

Gregor Schmidt wrote:

Removing the link content and only relying on the icon, which is added via CSS, severely hampers accessibility. To make life easier for screen readers and their users, I think the link description should be made available in a hidden span, e.g. using a technique described in this document.

Only relying on the title attribute might not be sufficient.

Current user agents and assistive technology provide no feedback to the user when links have title attribute content available.

https://www.w3.org/TR/2013/NOTE-WCAG20-TECHS-20130905/H33.html

Daniel Ritz wrote:

Your suggested hidden span is relatively easy to implement on top of what we have now. It however means checking all icon links. The new icon-only are few and easy to find, but there are others without the "icon-only" class so they need to be checked manually. I can have a look over the weekend if time permits...

I implemented this in #21805. Please have a look. Maybe the discussion should move to the new patch with this one closed?