From 9425ab5de690d6708a4f0d6d8fee3046330577c9 Mon Sep 17 00:00:00 2001
From: Tessa Kelly <tessakkelly@gmail.com>
Date: Wed, 5 Jan 2022 16:09:12 -0800
Subject: [PATCH] adds role img and hide decorative svgs

---
 src/Nri/Ui/Svg/V1.elm           | 13 ++++++------
 styleguide-app/Examples/Svg.elm | 37 ++++++++++++++++++++-------------
 2 files changed, 30 insertions(+), 20 deletions(-)

diff --git a/src/Nri/Ui/Svg/V1.elm b/src/Nri/Ui/Svg/V1.elm
index 03dbf4a7..023dcda6 100644
--- a/src/Nri/Ui/Svg/V1.elm
+++ b/src/Nri/Ui/Svg/V1.elm
@@ -12,6 +12,7 @@ module Nri.Ui.Svg.V1 exposing
 
 -}
 
+import Accessibility.Styled.Role as Role
 import Accessibility.Styled.Widget as Widget
 import Css exposing (Color)
 import Html.Styled as Html exposing (Html)
@@ -56,6 +57,8 @@ withColor color (Svg record) =
 See [Using the aria-label attribute](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Techniques/Using_the_aria-label_attribute) for
 guidelines of when and how to use this attribute.
 
+Note that when the label is _not_ present, `aria-hidden` will be added. See <https://css-tricks.com/accessible-svg-icons/> for a quick summary on why.
+
 -}
 withLabel : String -> Svg -> Svg
 withLabel label (Svg record) =
@@ -102,11 +105,9 @@ toHtml (Svg record) =
                   else
                     Just (Attributes.css (Css.display Css.inlineBlock :: css))
                 , Maybe.map Widget.label record.label
+                    |> Maybe.withDefault (Widget.hidden True)
+                    |> Just
+                , Just Role.img
                 ]
     in
-    case attributes of
-        [] ->
-            Html.map never record.icon
-
-        _ ->
-            Html.div attributes [ Html.map never record.icon ]
+    Html.div attributes [ Html.map never record.icon ]
diff --git a/styleguide-app/Examples/Svg.elm b/styleguide-app/Examples/Svg.elm
index 7a3c9c4a..a11e07fa 100644
--- a/styleguide-app/Examples/Svg.elm
+++ b/styleguide-app/Examples/Svg.elm
@@ -104,21 +104,24 @@ viewResults state =
                 , Css.marginRight (Css.px 20)
                 ]
             ]
-            [ [ "color : Css.Color"
-              , "color ="
+            [ [ "color : Css.Color\n"
+              , "color =\n"
               , "    Css.rgb " ++ String.fromFloat red ++ " " ++ String.fromFloat green ++ " " ++ String.fromFloat blue
-              , ""
-              , ""
-              , "renderedSvg : Svg "
-              , "renderedSvg = "
-              , "   UiIcon.newspaper"
-              , "       |> Svg.withColor color"
-              , "       |> Svg.withWidth (Css.px " ++ String.fromFloat state.width ++ ")"
-              , "       |> Svg.withHeight (Css.px " ++ String.fromFloat state.height ++ ")"
-              , "       |> Svg.withLabel \"" ++ state.label ++ "\""
-              , "       |> Svg.toHtml"
+              , "\n\n\n"
+              , "renderedSvg : Svg\n"
+              , "renderedSvg =\n"
+              , "   UiIcon.newspaper\n"
+              , "       |> Svg.withColor color\n"
+              , "       |> Svg.withWidth (Css.px " ++ String.fromFloat state.width ++ ")\n"
+              , "       |> Svg.withHeight (Css.px " ++ String.fromFloat state.height ++ ")\n"
+              , if String.isEmpty state.label then
+                    ""
+
+                else
+                    "       |> Svg.withLabel \"" ++ state.label ++ "\"\n"
+              , "       |> Svg.toHtml\n"
               ]
-                |> String.join "\n"
+                |> String.join ""
                 |> Html.text
             ]
         , Html.div
@@ -131,7 +134,13 @@ viewResults state =
                 |> Svg.withColor (toCssColor state.color)
                 |> Svg.withWidth (Css.px state.width)
                 |> Svg.withHeight (Css.px state.height)
-                |> Svg.withLabel state.label
+                |> (\svg ->
+                        if String.isEmpty state.label then
+                            svg
+
+                        else
+                            Svg.withLabel state.label svg
+                   )
                 |> Svg.toHtml
             ]
         ]