Merge pull request #595 from NoRedInk/tessa/fix-checkbox-background

Tessa/fix checkbox background
2024-12-19 03:31:32 +03:00 · 2020-09-08 14:32:07 -03:00 · 2020-09-08 14:32:07 -03:00 · 0854f2bc60
commit 0854f2bc60
parent a3d2ab6e4b fb9810354a
5 changed files with 162 additions and 96 deletions
--- a/script/format-axe-report.sh
+++ b/script/format-axe-report.sh
@ -9,28 +9,11 @@ fi

 jq -r -f script/axe-report.jq "$JSON_FILE"

-# Hey there! Did this script tell you to check out this file because the
-# expected error count went down? Well done! Just change this number to the new
-# value.
-TARGET_ERRORS=5
-
-# ideally we'd fail on any failures, but we have had a bunch build up over time!
-# So right now, we need to fail if the error count is not exactly what we
-# expect. This failure reminds us to come back and ratchet down the number of
-# failures to the correct value.
 NUM_ERRORS="$(jq '.violations | map(.nodes | length) | add' "$JSON_FILE")"
-if test "$NUM_ERRORS" -ne "$TARGET_ERRORS"; then
-    echo "got $NUM_ERRORS errors, but expected $TARGET_ERRORS."
+if [ -z "$NUM_ERRORS" ];
+then
+    echo "$NUM_ERRORS accessibility errors"
    echo
-    echo 'If it went down, hooray!'
-    echo "Check out ${0:-} and change the count to the reported value above."
-    echo
-    echo "If it went up, let's fix it instead."
-    echo "Since there are so many errors right now, a decent debugging strategy is:"
-    echo
-    echo " 1. save this output somewhere ('make axe-report > errors.new')"
-    echo " 2. undo your changes ('git stash' or 'checkout master')"
-    echo " 3. regenerate the log with 'make axe-report > errors.old'"
-    echo " 4. see what's new with 'diff -u errors.old errors.new'"
+    echo "To see these errors, run 'make axe-report > errors.new' and open 'errors.new'"
    exit 1
 fi
--- a/src/Nri/Ui/Checkbox/V5.elm
+++ b/src/Nri/Ui/Checkbox/V5.elm
@ -10,6 +10,7 @@ module Nri.Ui.Checkbox.V5 exposing
 # Patch changes

  - Use Nri.Ui.Svg.V1 rather than a custom Icon type specific to this module
+  - Make the filter ids within the svg unique (now the id depends on the checkbox identifier)


 # Changes from V5:
@ -131,13 +132,13 @@ buildCheckbox model labelView =
                    icon =
                        case model.selected of
                            Selected ->
-                                checkboxChecked
+                                checkboxChecked model.identifier

                            NotSelected ->
-                                checkboxUnchecked
+                                checkboxUnchecked model.identifier

                            PartiallySelected ->
-                                checkboxCheckedPartially
+                                checkboxCheckedPartially model.identifier
                in
                if model.disabled then
                    viewDisabledLabel model labelView icon
@ -147,10 +148,10 @@ buildCheckbox model labelView =

            Locked ->
                if model.disabled then
-                    viewDisabledLabel model labelView checkboxLockOnInside
+                    viewDisabledLabel model labelView (checkboxLockOnInside model.identifier)

                else
-                    viewEnabledLabel model labelView checkboxLockOnInside
+                    viewEnabledLabel model labelView (checkboxLockOnInside model.identifier)
        ]


@ -308,8 +309,15 @@ viewIcon styles icon =
        ]


-checkboxUnchecked : Svg
-checkboxUnchecked =
+checkboxUnchecked : String -> Svg
+checkboxUnchecked idSuffix =
+    let
+        filterId =
+            "filter-2" ++ idSuffix
+
+        filterUrl =
+            "url(#" ++ filterId ++ ")"
+    in
    Svg.svg
        [ SvgAttributes.width "27px"
        , SvgAttributes.height "27px"
@ -322,7 +330,7 @@ checkboxUnchecked =
                , SvgAttributes.width "107.4%"
                , SvgAttributes.height "107.4%"
                , SvgAttributes.filterUnits "objectBoundingBox"
-                , SvgAttributes.id "filter-2"
+                , SvgAttributes.id filterId
                ]
                [ Svg.feOffset
                    [ SvgAttributes.dx "0"
@ -362,7 +370,7 @@ checkboxUnchecked =
                , checkboxBackground
                    [ SvgAttributes.fill "black"
                    , SvgAttributes.fillOpacity "1"
-                    , SvgAttributes.filter "url(#filter-2)"
+                    , SvgAttributes.filter filterUrl
                    ]
                ]
            ]
@ -370,8 +378,15 @@ checkboxUnchecked =
        |> Nri.Ui.Svg.V1.fromHtml


-checkboxChecked : Svg
-checkboxChecked =
+checkboxChecked : String -> Svg
+checkboxChecked idSuffix =
+    let
+        filterId =
+            "filter-2" ++ idSuffix
+
+        filterUrl =
+            "url(#" ++ filterId ++ ")"
+    in
    Svg.svg
        [ SvgAttributes.width "27px"
        , SvgAttributes.height "27px"
@ -384,7 +399,7 @@ checkboxChecked =
                , SvgAttributes.width "107.4%"
                , SvgAttributes.height "107.4%"
                , SvgAttributes.filterUnits "objectBoundingBox"
-                , SvgAttributes.id "filter-2"
+                , SvgAttributes.id filterId
                ]
                [ Svg.feOffset
                    [ SvgAttributes.dx "0"
@ -425,7 +440,7 @@ checkboxChecked =
                    , checkboxBackground
                        [ SvgAttributes.fill "black"
                        , SvgAttributes.fillOpacity "1"
-                        , SvgAttributes.filter "url(#filter-2)"
+                        , SvgAttributes.filter filterUrl
                        ]
                    ]
                , Svg.g
@ -443,8 +458,15 @@ checkboxChecked =
        |> Nri.Ui.Svg.V1.fromHtml


-checkboxCheckedPartially : Svg
-checkboxCheckedPartially =
+checkboxCheckedPartially : String -> Svg
+checkboxCheckedPartially idSuffix =
+    let
+        filterId =
+            "filter-2" ++ idSuffix
+
+        filterUrl =
+            "url(#" ++ filterId ++ ")"
+    in
    Svg.svg
        [ SvgAttributes.width "27px"
        , SvgAttributes.height "27px"
@ -457,7 +479,7 @@ checkboxCheckedPartially =
                , SvgAttributes.width "107.4%"
                , SvgAttributes.height "107.4%"
                , SvgAttributes.filterUnits "objectBoundingBox"
-                , SvgAttributes.id "filter-2"
+                , SvgAttributes.id filterId
                ]
                [ Svg.feOffset
                    [ SvgAttributes.dx "0"
@ -499,7 +521,7 @@ checkboxCheckedPartially =
                    , checkboxBackground
                        [ SvgAttributes.fill "black"
                        , SvgAttributes.fillOpacity "1"
-                        , SvgAttributes.filter "url(#filter-2)"
+                        , SvgAttributes.filter filterUrl
                        ]
                    ]
                , Svg.path
@ -526,8 +548,15 @@ checkboxBackground attrs =
        []


-checkboxLockOnInside : Svg
-checkboxLockOnInside =
+checkboxLockOnInside : String -> Svg
+checkboxLockOnInside idSuffix =
+    let
+        filterId =
+            "filter-2" ++ idSuffix
+
+        filterUrl =
+            "url(#" ++ filterId ++ ")"
+    in
    Svg.svg
        [ SvgAttributes.width "27px"
        , SvgAttributes.height "27px"
@ -540,7 +569,7 @@ checkboxLockOnInside =
                , SvgAttributes.width "107.4%"
                , SvgAttributes.height "107.4%"
                , SvgAttributes.filterUnits "objectBoundingBox"
-                , SvgAttributes.id "filter-2"
+                , SvgAttributes.id filterId
                ]
                [ Svg.feOffset
                    [ SvgAttributes.dx "0"
@ -582,7 +611,7 @@ checkboxLockOnInside =
                    , checkboxBackground
                        [ SvgAttributes.fill "black"
                        , SvgAttributes.fillOpacity "1"
-                        , SvgAttributes.filter "url(#filter-2)"
+                        , SvgAttributes.filter filterUrl
                        ]
                    ]
                , Svg.g
--- a/src/Nri/Ui/RadioButton/V1.elm
+++ b/src/Nri/Ui/RadioButton/V1.elm
@ -1,6 +1,13 @@
 module Nri.Ui.RadioButton.V1 exposing (view, premium)

-{-| Changes from monolith version:
+{-|
+
+
+# Patch changes
+
+  - Make the filter ids within the svg unique (now the id depends on the radio value)
+
+Changes from monolith version:

    - uses Nri.Ui.Data.PremiumLevel rather than monolith version
    - uses Nri.Ui.Html.* rather than deprecated monolith extras
@ -224,7 +231,8 @@ internalView config =
                ]
            ]
            [ radioInputIcon
-                { isLocked = config.isLocked
+                { idSuffix = id_
+                , isLocked = config.isLocked
                , isDisabled = config.isDisabled
                , isChecked = isChecked
                }
@ -275,7 +283,8 @@ onEnterAndSpacePreventDefault msg =


 radioInputIcon :
-    { isChecked : Bool
+    { idSuffix : String
+    , isChecked : Bool
    , isLocked : Bool
    , isDisabled : Bool
    }
@ -285,16 +294,16 @@ radioInputIcon config =
        image =
            case ( config.isDisabled, config.isLocked, config.isChecked ) of
                ( _, True, _ ) ->
-                    lockedSvg
+                    lockedSvg config.idSuffix

                ( True, _, _ ) ->
-                    unselectedSvg
+                    unselectedSvg config.idSuffix

                ( _, False, True ) ->
-                    selectedSvg
+                    selectedSvg config.idSuffix

                ( _, False, False ) ->
-                    unselectedSvg
+                    unselectedSvg config.idSuffix
    in
    div
        [ classList
@ -319,12 +328,25 @@ radioInputIcon config =
        [ Nri.Ui.Svg.V1.toHtml image ]


-unselectedSvg : Svg
-unselectedSvg =
+unselectedSvg : String -> Svg
+unselectedSvg idSuffix =
+    let
+        pathId =
+            "unselected-path-1" ++ idSuffix
+
+        xlinkPathHref =
+            SvgAttributes.xlinkHref ("#" ++ pathId)
+
+        filterId =
+            "unselected-filter-1" ++ idSuffix
+
+        filterUrl =
+            "url(#" ++ filterId ++ ")"
+    in
    Svg.svg [ SvgAttributes.viewBox "0 0 27 27" ]
        [ Svg.defs []
-            [ Svg.rect [ SvgAttributes.id "unselected-path-1", SvgAttributes.x "0", SvgAttributes.y "0", SvgAttributes.width "27", SvgAttributes.height "27", SvgAttributes.rx "13.5" ] []
-            , Svg.filter [ SvgAttributes.id "unselected-filter-2", SvgAttributes.x "-3.7%", SvgAttributes.y "-3.7%", SvgAttributes.width "107.4%", SvgAttributes.height "107.4%", SvgAttributes.filterUnits "objectBoundingBox" ] [ Svg.feOffset [ SvgAttributes.dx "0", SvgAttributes.dy "2", SvgAttributes.in_ "SourceAlpha", SvgAttributes.result "shadowOffsetInner1" ] [], Svg.feComposite [ SvgAttributes.in_ "shadowOffsetInner1", SvgAttributes.in2 "SourceAlpha", SvgAttributes.operator "arithmetic", SvgAttributes.k2 "-1", SvgAttributes.k3 "1", SvgAttributes.result "shadowInnerInner1" ] [], Svg.feColorMatrix [ SvgAttributes.values "0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0.1 0", SvgAttributes.in_ "shadowInnerInner1" ] [] ]
+            [ Svg.rect [ SvgAttributes.id pathId, SvgAttributes.x "0", SvgAttributes.y "0", SvgAttributes.width "27", SvgAttributes.height "27", SvgAttributes.rx "13.5" ] []
+            , Svg.filter [ SvgAttributes.id filterId, SvgAttributes.x "-3.7%", SvgAttributes.y "-3.7%", SvgAttributes.width "107.4%", SvgAttributes.height "107.4%", SvgAttributes.filterUnits "objectBoundingBox" ] [ Svg.feOffset [ SvgAttributes.dx "0", SvgAttributes.dy "2", SvgAttributes.in_ "SourceAlpha", SvgAttributes.result "shadowOffsetInner1" ] [], Svg.feComposite [ SvgAttributes.in_ "shadowOffsetInner1", SvgAttributes.in2 "SourceAlpha", SvgAttributes.operator "arithmetic", SvgAttributes.k2 "-1", SvgAttributes.k3 "1", SvgAttributes.result "shadowInnerInner1" ] [], Svg.feColorMatrix [ SvgAttributes.values "0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0.1 0", SvgAttributes.in_ "shadowInnerInner1" ] [] ]
            ]
        , Svg.g
            [ SvgAttributes.stroke "none"
@ -337,14 +359,14 @@ unselectedSvg =
                    [ Svg.use
                        [ SvgAttributes.fill "#EBEBEB"
                        , SvgAttributes.fillRule "evenodd"
-                        , SvgAttributes.xlinkHref "#unselected-path-1"
+                        , xlinkPathHref
                        ]
                        []
                    , Svg.use
                        [ SvgAttributes.fill "black"
                        , SvgAttributes.fillOpacity "1"
-                        , SvgAttributes.filter "url(#unselected-filter-2)"
-                        , SvgAttributes.xlinkHref "#unselected-path-1"
+                        , SvgAttributes.filter filterUrl
+                        , xlinkPathHref
                        ]
                        []
                    ]
@ -354,13 +376,26 @@ unselectedSvg =
        |> Nri.Ui.Svg.V1.fromHtml


-selectedSvg : Svg
-selectedSvg =
+selectedSvg : String -> Svg
+selectedSvg idSuffix =
+    let
+        pathId =
+            "selected-path-1" ++ idSuffix
+
+        xlinkPathHref =
+            SvgAttributes.xlinkHref ("#" ++ pathId)
+
+        filterId =
+            "selected-filter-1" ++ idSuffix
+
+        filterUrl =
+            "url(#" ++ filterId ++ ")"
+    in
    Svg.svg [ SvgAttributes.viewBox "0 0 27 27" ]
        [ Svg.defs []
-            [ Svg.rect [ SvgAttributes.id "selected-path-1", SvgAttributes.x "0", SvgAttributes.y "0", SvgAttributes.width "27", SvgAttributes.height "27", SvgAttributes.rx "13.5" ] []
+            [ Svg.rect [ SvgAttributes.id pathId, SvgAttributes.x "0", SvgAttributes.y "0", SvgAttributes.width "27", SvgAttributes.height "27", SvgAttributes.rx "13.5" ] []
            , Svg.filter
-                [ SvgAttributes.id "selected-filter-2", SvgAttributes.x "-3.7%", SvgAttributes.y "-3.7%", SvgAttributes.width "107.4%", SvgAttributes.height "107.4%", SvgAttributes.filterUnits "objectBoundingBox" ]
+                [ SvgAttributes.id filterId, SvgAttributes.x "-3.7%", SvgAttributes.y "-3.7%", SvgAttributes.width "107.4%", SvgAttributes.height "107.4%", SvgAttributes.filterUnits "objectBoundingBox" ]
                [ Svg.feOffset [ SvgAttributes.dx "0", SvgAttributes.dy "2", SvgAttributes.in_ "SourceAlpha", SvgAttributes.result "shadowOffsetInner1" ] [], Svg.feComposite [ SvgAttributes.in_ "shadowOffsetInner1", SvgAttributes.in2 "SourceAlpha", SvgAttributes.operator "arithmetic", SvgAttributes.k2 "-1", SvgAttributes.k3 "1", SvgAttributes.result "shadowInnerInner1" ] [], Svg.feColorMatrix [ SvgAttributes.values "0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0.1 0", SvgAttributes.in_ "shadowInnerInner1" ] [] ]
            ]
        , Svg.g
@ -374,14 +409,14 @@ selectedSvg =
                    [ Svg.use
                        [ SvgAttributes.fill "#D4F0FF"
                        , SvgAttributes.fillRule "evenodd"
-                        , SvgAttributes.xlinkHref "#selected-path-1"
+                        , xlinkPathHref
                        ]
                        []
                    , Svg.use
                        [ SvgAttributes.fill "black"
                        , SvgAttributes.fillOpacity "1"
-                        , SvgAttributes.filter "url(#selected-filter-2)"
-                        , SvgAttributes.xlinkHref "#selected-path-1"
+                        , SvgAttributes.filter filterUrl
+                        , xlinkPathHref
                        ]
                        []
                    ]
@ -398,12 +433,25 @@ selectedSvg =
        |> Nri.Ui.Svg.V1.fromHtml


-lockedSvg : Svg
-lockedSvg =
+lockedSvg : String -> Svg
+lockedSvg idSuffix =
+    let
+        pathId =
+            "locked-path-1" ++ idSuffix
+
+        xlinkPathHref =
+            SvgAttributes.xlinkHref ("#" ++ pathId)
+
+        filterId =
+            "locked-filter-1" ++ idSuffix
+
+        filterUrl =
+            "url(#" ++ filterId ++ ")"
+    in
    Svg.svg [ SvgAttributes.viewBox "0 0 30 30" ]
        [ Svg.defs []
-            [ Svg.rect [ SvgAttributes.id "locked-path-1", SvgAttributes.x "0", SvgAttributes.y "0", SvgAttributes.width "30", SvgAttributes.height "30", SvgAttributes.rx "15" ] []
-            , Svg.filter [ SvgAttributes.id "locked-filter-2", SvgAttributes.x "-3.3%", SvgAttributes.y "-3.3%", SvgAttributes.width "106.7%", SvgAttributes.height "106.7%", SvgAttributes.filterUnits "objectBoundingBox" ] [ Svg.feOffset [ SvgAttributes.dx "0", SvgAttributes.dy "2", SvgAttributes.in_ "SourceAlpha", SvgAttributes.result "shadowOffsetInner1" ] [], Svg.feComposite [ SvgAttributes.in_ "shadowOffsetInner1", SvgAttributes.in2 "SourceAlpha", SvgAttributes.operator "arithmetic", SvgAttributes.k2 "-1", SvgAttributes.k3 "1", SvgAttributes.result "shadowInnerInner1" ] [], Svg.feColorMatrix [ SvgAttributes.values "0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0.1 0", SvgAttributes.in_ "shadowInnerInner1" ] [] ]
+            [ Svg.rect [ SvgAttributes.id pathId, SvgAttributes.x "0", SvgAttributes.y "0", SvgAttributes.width "30", SvgAttributes.height "30", SvgAttributes.rx "15" ] []
+            , Svg.filter [ SvgAttributes.id filterId, SvgAttributes.x "-3.3%", SvgAttributes.y "-3.3%", SvgAttributes.width "106.7%", SvgAttributes.height "106.7%", SvgAttributes.filterUnits "objectBoundingBox" ] [ Svg.feOffset [ SvgAttributes.dx "0", SvgAttributes.dy "2", SvgAttributes.in_ "SourceAlpha", SvgAttributes.result "shadowOffsetInner1" ] [], Svg.feComposite [ SvgAttributes.in_ "shadowOffsetInner1", SvgAttributes.in2 "SourceAlpha", SvgAttributes.operator "arithmetic", SvgAttributes.k2 "-1", SvgAttributes.k3 "1", SvgAttributes.result "shadowInnerInner1" ] [], Svg.feColorMatrix [ SvgAttributes.values "0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0.1 0", SvgAttributes.in_ "shadowInnerInner1" ] [] ]
            ]
        , Svg.g
            [ SvgAttributes.stroke "none"
@ -415,13 +463,13 @@ lockedSvg =
                [ Svg.use
                    [ SvgAttributes.fill "#EBEBEB"
                    , SvgAttributes.fillRule "evenodd"
-                    , SvgAttributes.xlinkHref "#locked-path-1"
+                    , xlinkPathHref
                    ]
                    []
                , Svg.use
                    [ SvgAttributes.fill "black"
                    , SvgAttributes.fillOpacity "1"
-                    , SvgAttributes.filter "url(#locked-filter-2)"
+                    , SvgAttributes.filter filterUrl
                    , SvgAttributes.xlinkHref "#locked-path-1"
                    ]
                    []
--- a/styleguide-app/Examples/Checkbox.elm
+++ b/styleguide-app/Examples/Checkbox.elm
@ -175,45 +175,51 @@ viewMultilineCheckboxes =

 viewPremiumCheckboxes : State -> Html Msg
 viewPremiumCheckboxes state =
-    let
-        safeId =
-            String.replace " " "-"
-
-        checkbox config =
-            PremiumCheckbox.view
-                { label = config.label
-                , id = "premium-checkbox-" ++ safeId config.label
-                , selected =
-                    if Set.member config.label state.isChecked then
-                        Checkbox.Selected
-
-                    else
-                        Checkbox.NotSelected
-                , disabled = config.disabled
-                , isLocked = config.isLocked
-                , isPremium = config.isPremium
-                , onChange = ToggleCheck config.label
-                , onLockedClick = NoOp
-                }
-    in
    Html.div []
-        [ checkbox
-            { label = "Identify Adjectives 2 (Premium)"
+        [ PremiumCheckbox.view
+            { label = "Identify Adjectives 1 (Premium)"
+            , id = "premium-checkbox-identify-adjectives-premium"
+            , selected =
+                if Set.member "premium-1" state.isChecked then
+                    Checkbox.Selected
+
+                else
+                    Checkbox.NotSelected
            , disabled = False
            , isLocked = False
            , isPremium = True
+            , onChange = ToggleCheck "premium-1"
+            , onLockedClick = NoOp
            }
-        , checkbox
+        , PremiumCheckbox.view
            { label = "Identify Adjectives 2 (Free)"
+            , id = "premium-checkbox-identify-adjectives-free"
+            , selected =
+                if Set.member "premium-2" state.isChecked then
+                    Checkbox.Selected
+
+                else
+                    Checkbox.NotSelected
            , disabled = False
            , isLocked = False
            , isPremium = False
+            , onChange = ToggleCheck "premium-2"
+            , onLockedClick = NoOp
            }
-        , checkbox
-            { label = "Revising Wordy Phrases 2 (Premium, Disabled)"
+        , PremiumCheckbox.view
+            { label = "Revising Wordy Phrases 3 (Premium, Disabled)"
+            , id = "premium-checkbox-premium-disabled"
+            , selected =
+                if Set.member "premium-3" state.isChecked then
+                    Checkbox.Selected
+
+                else
+                    Checkbox.NotSelected
            , disabled = True
            , isLocked = True
            , isPremium = True
+            , onChange = ToggleCheck "premium-3"
+            , onLockedClick = NoOp
            }
        ]

--- a/styleguide-app/Examples/RadioButton.elm
+++ b/styleguide-app/Examples/RadioButton.elm
@ -115,7 +115,7 @@ viewInvisibleLabel state =
            , selectedValue = state.selectedValue
            , onSelect = Select
            , noOpMsg = NoOp
-            , valueToString = identity
+            , valueToString = \_ -> "i-m-a-secret-but-not-to-screen-readers"
            }
        ]