Cleanup SVG icon markup

@Balu Ertl I think 8px for the icon looks very small.

Just a heads up, I want to add a patch later, i'm reviewing this module in our project as we speak.

We shouldn't use pixels at all, it should be set in EM's, so it dynamically will take the appropriate size.

The viewbox in the latest patch is also incorrect (the icon itself is a perfect square), and we should add the width/height attributes to the SVG itself, that way, you only have to define one dimension and the other attribute can be "auto" (height, .5em; width: auto;)

Also some other improvements, but these could be well out of scope of this ticket:
could be to clean up the CSS a bit more, use "currentColor" instead of a hardcoded fill/stroke color, and set them all on the class "ext" themselves, instead of introducing extra specificity with the `.ext path {stroke}` styles.

Lastly the padding inside the SVG is also strange and should be removed or at least replaced by margin, and should be dependent on the setting that defines if the icon is in front or after the link.

I'm reviewing this module now, and I'll try to get a patch ready as soon as possible, probably tomorrow.

When trying to fix the SVG viewbox and cleanup, i've noticed a lot of legacy code, and inability to really fix some stylingproblems. This might be a bit out of scope of the ticket, but I think this will improve the theming for everybody, and usability of the module, and bring it into 2020 with modern styling standards (For SVG). And also improve the accessibility.

JS

• Adjusted the 2 SVG's (mail/link) to have a default size (width/height) and viewport (14px high), This fixes having gigantic icons when CSS fails loading.
• Replaced `aria-label` with `aria-labelledby` that points to the unique ID of the title of the SVG. This makes it able for (google) translate to also translate those labels.
• Add mail/link classes for SVG that are now available in the admin form so people can set their own classes. (just like font awesome)
• Add "placement" classes, so we know if the icon is prepended or appended
• Add "fill=currentcolor" so the icons gets the same color is the link itself, and not a hardcoded unaccessible gray.

Admin settings

• NEW: Added a "generic class" setting for both SVG and font-awesome, set to "extlink" and replaced the hardcoded class in font-awesome with the adjustable one.
• NEW: just like font-awesome, you can now set classes for the SVG icons
• Improved the Admin form, so you can only see the relevant checkboxes if you are using icons:
  • "icon for image links" only shows if "use icons for external links" is checked
  • "prepend/append" only shows if mail OR link icons are checked
  • "use font-awesome" only shows if mail OR link icons are checked
  • And you can set Classes for SVG or font-awesome, depending on what you have chosen to use.

CSS:
Complete rewrite, because a lot was really old fashioned.

• Used the generic class to add "placement" classes, so we can prepend/append spacing between the icon, and link text in a proper way.
• Remove "fill" colors, icons get the same color as the link now.
• Removed the CSS stroke because of two reasons:
  • Because the viewbox is just as big as the icon, the stroke gets "cut off" because SVG have a default "overflow: hidden" in browsers
  • IMHO, the stroke made the icons really unreadable and not "crisp", if we want a "stroke" we should just replace it with better icons.
• Removed broken and deprecated/legacy "extlink i" style (it was missing the "dot"), but the <i> element also isn't used anymore for font-awesome
• Changed the hardcoded pixels height to EM's so the icon scales with the font text.
• Set width's to "auto" so they are responsive, and you only have to change the height of the icon.
• Now SVG & font-awesome icons aren't printed.

Created an update hook, to set the newly create classes textfields.

In a nutshell
- Font-awesome should stay the same (no class changes), but the "generic" class isn't hardcoded anymore (extlink).
- SVG has better classes, and more modern styling, with better separation of concerns, and less specificity.
- Improved Admin form expierience.
- Clean SVG's with modern accessibility standards, that don't "blow up" when CSS fails.

Here's an example how the SVG icons look without any custom styling, just the module itself:

Changed to need review

IE support:
Almost identical patch as #9, except i've added an @support rule so the "width: auto" is ignored by IE11.
This prevents IE11 breaking the images (they become full width with "auto") if people want different sizes, they will have to add width & heights. But it looks good in IE11 now too out of the box:

I removed some "conditional" code in the admin form because I think the tests fail on it. And updated the schema's/install's to match. And fixed a typo, should work now.

This patch is made for the latest DEV branch.
https://www.drupal.org/files/issues/2021-02-12/3186584-cleanup-svg-icon-...

I've also made the patch to apply to the 8.x.1.5 branch, so you can use/test it straight away
https://www.drupal.org/files/issues/2021-02-12/3186584-cleanup-svg-icon-...

@bskibinski I think tests are failing and patch in #14 is not able to apply, so i will be moving this back to needs work.

Re-applied patch because it didn't apply against 1.6

Forgot some code

Because of a recent accessibility audit, we had to change the way the text "this link/mail is external" is set.
aria-labelledby on the svg tag is not supported by all screenreaders.
We went for the most reliable way, of just including a 'visually-hidden' next to the icon, that is supported by all screenreaders.
The SVG icon still has it's own title, so visual users can hover over the icon, and get a popup with the text.
But we've also added aria-hidden=true to the SVG itself, so that screenreaders won't pronounce the text twice.
We do see further improvements to accessibility for the placement of the text (it should always be after the link, despite of the position settings), but that is out of scope for this ticket, and we'll make a new one for that.

Very good, thank you!

The patch was not applying to latest dev version, so this is the new one.

Actually there was an error at extlink.install: the update was re-declared but I think it should replace the previous extlink_update_8103

I'm noob but after this my updb worked.

I consider this reviewed as I only re-applied @sander.bras work from #23 and tested that it worked.

Consider this one

@kopeboy thank you, but please provide it as MR.

This will need an issue summary update as not super clear what's being proposed. Seems to be adding new settings.

Can the IS be updated, else going to close this one out.

Current solution should land in 2.0.x first as there have been a ton of changes there. Will be very different

And the user bskibinski is already checked at the bottom

I created a PR for 2.0.x, but it still needs some work on the SVGs, I think.

Here is the PR as a patch.

Seems more like a feature request looking at the patch. Am tagging for issue summary as the proposed solution doesn't match.

So I removed the adobe stuff per the title.

Not sure why the additional stuff was needed so will leave the ticket open but think it can be closed.