--------------------------------------------------------------------------------
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!
--------------------------------------------------------------------------------
Updated patch for r15041
--------------------------------------------------------------------------------

--------------------------------------------------------------------------------
Patch committed, thanks for this nice work Daniel.
--------------------------------------------------------------------------------
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":https://www.w3.org/TR/2013/NOTE-WCAG20-TECHS-20130905/C7.html.

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
--------------------------------------------------------------------------------
Gregor Schmidt wrote:
> 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":https://www.w3.org/TR/2013/NOTE-WCAG20-TECHS-20130905/C7.html.

Patch welcome.
--------------------------------------------------------------------------------
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":https://www.w3.org/TR/2013/NOTE-WCAG20-TECHS-20130905/C7.html.
>
> Only relying on the title attribute might not be sufficient.

For cases where the icon was missing an adjacent label, I'm completely with you.

However, in most cases, the current icons were "bundled" with a label like this:
!21256-label-example.png!

The label should then be enough as a description for the CSS icon attached to it, don't you think so?
--------------------------------------------------------------------------------
> 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: 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":https://www.w3.org/TR/2013/NOTE-WCAG20-TECHS-20130905/C7.html.
>
> 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?
--------------------------------------------------------------------------------
Daniel Ritz wrote:
> I implemented this in #21805. Please have a look. Maybe the discussion should move to the new patch with this one closed?

Agreed.
--------------------------------------------------------------------------------

--------------------------------------------------------------------------------

relates,New,21257,Use PDF icon for "Also available in PDF"
relates,Closed,31510,Fix missing closing tags in workflows/permissions.html.erb
duplicates,Closed,21245,UI/CSS refinements, part 1