IE11 - <a> element is hard to click with child lexicon-icon

[SpencerWoo]

We originally added pointer-events:none to lexicon icons here 0bec132. This makes sense as lexicon-icons shouldn't have to trigger events however in IE11 the lexicon-icon "covers" the parent element preventing it from being clickable.

https://jsfiddle.net/L430qpmc/7/

EDIT :

• I realize my example isn't the best but to properly explain the issue :
  The first image link on IE11 can only be clicked by a small sliver of the image. The expected behavior is being able to click the image link a more or less wherever the red background is. The blue background "blocks" the red from being clicked -- contrast the first image with the second image.

[pat270]

Hey @SpencerWoo,

I'm actually ok with removing it, we just need to add the attribute focusable="false" to all our svgs. I think the Clay Soy Icon component does, but I don't think it's added in the icon taglib in CE/DXP.

This is a nitpick, but I'm not a fan of linking icons that way. The a tag is inline by default and doesn't grow with the content inside. If I'm linking an icon I like to change the display on the a tag to inline-block, inline-flex, or block. That way the anchor tag grows with the content and is clickable.

There is also a Clay CSS component that will do that for you, but you need to change the width, height and font-size if you want it to be something other than 32px. The font-size property will size the icon for you if you're using a Lexicon icon. See https://clayui.com/docs/css-framework/satellites/links.html

<a class="link-monospaced" style="width:50px;height:50px;font-size:50px;">
    <svg class="lexicon-icon"></svg>
</a>

[SpencerWoo]

@pat270 thank you for the quick response!

• I realize my example isn't the best but to properly explain the issue :
  The first image on IE11 can only be clicked by a small sliver. The expected behavior is being able to click the a more or less wherever the red background is. The blue background "blocks" the red from being clicked -- contrast the first image with the second image.

I'm actually ok with removing it

• Okay sweet, I'll look into adding the focusable attribute in the icon taglib on the Liferay side

[SpencerWoo]

Hi @pat270 looking at the code I think https://issues.liferay.com/browse/LPS-71422 added focusable="false" to the svgs in taglibs ATag, IconTag.

Assuming that's what we needed, are we good to remove pointer-events:none from lexicon icons? Or perhaps even just from <a> children?

a .lexicon-icon{
   pointer-events: auto
}

Let me know -- thanks!

[pat270]

I removed it from lexicon-icon should be in when we release and put a new version in DXP.

[SpencerWoo]

Awesome, thanks. Also once it's in Master I'll also want to also put it on 1.x for 7.0.x.

[pat270]

This isn't an issue in 1.x, there is no pointer-events: none declaration on .lexicon-icon in 1.x.

[SpencerWoo]

If I'm not mistaken isn't 70x using 1.0.28 https://github.com/liferay/liferay-portal-ee/blob/7.0.x/modules/apps/foundation/frontend-js/frontend-js-web/build.gradle#L19 which has https://github.com/liferay/clay/blob/v1.0.28/src/scss/lexicon-base/_icons.scss#L5? If I'm mistaken and it isn't in 70x then nvm

[pat270]

ugh, you're right. I don't know what to do about #926. Reverting that change will introduce that again. You happen to have a link to the ticket @gregory-bretall was fixing or some example of the bug?

[SpencerWoo]

https://issues.liferay.com/browse/LPS-81456 tested and removing pointer-events:none unfortunately does reintroduce that bug again -- adding a portlet and hovering to view the tooltip.

EDIT : @gregory-bretall any ideas on how we can resolve https://issues.liferay.com/browse/LPS-81456 and https://issues.liferay.com/browse/LPS-86160?

[pat270]

So the only workaround I'm able to find for removing pointer-events: none; is to remove/replace the title element from svg's added at aeac244 and in the aui:icon taglib https://github.com/liferay/liferay-portal/blame/master/util-taglib/src/com/liferay/taglib/aui/IconTag.java#L211-L213. I'm pretty sure it was added to pass some accessibility test. We might be able to replace title with aria-labelledby. We can do something like:

<svg aria-labelledby="uniqueSvgTitle">
    <desc id="uniqueSvgTitle"></desc>
</svg>

or

<svg aria-labelledby="the-svg-description"></svg>

I'm not sure if this will make it pass whatever test we are using. I'm including my references below:

https://www.w3.org/TR/SVG11/struct.html#DescriptionAndTitleElements
https://developer.paciellogroup.com/blog/2013/12/using-aria-enhance-svg-accessibility/

/cc @jbalsas @carloslancha

[SpencerWoo]

hey @pat270 thanks for this. I missed your comment but I've come to a similar conclusion and just saw it now -- I think LPS-81456 is incorrect using pointer-events: none to address the tooltip issue. And I think it should be added back to resolve the IE11 issue LPS-86160 and the discrepancy shouldn't be LPS-81456 and LPS-86160 but rather between LPS-81456 and LPS-68726.

[jbalsas]

@carloslancha, @marcoscv, could you please weigh in here? I remember we needed the <title> attribute in <svg> based on what we did on https://issues.liferay.com/browse/LPS-68726

[marcoscv-work]

the title in an SVG is needed but only for accessibility validation reasons, we don't need icons focus because they will be associated with a group text, button or title attribute (when it is a link/button).

I'm not sure if this helps to take a solution.

[pat270]

@marcoscv-work so we can't remove the title attribute from SVG?

[marcoscv-work]

@pat270 We followed this to make the decision to add a title:
https://www.w3.org/TR/SVG11/struct.html#DescriptionAndTitleElements

But currently I'm not able to get an error with Axe plugin when I remove the title attribute in an SVG, so if you really need to remove 'title' we can make a deeper look

[pat270]

To fix both issues mentioned by @SpencerWoo, we need to remove the title element from the svg.

This is another solution that seems most reasonable:

<svg aria-hidden="true" focusable="false" role="img"></svg>
<span class="sr-only">ellipsis-v</span>

from https://simplyaccessible.com/article/7-solutions-svgs/.

[SpencerWoo]

Okay so it sounds like we're good to remove pointer-events: none in Clay to fix this issue #1242 (LPS-86160).

And then we're trying to properly fix #926 (LPS-81456) without regressing LPS-68726 by replacing svg <title> with accessible alternatives. Potentially using <span class="sr-only"> instead?

---

Can we resolve this issue of pointer-events:none first/now? Or are we waiting to have the accessibility change complete

[marcoscv-work]

It is Ok for me

[pat270]

@SpencerWoo we should wait for the accessibility changes first.

[SpencerWoo]

Okay, then is #1242 (comment) <span class="sr-only"> a suitable accessibility replacement for <title>? What about <desc>?

I'm not seeing where <title> is advocated in svgs in https://www.w3.org/TR/SVG11/struct.html#DescriptionAndTitleElements or LPS-68726 but I'd appreciate figuring this out.

[blzaugg]

My $0.02.

I don't feel the bulk, indiscriminate, adding of any type of title/labelledby/etc at the SVG level is helping accessibility. Sure we may pass some test and meet the letter-of-the-law, but we aren't meeting the spirit-of-the-law. Those tests are aids, not targets.

For example, if I've got some user control for, uploading a user avatar. I may want these elements in the button.

• icon (camera icon)
• text ("Upload")

But I don't want my screen reading saying:

Button. Camera Icon. Upload.

More likely, I'd want something like:

Button. Upload User Avatar.

You can't accomplish this by labeling every SVG. This has to be done on a case-by-case basis, by adding a label to the button element, describing it in context.

Whoever implements the button is responsible to maintain accessibility in that context, not the SVG. The SVG is just an abstract visual aid. You cannot make it accessible abstractly.

[pat270]

Ok so I decided to wrap the svg icons in a span tag and use aria attributes to announce the icon names. It uses the icon name by default which is mostly unhelpful. I agree with @blzaugg that whoever is implementing the button/link/whatever should be responsible for maintaining accessibility. #1296 just provides a fallback if someone fails to do so.

The way to make links/buttons/whatever accessible is to add an aria-label attribute on the a, button, whatever element. For instances where the text inside the element is sufficient for describing the component you can set decorative to true in ClayIcon which will set the attribute aria-hidden="true" on the icon container.

[marcoscv-work]

I am not sure what were best place for this comment:
#1224 (comment)

[SpencerWoo]

Any update on this one?