Problem/Motivation

The SVG files live in standalone files now, but they previously were embedded in page template files.

They still have some <title> and <desc> elements inside them, which should be removed because:

  • They are not guaranteed to be appropriate for the situation the are presented in.
  • It's better for text alternatives to be specified by the templates and/or content that includes them them, according to context.
  • They are English phrases which are not translatable

Proposed resolution

  • Remove the <title> and <desc> elements form the SVG files.
  • Also remove any aria-labelledby and.or aria-describedby attributes which refer to these by ID reference.
  • Preserve other attirbutes that are part of SVG and not ARIA.

Example...
Before:

  <svg aria-labelledby="menu-toggle-title menu-toggle-desc">
       <title id="menu-toggle-title">Menu toggle icon</title>
       <desc id="menu-toggle-desc">Hamburger icon for menu toggle.</desc>
       // ... the graphic parts
  </svg>

After:

  <svg>
        // ... the graphic parts
  </svg>

Remaining tasks

Examine each SVG file for these issues
TODO: make a list of SVG files, cross them off when done.

User interface changes

none.

API changes

none.

Data model changes

none.

Original report

Follow-up from earlier issue in https://github.com/lauriii/umami/issues/138

CommentFileSizeAuthor
#25 remove-unneccessary-from-svg-2942258-25.patch87.25 KBpawandubey
#19 interdiff_16-17.txt72.12 KBpawandubey
#17 remove-unneccessary-from-svg-2942258-17.patch87.29 KBpawandubey
#16 remove-unneccessary-from-svg-2942258-16.patch15.79 KBtim.cox
#3 remove-unneccessary-from-svg-2942258-3.patch15.14 KBbn_code

Comments

andrewmacpherson created an issue. See original summary.

bn_code’s picture

bn_code commented
Assigned: Unassigned » bn_code
bn_code’s picture

bn_code commented
StatusFileSize
new remove-unneccessary-from-svg-2942258-3.patch15.14 KB

List of svg files with changes:
cake.svg - title
chefs-hat.svg - title
health.svg - title
learn-to-cook.svg - title
menu-icon.svg - title, desc, attr aria-labelledby
pointer.svg - title

bn_code’s picture

bn_code commented
Status: Active » Needs review
markconroy’s picture

markconroy commented
Status: Needs review » Reviewed & tested by the community

Patch applies cleanly. I can't see any SVGs with titles or desc tags. Moving to RTBC.

Thanks for the patch.

markconroy’s picture

markconroy commented
Assigned: bn_code » Unassigned
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs review

What's odd to me is that only pointer, pointer--white and search SVGs seem to be used. Is there any reason we are keeping the rest about. And at least with the pointer SVG we're losing info because it's being used as a background image. Not sure what's the right thing to do here.

andrewmacpherson’s picture

andrewmacpherson commented

And at least with the pointer SVG we're losing info because it's being used as a background image

No we aren't - CSS background images aren't conveyed to accessibility APIs at all, so the <title> in pointer.svg is not serving as a text alternative. In this case, it's being used as a decorative image only. The pointer icon doesn't need a text alternative, accessibility APIs get told it has an AccessibleRole of "link".

SVG title elements can serve as text alternatives, but there are more are more reliable approaches, such as alt or aria-labelledby in HTML. The SVG <title> and <desc> elements that are currently there are not serving as text alternatives:

  • Some are used as CSS background images where a text alternative is not needed (search, pointer)
  • One is used as a CSS background image where a text alternative has been provided with an aria-label attribute (menu-icon).
  • Some are are used in HTML IMG elements, which have an alt attribute (logo).
  • The mobile search link needs a text alternative, addressed in #2942406: Text alternative for Umami mobile search link

Another problem with these title and desc elements is that they are not translatable, at least in standalone SVG files. They can be made translatable, but only where they are in twig files.

What's odd to me is that only pointer, pointer--white and search SVGs seem to be used.

The difficulty, knife, serves, and timer icons appear on the recipe page. The standalone SVGs are not used though, instead the recipe node template has some embedded SVGs. Maybe some template clean-up was started, but not finished?

Standalone SVG files which are currently used:

  • logo.svg - used as source for a HTML IMG, which has an alt attribute. The SVG "Umami logo" isn't a good text alternative anyway.
  • menu-icon.svg
  • pointer.svg
  • pointer--white.svg
  • search.svg

Standalone SVG files which are not used, but have equivalent copies in the recipe node template:

  • difficulty.svg
  • knife.svg
  • serves.svg
  • timer.svg

Perhaps some clean-up plan was started but not finished - should these be tidied up, removing either the standalone files or the embedded ones? In any case the SVG <title> is duplicating visible text immediately next to the icon, so isn't necessary as a text-alternative. For example we hear "Preparation icon, preparation time 10 minutes", which is (a) just too verbose, and (b) doesn't rely on the fact that it's a knife - the whisk would do just as well here.

Standalone SVGs not being used - are they still needed?

  • cake.svg
  • chef-hat.svg
  • health.svg
  • spoon.svg
  • whisk.svg
ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
commented

Some of this icons will be needed here: #2943353: Umami Theme - Quick links block homepage, and I guess depending how the block is created it will influence this issue.

andrewmacpherson’s picture

andrewmacpherson commented

@ckrina We can postpone this until we know what's happening with the quick links block, if you like

ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
commented

Maybe, instead of blocking this issue, they could be added in the patch for #2943353: Umami Theme - Quick links block homepage and deleted here.

ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
commented

I've just added a patch to #2943353: Umami Theme - Quick links block homepage with @bn_code changes on the 4 SVGs needed so they can be safely removed (or not) here.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

eli-t’s picture

eli-t
he/him
Primary language English
Location Manchester
commented

All the svgs not currently used but intended to be implemented in future work are included on the issue #2943353

Therefore we can safely delete them from this issue

cake.svg
chef-hat.svg
health.svg
spoon.svg
whisk.svg

These svgs are now used as files and are no longer embedded in the recipe node template so should not be removed as hypothesized in

difficulty.svg
knife.svg
serves.svg
timer.svg

The embedded title and desc elements and referring aria references to them should be deleted as detailed originally by @andrewmacpherson in the issue summary. Some of this may be in the latest patch.

andrewmacpherson’s picture

andrewmacpherson commented

The "umami logo" in logo.svg is another unnecessary title element. The alt text for that comes from the branding block template, and is translatable there.

tim.cox’s picture

tim.cox commented
StatusFileSize
new remove-unneccessary-from-svg-2942258-16.patch15.79 KB

I have modified the patch in #3 to also remove the title element from logo.svg as requested by #15.

pawandubey’s picture

pawandubey commented
StatusFileSize
new remove-unneccessary-from-svg-2942258-17.patch87.29 KB

I have tested the patch#16 and its working fine.

Just wanted to highlight few things:

1. Do we have to minify the following SVG files like other SVG files?

  • search.svg
  • difficulty.svg
  • knife.svg
  • serves.svg
  • spoon.svg
  • timer.svg
  • whisk.svg
  • logo.svg

2. In the patch#16, no changes applied to search.svg file where there is still <title> tag available which is used as a background-image for <input> tag. Following is the HTML code of search where the image is applied in background:
<input title="Enter the terms you wish to search for." placeholder="Search by keyword, ingredient, dish" data-drupal-selector="edit-keys" type="search" id="edit-keys" name="keys" value="" size="15" maxlength="128" class="form-search">

I have re-rolled the patch with the both above changes. Please let me know if any further changes are require in this or if this change not require then patch#16 is verified any you can share the feedback accordingly.

andrewmacpherson’s picture

andrewmacpherson commented

@pawandubey - thanks for the updated patch. Can you provide an interdiff to show what's changed between #16 and #17?

pawandubey’s picture

pawandubey commented
StatusFileSize
new interdiff_16-17.txt72.12 KB

@andrewmacpherson

Please find the attached interdiff file and let me know your thought on the patch.

volkswagenchick’s picture

volkswagenchick
she/her
Primary language English
Location San Francisco Bay Area
commented
Issue tags: +fldc19, +sfdug, +dcnj19

Tagging for upcoming contribution days.

volkswagenchick’s picture

volkswagenchick
she/her
Primary language English
Location San Francisco Bay Area
commented
Issue tags: +midcamp2019

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

gobinathm’s picture

gobinathm
he/him
Primary language English
Location Milton, Ontario
commented
Issue tags: +Seattle2019

Tagging for the SeattleCon's Mentored Contribution Sprint

bCounts’s picture

bCounts commented

I'm In the Druaplcon Novice room and will look into this.

pawandubey’s picture

pawandubey commented
StatusFileSize
new remove-unneccessary-from-svg-2942258-25.patch87.25 KB

I have re-rolled the patch#17 as per the latest changes in 8.8.x version.

Currently, interdiff is not possible for it, please review the same and let me know your feedback.

tatarbj’s picture

tatarbj
Primary language Hungarian
Location Brussels
commented
Issue tags: +DrupalCampBelarus2019
bCounts’s picture

bCounts commented

I reviewed the interdiff_16-17.txt and the latest patch. I found the following changes:

Minified like other SVG files:
learn-to-cook.svg
serves.svg
spoon.svg
timer.svg
whisk.svg
knife.svg

Title removed and minified:
cake.svg
chefs-hat.svg
health.svg
pointer.svg
search.svg
logo.svg

Minified and additional content removed as detailed:
-difficulty.svg - Removed '

' -menu-icon.svg - Removed 'aria-labelledby="menu-toggle-title menu-toggle-desc">Menu toggle iconHamburger icon for menu toggle.
bCounts’s picture

bCounts commented

I reviewed the interdiff_16-17.txt and the latest patch. I found the following changes:

Minified like other SVG files:
learn-to-cook.svg
serves.svg
spoon.svg
timer.svg
whisk.svg
knife.svg

Title removed and minified:
cake.svg
chefs-hat.svg
health.svg
pointer.svg
search.svg
logo.svg

Minified and additional content removed as detailed:
-difficulty.svg - Removed '

' -menu-icon.svg - Removed 'aria-labelledby="menu-toggle-title menu-toggle-desc">Menu toggle iconHamburger icon for menu toggle.
aaronmchale’s picture

aaronmchale
Primary language English
Location Edinburgh, Scotland
commented
Issue tags: +DCScot19
sushma9’s picture

sushma9 commented

I have picked this up.
+DCScot19

sushma9’s picture

sushma9 commented

I have reviewed the latest patch in #25.
The title is removed for the below svg's:
cake.svg
chefs-hat.svg
health.svg
learn-to-cook.svg
pointer.svg
logo.svg
menu-icon.svg

Below svg's are minified:
difficulty.svg
knife.svg
search.svg
serves.svg
spoon.svg
timer.svg
whisk.svg

sushma9’s picture

sushma9 commented
Status: Needs review » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 25: remove-unneccessary-from-svg-2942258-25.patch, failed testing. View results

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs work » Reviewed & tested by the community

vimeo was down - #3059022: If Vimeo is down our tests break

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 25: remove-unneccessary-from-svg-2942258-25.patch, failed testing. View results

pawandubey’s picture

pawandubey commented
Status: Needs work » Reviewed & tested by the community
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed db82a8e and pushed to 8.8.x. Thanks!

  • catch committed db82a8e on 8.8.x 
    Issue #2942258 by pawandubey, bn_code, Tim Cox, andrewmacpherson,...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.