From 0189502ed3f514378f1cb67273f04a6e816e4bce Mon Sep 17 00:00:00 2001 From: Tessa Kelly Date: Mon, 16 May 2022 09:40:09 -0700 Subject: [PATCH 01/13] Indicate that disabled links are supported in the styleguide --- styleguide-app/Examples/Button.elm | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/styleguide-app/Examples/Button.elm b/styleguide-app/Examples/Button.elm index fa6098358..d6c9c702c 100644 --- a/styleguide-app/Examples/Button.elm +++ b/styleguide-app/Examples/Button.elm @@ -158,11 +158,10 @@ initDebugControls = , ( "fillContainerWidth", Button.fillContainerWidth ) ] ) + |> ControlExtra.optionalBoolListItem "disabled" ( "disabled", Button.disabled ) |> ControlExtra.optionalListItem "state (button only)" (CommonControls.choice moduleName - [ ( "enabled", Button.enabled ) - , ( "disabled", Button.disabled ) - , ( "error", Button.error ) + [ ( "error", Button.error ) , ( "unfulfilled", Button.unfulfilled ) , ( "loading", Button.loading ) , ( "success", Button.success ) From 24826749322b9f36f464c4ea6d58dc640e7c3447 Mon Sep 17 00:00:00 2001 From: Tessa Kelly Date: Mon, 16 May 2022 09:48:02 -0700 Subject: [PATCH 02/13] Pass through disabled state to ClickableAttributes.toLinkAttributes shared helper --- src/ClickableAttributes.elm | 4 +-- src/Nri/Ui/Button/V10.elm | 51 +++++++++++++++++++-------------- src/Nri/Ui/ClickableSvg/V2.elm | 6 +++- src/Nri/Ui/ClickableText/V3.elm | 6 +++- src/Nri/Ui/SideNav/V1.elm | 5 +++- src/Nri/Ui/SideNav/V2.elm | 5 +++- src/Nri/Ui/SideNav/V3.elm | 5 +++- 7 files changed, 53 insertions(+), 29 deletions(-) diff --git a/src/ClickableAttributes.elm b/src/ClickableAttributes.elm index 5d0ad1537..5b5887e8a 100644 --- a/src/ClickableAttributes.elm +++ b/src/ClickableAttributes.elm @@ -112,8 +112,8 @@ toButtonAttributes clickableAttributes = {-| -} -toLinkAttributes : (route -> String) -> ClickableAttributes route msg -> ( String, List (Attribute msg) ) -toLinkAttributes routeToString clickableAttributes = +toLinkAttributes : { routeToString : route -> String, isDisabled : Bool } -> ClickableAttributes route msg -> ( String, List (Attribute msg) ) +toLinkAttributes { routeToString } clickableAttributes = let stringUrl = case ( clickableAttributes.urlString, clickableAttributes.url ) of diff --git a/src/Nri/Ui/Button/V10.elm b/src/Nri/Ui/Button/V10.elm index 980c77d77..92f2bfcde 100644 --- a/src/Nri/Ui/Button/V10.elm +++ b/src/Nri/Ui/Button/V10.elm @@ -28,6 +28,7 @@ adding a span around the text could potentially lead to regressions. - adds `modal` helper, an alias for `large` size - adds `notMobileCss`, `mobileCss`, `quizEngineMobileCss` - adds `hideIconForMobile` and `hideIconFor` + - support 'disabled' links according to [Scott O'Hara's disabled links](https://www.scottohara.me/blog/2021/05/28/disabled-links.html) article # Changes from V9: @@ -463,6 +464,28 @@ type ButtonState | Success +isDisabled : ButtonState -> Bool +isDisabled state = + case state of + Enabled -> + False + + Disabled -> + True + + Error -> + True + + Unfulfilled -> + False + + Loading -> + True + + Success -> + True + + {-| -} enabled : Attribute msg enabled = @@ -557,32 +580,12 @@ renderButton ((ButtonOrLink config) as button_) = let buttonStyle_ = getColorPalette button_ - - isDisabled = - case config.state of - Enabled -> - False - - Disabled -> - True - - Error -> - True - - Unfulfilled -> - False - - Loading -> - True - - Success -> - True in Nri.Ui.styled Html.button (styledName "customButton") [ buttonStyles config.size config.width buttonStyle_ config.customStyles ] (ClickableAttributes.toButtonAttributes config.clickableAttributes - ++ Attributes.disabled isDisabled + ++ Attributes.disabled (isDisabled config.state) :: Attributes.type_ "button" :: config.customAttributes ) @@ -596,7 +599,11 @@ renderLink ((ButtonOrLink config) as link_) = getColorPalette link_ ( linkFunctionName, attributes ) = - ClickableAttributes.toLinkAttributes identity config.clickableAttributes + ClickableAttributes.toLinkAttributes + { routeToString = identity + , isDisabled = isDisabled config.state + } + config.clickableAttributes in Nri.Ui.styled Styled.a (styledName linkFunctionName) diff --git a/src/Nri/Ui/ClickableSvg/V2.elm b/src/Nri/Ui/ClickableSvg/V2.elm index 5dabb1653..41caaf39d 100644 --- a/src/Nri/Ui/ClickableSvg/V2.elm +++ b/src/Nri/Ui/ClickableSvg/V2.elm @@ -495,7 +495,11 @@ renderLink : ButtonOrLink msg -> Html msg renderLink ((ButtonOrLink config) as link_) = let ( linkFunctionName, extraAttrs ) = - ClickableAttributes.toLinkAttributes identity config.clickableAttributes + ClickableAttributes.toLinkAttributes + { routeToString = identity + , isDisabled = config.disabled + } + config.clickableAttributes theme = if config.disabled then diff --git a/src/Nri/Ui/ClickableText/V3.elm b/src/Nri/Ui/ClickableText/V3.elm index ba0cd74ea..9f7569eb7 100644 --- a/src/Nri/Ui/ClickableText/V3.elm +++ b/src/Nri/Ui/ClickableText/V3.elm @@ -365,7 +365,11 @@ link label_ attributes = |> List.foldl (\(Attribute attribute) l -> attribute l) defaults ( name, clickableAttributes ) = - ClickableAttributes.toLinkAttributes identity config.clickableAttributes + ClickableAttributes.toLinkAttributes + { routeToString = identity + , isDisabled = False + } + config.clickableAttributes in Nri.Ui.styled Html.a (dataDescriptor name) diff --git a/src/Nri/Ui/SideNav/V1.elm b/src/Nri/Ui/SideNav/V1.elm index 2094a6394..9c66c48dd 100644 --- a/src/Nri/Ui/SideNav/V1.elm +++ b/src/Nri/Ui/SideNav/V1.elm @@ -180,7 +180,10 @@ viewSidebarLeaf : viewSidebarLeaf config extraStyles entryConfig = let ( linkFunctionName, attributes ) = - ClickableAttributes.toLinkAttributes config.routeToString + ClickableAttributes.toLinkAttributes + { routeToString = config.routeToString + , isDisabled = False + } entryConfig.clickableAttributes in Nri.Ui.styled Html.Styled.a diff --git a/src/Nri/Ui/SideNav/V2.elm b/src/Nri/Ui/SideNav/V2.elm index 545818194..230894c4c 100644 --- a/src/Nri/Ui/SideNav/V2.elm +++ b/src/Nri/Ui/SideNav/V2.elm @@ -181,7 +181,10 @@ viewSidebarLeaf : viewSidebarLeaf config extraStyles entryConfig = let ( linkFunctionName, attributes ) = - ClickableAttributes.toLinkAttributes config.routeToString + ClickableAttributes.toLinkAttributes + { routeToString = config.routeToString + , isDisabled = False + } entryConfig.clickableAttributes in Nri.Ui.styled Html.Styled.a diff --git a/src/Nri/Ui/SideNav/V3.elm b/src/Nri/Ui/SideNav/V3.elm index 213f01f02..552153504 100644 --- a/src/Nri/Ui/SideNav/V3.elm +++ b/src/Nri/Ui/SideNav/V3.elm @@ -259,7 +259,10 @@ viewSidebarLeaf : viewSidebarLeaf config extraStyles entryConfig = let ( linkFunctionName, attributes ) = - ClickableAttributes.toLinkAttributes config.routeToString + ClickableAttributes.toLinkAttributes + { routeToString = config.routeToString + , isDisabled = False + } entryConfig.clickableAttributes in Nri.Ui.styled Html.Styled.a From a6d6f275c0d9e72d4456df7429878427caeaae1e Mon Sep 17 00:00:00 2001 From: Tessa Kelly Date: Mon, 16 May 2022 10:04:25 -0700 Subject: [PATCH 03/13] Adds some basic tests against non-disabled links --- tests/Spec/ClickableAttributes.elm | 89 ++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 tests/Spec/ClickableAttributes.elm diff --git a/tests/Spec/ClickableAttributes.elm b/tests/Spec/ClickableAttributes.elm new file mode 100644 index 000000000..983de2a3e --- /dev/null +++ b/tests/Spec/ClickableAttributes.elm @@ -0,0 +1,89 @@ +module Spec.ClickableAttributes exposing (suite) + +import ClickableAttributes +import Expect +import Html.Attributes exposing (href) +import Html.Styled exposing (a, text, toUnstyled) +import Test exposing (..) +import Test.Html.Query as Query +import Test.Html.Selector as Selector + + +suite : Test +suite = + describe "ClickableAttributes" + [ validLinkAttributes ] + + +validLinkAttributes : Test +validLinkAttributes = + let + renderTestAnchorTag ( _, attributes ) = + a attributes [ text "Test link" ] + |> toUnstyled + |> Query.fromHtml + in + describe "link attributes" + [ test "with an href" <| + \() -> + ClickableAttributes.init + |> ClickableAttributes.href "some-route" + |> ClickableAttributes.toLinkAttributes + { routeToString = identity, isDisabled = False } + |> renderTestAnchorTag + |> Expect.all + [ Query.has [ Selector.attribute (href "some-route") ] + ] + , test "with an external href" <| + \() -> + ClickableAttributes.init + |> ClickableAttributes.linkExternal "some-route" + |> ClickableAttributes.toLinkAttributes + { routeToString = identity, isDisabled = False } + |> renderTestAnchorTag + |> Expect.all + [ Query.has [ Selector.attribute (href "some-route") ] + ] + , test "with an external href that also supports tracking" <| + \() -> + ClickableAttributes.init + |> ClickableAttributes.linkExternalWithTracking + { track = "track it!", url = "some-route" } + |> ClickableAttributes.toLinkAttributes + { routeToString = identity, isDisabled = False } + |> renderTestAnchorTag + |> Expect.all + [ Query.has [ Selector.attribute (href "some-route") ] + ] + , test "with a SPA link" <| + \() -> + ClickableAttributes.init + |> ClickableAttributes.linkSpa "some-route" + |> ClickableAttributes.toLinkAttributes + { routeToString = identity, isDisabled = False } + |> renderTestAnchorTag + |> Expect.all + [ Query.has [ Selector.attribute (href "some-route") ] + ] + , test "with a link with a method" <| + \() -> + ClickableAttributes.init + |> ClickableAttributes.linkWithMethod { method = "the right way", url = "some-route" } + |> ClickableAttributes.toLinkAttributes + { routeToString = identity, isDisabled = False } + |> renderTestAnchorTag + |> Expect.all + [ Query.has [ Selector.attribute (href "some-route") ] + ] + , test "with a link with tracking" <| + \() -> + ClickableAttributes.init + |> ClickableAttributes.linkWithTracking + { track = "track it!", url = "some-route" } + |> ClickableAttributes.toLinkAttributes + { routeToString = identity, isDisabled = False } + |> renderTestAnchorTag + |> Expect.all + [ Query.has [ Selector.attribute (href "some-route") ] + ] + ] From 507915d8ddbab23a752d7ffeeb2bfb64dc4efaeb Mon Sep 17 00:00:00 2001 From: Tessa Kelly Date: Mon, 16 May 2022 10:10:30 -0700 Subject: [PATCH 04/13] Refactor to focus the tests on the pieces of info that matter --- tests/Spec/ClickableAttributes.elm | 67 +++++++++++++----------------- 1 file changed, 28 insertions(+), 39 deletions(-) diff --git a/tests/Spec/ClickableAttributes.elm b/tests/Spec/ClickableAttributes.elm index 983de2a3e..108f22d9b 100644 --- a/tests/Spec/ClickableAttributes.elm +++ b/tests/Spec/ClickableAttributes.elm @@ -1,6 +1,6 @@ module Spec.ClickableAttributes exposing (suite) -import ClickableAttributes +import ClickableAttributes exposing (ClickableAttributes) import Expect import Html.Attributes exposing (href) import Html.Styled exposing (a, text, toUnstyled) @@ -17,73 +17,62 @@ suite = validLinkAttributes : Test validLinkAttributes = - let - renderTestAnchorTag ( _, attributes ) = - a attributes [ text "Test link" ] - |> toUnstyled - |> Query.fromHtml - in describe "link attributes" [ test "with an href" <| \() -> - ClickableAttributes.init - |> ClickableAttributes.href "some-route" - |> ClickableAttributes.toLinkAttributes - { routeToString = identity, isDisabled = False } - |> renderTestAnchorTag + ClickableAttributes.href "some-route" + |> setupLinkTest |> Expect.all [ Query.has [ Selector.attribute (href "some-route") ] ] , test "with an external href" <| \() -> - ClickableAttributes.init - |> ClickableAttributes.linkExternal "some-route" - |> ClickableAttributes.toLinkAttributes - { routeToString = identity, isDisabled = False } - |> renderTestAnchorTag + ClickableAttributes.linkExternal "some-route" + |> setupLinkTest |> Expect.all [ Query.has [ Selector.attribute (href "some-route") ] ] , test "with an external href that also supports tracking" <| \() -> - ClickableAttributes.init - |> ClickableAttributes.linkExternalWithTracking - { track = "track it!", url = "some-route" } - |> ClickableAttributes.toLinkAttributes - { routeToString = identity, isDisabled = False } - |> renderTestAnchorTag + ClickableAttributes.linkExternalWithTracking { track = "track it!", url = "some-route" } + |> setupLinkTest |> Expect.all [ Query.has [ Selector.attribute (href "some-route") ] ] , test "with a SPA link" <| \() -> - ClickableAttributes.init - |> ClickableAttributes.linkSpa "some-route" - |> ClickableAttributes.toLinkAttributes - { routeToString = identity, isDisabled = False } - |> renderTestAnchorTag + ClickableAttributes.linkSpa "some-route" + |> setupLinkTest |> Expect.all [ Query.has [ Selector.attribute (href "some-route") ] ] , test "with a link with a method" <| \() -> - ClickableAttributes.init - |> ClickableAttributes.linkWithMethod { method = "the right way", url = "some-route" } - |> ClickableAttributes.toLinkAttributes - { routeToString = identity, isDisabled = False } - |> renderTestAnchorTag + ClickableAttributes.linkWithMethod { method = "the right way", url = "some-route" } + |> setupLinkTest |> Expect.all [ Query.has [ Selector.attribute (href "some-route") ] ] , test "with a link with tracking" <| \() -> - ClickableAttributes.init - |> ClickableAttributes.linkWithTracking - { track = "track it!", url = "some-route" } - |> ClickableAttributes.toLinkAttributes - { routeToString = identity, isDisabled = False } - |> renderTestAnchorTag + ClickableAttributes.linkWithTracking { track = "track it!", url = "some-route" } + |> setupLinkTest |> Expect.all [ Query.has [ Selector.attribute (href "some-route") ] ] ] + + +setupLinkTest : (ClickableAttributes String msg -> ClickableAttributes String msg) -> Query.Single msg +setupLinkTest withLink = + let + renderTestAnchorTag ( _, attributes ) = + a attributes [ text "Test link" ] + |> toUnstyled + |> Query.fromHtml + in + ClickableAttributes.init + |> withLink + |> ClickableAttributes.toLinkAttributes + { routeToString = identity, isDisabled = False } + |> renderTestAnchorTag From fcbba87765b91b5f154fbe84aec0ce04d2256df5 Mon Sep 17 00:00:00 2001 From: Tessa Kelly Date: Mon, 16 May 2022 10:14:15 -0700 Subject: [PATCH 05/13] Get tests to fail --- tests/Spec/ClickableAttributes.elm | 76 ++++++++++++++++++++++++++---- 1 file changed, 67 insertions(+), 9 deletions(-) diff --git a/tests/Spec/ClickableAttributes.elm b/tests/Spec/ClickableAttributes.elm index 108f22d9b..ba558fc62 100644 --- a/tests/Spec/ClickableAttributes.elm +++ b/tests/Spec/ClickableAttributes.elm @@ -12,11 +12,11 @@ import Test.Html.Selector as Selector suite : Test suite = describe "ClickableAttributes" - [ validLinkAttributes ] + [ linkAttributes, disabledLinkAttributes ] -validLinkAttributes : Test -validLinkAttributes = +linkAttributes : Test +linkAttributes = describe "link attributes" [ test "with an href" <| \() -> @@ -63,16 +63,74 @@ validLinkAttributes = ] +disabledLinkAttributes : Test +disabledLinkAttributes = + describe "disabled link attributes" + [ test "with an href" <| + \() -> + ClickableAttributes.href "some-route" + |> setupDisabledLinkTest + |> Expect.all + [ Query.hasNot [ Selector.attribute (href "some-route") ] + ] + , test "with an external href" <| + \() -> + ClickableAttributes.linkExternal "some-route" + |> setupDisabledLinkTest + |> Expect.all + [ Query.hasNot [ Selector.attribute (href "some-route") ] + ] + , test "with an external href that also supports tracking" <| + \() -> + ClickableAttributes.linkExternalWithTracking { track = "track it!", url = "some-route" } + |> setupDisabledLinkTest + |> Expect.all + [ Query.hasNot [ Selector.attribute (href "some-route") ] + ] + , test "with a SPA link" <| + \() -> + ClickableAttributes.linkSpa "some-route" + |> setupDisabledLinkTest + |> Expect.all + [ Query.hasNot [ Selector.attribute (href "some-route") ] + ] + , test "with a link with a method" <| + \() -> + ClickableAttributes.linkWithMethod { method = "the right way", url = "some-route" } + |> setupDisabledLinkTest + |> Expect.all + [ Query.hasNot [ Selector.attribute (href "some-route") ] + ] + , test "with a link with tracking" <| + \() -> + ClickableAttributes.linkWithTracking { track = "track it!", url = "some-route" } + |> setupDisabledLinkTest + |> Expect.all + [ Query.hasNot [ Selector.attribute (href "some-route") ] + ] + ] + + setupLinkTest : (ClickableAttributes String msg -> ClickableAttributes String msg) -> Query.Single msg setupLinkTest withLink = - let - renderTestAnchorTag ( _, attributes ) = - a attributes [ text "Test link" ] - |> toUnstyled - |> Query.fromHtml - in ClickableAttributes.init |> withLink |> ClickableAttributes.toLinkAttributes { routeToString = identity, isDisabled = False } |> renderTestAnchorTag + + +setupDisabledLinkTest : (ClickableAttributes String msg -> ClickableAttributes String msg) -> Query.Single msg +setupDisabledLinkTest withLink = + ClickableAttributes.init + |> withLink + |> ClickableAttributes.toLinkAttributes + { routeToString = identity, isDisabled = True } + |> renderTestAnchorTag + + +renderTestAnchorTag : ( a, List (Html.Styled.Attribute msg) ) -> Query.Single msg +renderTestAnchorTag ( _, attributes ) = + a attributes [ text "Test link" ] + |> toUnstyled + |> Query.fromHtml From 61db0f617fe434bb0fb22c8ab892606c9aa3c01b Mon Sep 17 00:00:00 2001 From: Tessa Kelly Date: Mon, 16 May 2022 14:03:50 -0700 Subject: [PATCH 06/13] Adds role assertions --- tests/Spec/ClickableAttributes.elm | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/Spec/ClickableAttributes.elm b/tests/Spec/ClickableAttributes.elm index ba558fc62..f3c0d6878 100644 --- a/tests/Spec/ClickableAttributes.elm +++ b/tests/Spec/ClickableAttributes.elm @@ -1,5 +1,6 @@ module Spec.ClickableAttributes exposing (suite) +import Accessibility.Role as Role import ClickableAttributes exposing (ClickableAttributes) import Expect import Html.Attributes exposing (href) @@ -24,6 +25,8 @@ linkAttributes = |> setupLinkTest |> Expect.all [ Query.has [ Selector.attribute (href "some-route") ] + , -- should not include redundant roles + Query.hasNot [ Selector.attribute Role.link ] ] , test "with an external href" <| \() -> @@ -31,6 +34,8 @@ linkAttributes = |> setupLinkTest |> Expect.all [ Query.has [ Selector.attribute (href "some-route") ] + , -- should not include redundant roles + Query.hasNot [ Selector.attribute Role.link ] ] , test "with an external href that also supports tracking" <| \() -> @@ -38,6 +43,8 @@ linkAttributes = |> setupLinkTest |> Expect.all [ Query.has [ Selector.attribute (href "some-route") ] + , -- should not include redundant roles + Query.hasNot [ Selector.attribute Role.link ] ] , test "with a SPA link" <| \() -> @@ -45,6 +52,8 @@ linkAttributes = |> setupLinkTest |> Expect.all [ Query.has [ Selector.attribute (href "some-route") ] + , -- should not include redundant roles + Query.hasNot [ Selector.attribute Role.link ] ] , test "with a link with a method" <| \() -> @@ -52,6 +61,8 @@ linkAttributes = |> setupLinkTest |> Expect.all [ Query.has [ Selector.attribute (href "some-route") ] + , -- should not include redundant roles + Query.hasNot [ Selector.attribute Role.link ] ] , test "with a link with tracking" <| \() -> @@ -59,6 +70,8 @@ linkAttributes = |> setupLinkTest |> Expect.all [ Query.has [ Selector.attribute (href "some-route") ] + , -- should not include redundant roles + Query.hasNot [ Selector.attribute Role.link ] ] ] @@ -72,6 +85,8 @@ disabledLinkAttributes = |> setupDisabledLinkTest |> Expect.all [ Query.hasNot [ Selector.attribute (href "some-route") ] + , -- should explicitly be tagged as having the link role + Query.has [ Selector.attribute Role.link ] ] , test "with an external href" <| \() -> @@ -79,6 +94,8 @@ disabledLinkAttributes = |> setupDisabledLinkTest |> Expect.all [ Query.hasNot [ Selector.attribute (href "some-route") ] + , -- should explicitly be tagged as having the link role + Query.has [ Selector.attribute Role.link ] ] , test "with an external href that also supports tracking" <| \() -> @@ -86,6 +103,8 @@ disabledLinkAttributes = |> setupDisabledLinkTest |> Expect.all [ Query.hasNot [ Selector.attribute (href "some-route") ] + , -- should explicitly be tagged as having the link role + Query.has [ Selector.attribute Role.link ] ] , test "with a SPA link" <| \() -> @@ -93,6 +112,8 @@ disabledLinkAttributes = |> setupDisabledLinkTest |> Expect.all [ Query.hasNot [ Selector.attribute (href "some-route") ] + , -- should explicitly be tagged as having the link role + Query.has [ Selector.attribute Role.link ] ] , test "with a link with a method" <| \() -> @@ -100,6 +121,8 @@ disabledLinkAttributes = |> setupDisabledLinkTest |> Expect.all [ Query.hasNot [ Selector.attribute (href "some-route") ] + , -- should explicitly be tagged as having the link role + Query.has [ Selector.attribute Role.link ] ] , test "with a link with tracking" <| \() -> @@ -107,6 +130,8 @@ disabledLinkAttributes = |> setupDisabledLinkTest |> Expect.all [ Query.hasNot [ Selector.attribute (href "some-route") ] + , -- should explicitly be tagged as having the link role + Query.has [ Selector.attribute Role.link ] ] ] From b5058f545e85a5049c107a56341d622ef52f4d68 Mon Sep 17 00:00:00 2001 From: Tessa Kelly Date: Mon, 16 May 2022 14:09:16 -0700 Subject: [PATCH 07/13] Adds expectations against the disabled status --- elm.json | 2 +- tests/Spec/ClickableAttributes.elm | 31 ++++++++++++++++++++++++------ tests/Spec/Nri/Ui/Tooltip.elm | 4 ++-- 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/elm.json b/elm.json index edb217ba4..53477a52c 100644 --- a/elm.json +++ b/elm.json @@ -98,6 +98,6 @@ "avh4/elm-program-test": "3.3.0 <= v < 4.0.0", "elm/html": "1.0.0 <= v < 2.0.0", "elm-explorations/test": "1.2.2 <= v < 2.0.0", - "tesk9/accessible-html": "4.1.0 <= v < 5.0.0" + "tesk9/accessible-html": "5.0.0 <= v < 6.0.0" } } diff --git a/tests/Spec/ClickableAttributes.elm b/tests/Spec/ClickableAttributes.elm index f3c0d6878..366f8927d 100644 --- a/tests/Spec/ClickableAttributes.elm +++ b/tests/Spec/ClickableAttributes.elm @@ -1,5 +1,6 @@ module Spec.ClickableAttributes exposing (suite) +import Accessibility.Aria as Aria import Accessibility.Role as Role import ClickableAttributes exposing (ClickableAttributes) import Expect @@ -25,8 +26,9 @@ linkAttributes = |> setupLinkTest |> Expect.all [ Query.has [ Selector.attribute (href "some-route") ] - , -- should not include redundant roles + , -- should not include a redundant role Query.hasNot [ Selector.attribute Role.link ] + , Query.hasNot [ Selector.attribute (Aria.disabled True) ] ] , test "with an external href" <| \() -> @@ -34,8 +36,9 @@ linkAttributes = |> setupLinkTest |> Expect.all [ Query.has [ Selector.attribute (href "some-route") ] - , -- should not include redundant roles + , -- should not include a redundant role Query.hasNot [ Selector.attribute Role.link ] + , Query.hasNot [ Selector.attribute (Aria.disabled True) ] ] , test "with an external href that also supports tracking" <| \() -> @@ -43,8 +46,9 @@ linkAttributes = |> setupLinkTest |> Expect.all [ Query.has [ Selector.attribute (href "some-route") ] - , -- should not include redundant roles + , -- should not include a redundant role Query.hasNot [ Selector.attribute Role.link ] + , Query.hasNot [ Selector.attribute (Aria.disabled True) ] ] , test "with a SPA link" <| \() -> @@ -52,8 +56,9 @@ linkAttributes = |> setupLinkTest |> Expect.all [ Query.has [ Selector.attribute (href "some-route") ] - , -- should not include redundant roles + , -- should not include a redundant role Query.hasNot [ Selector.attribute Role.link ] + , Query.hasNot [ Selector.attribute (Aria.disabled True) ] ] , test "with a link with a method" <| \() -> @@ -61,8 +66,9 @@ linkAttributes = |> setupLinkTest |> Expect.all [ Query.has [ Selector.attribute (href "some-route") ] - , -- should not include redundant roles + , -- should not include a redundant role Query.hasNot [ Selector.attribute Role.link ] + , Query.hasNot [ Selector.attribute (Aria.disabled True) ] ] , test "with a link with tracking" <| \() -> @@ -70,8 +76,9 @@ linkAttributes = |> setupLinkTest |> Expect.all [ Query.has [ Selector.attribute (href "some-route") ] - , -- should not include redundant roles + , -- should not include a redundant role Query.hasNot [ Selector.attribute Role.link ] + , Query.hasNot [ Selector.attribute (Aria.disabled True) ] ] ] @@ -87,6 +94,8 @@ disabledLinkAttributes = [ Query.hasNot [ Selector.attribute (href "some-route") ] , -- should explicitly be tagged as having the link role Query.has [ Selector.attribute Role.link ] + , -- should be marked as disabled for screenreader users + Query.has [ Selector.attribute (Aria.disabled True) ] ] , test "with an external href" <| \() -> @@ -96,6 +105,8 @@ disabledLinkAttributes = [ Query.hasNot [ Selector.attribute (href "some-route") ] , -- should explicitly be tagged as having the link role Query.has [ Selector.attribute Role.link ] + , -- should be marked as disabled for screenreader users + Query.has [ Selector.attribute (Aria.disabled True) ] ] , test "with an external href that also supports tracking" <| \() -> @@ -105,6 +116,8 @@ disabledLinkAttributes = [ Query.hasNot [ Selector.attribute (href "some-route") ] , -- should explicitly be tagged as having the link role Query.has [ Selector.attribute Role.link ] + , -- should be marked as disabled for screenreader users + Query.has [ Selector.attribute (Aria.disabled True) ] ] , test "with a SPA link" <| \() -> @@ -114,6 +127,8 @@ disabledLinkAttributes = [ Query.hasNot [ Selector.attribute (href "some-route") ] , -- should explicitly be tagged as having the link role Query.has [ Selector.attribute Role.link ] + , -- should be marked as disabled for screenreader users + Query.has [ Selector.attribute (Aria.disabled True) ] ] , test "with a link with a method" <| \() -> @@ -123,6 +138,8 @@ disabledLinkAttributes = [ Query.hasNot [ Selector.attribute (href "some-route") ] , -- should explicitly be tagged as having the link role Query.has [ Selector.attribute Role.link ] + , -- should be marked as disabled for screenreader users + Query.has [ Selector.attribute (Aria.disabled True) ] ] , test "with a link with tracking" <| \() -> @@ -132,6 +149,8 @@ disabledLinkAttributes = [ Query.hasNot [ Selector.attribute (href "some-route") ] , -- should explicitly be tagged as having the link role Query.has [ Selector.attribute Role.link ] + , -- should be marked as disabled for screenreader users + Query.has [ Selector.attribute (Aria.disabled True) ] ] ] diff --git a/tests/Spec/Nri/Ui/Tooltip.elm b/tests/Spec/Nri/Ui/Tooltip.elm index ffe39c449..e73a97689 100644 --- a/tests/Spec/Nri/Ui/Tooltip.elm +++ b/tests/Spec/Nri/Ui/Tooltip.elm @@ -1,6 +1,6 @@ module Spec.Nri.Ui.Tooltip exposing (spec) -import Accessibility.Widget as Widget +import Accessibility.Aria as Aria import Html.Attributes as Attributes import Html.Styled as HtmlStyled import Nri.Ui.Tooltip.V3 as Tooltip @@ -136,7 +136,7 @@ clickButtonByLabel label = ProgramTest.simulateDomEvent (Query.find [ Selector.tag "button" - , Selector.attribute (Widget.label label) + , Selector.attribute (Aria.label label) ] ) Event.click From aee4e3e0982df687abf3d41d15e2df32a703ce71 Mon Sep 17 00:00:00 2001 From: Tessa Kelly Date: Mon, 16 May 2022 14:12:37 -0700 Subject: [PATCH 08/13] Reogranize clickable attribute exports for better ease of use --- src/ClickableAttributes.elm | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/src/ClickableAttributes.elm b/src/ClickableAttributes.elm index 5b5887e8a..aeb5bbebd 100644 --- a/src/ClickableAttributes.elm +++ b/src/ClickableAttributes.elm @@ -1,18 +1,32 @@ module ClickableAttributes exposing - ( ClickableAttributes - , href - , init - , linkExternal - , linkExternalWithTracking - , linkSpa - , linkWithMethod - , linkWithTracking + ( ClickableAttributes, init , onClick , toButtonAttributes + , href, linkWithMethod, linkWithTracking + , linkSpa + , linkExternal, linkExternalWithTracking , toLinkAttributes ) -{-| -} +{-| + +@docs ClickableAttributes, init + + +# For buttons + +@docs onClick +@docs toButtonAttributes + + +# For links + +@docs href, linkWithMethod, linkWithTracking +@docs linkSpa +@docs linkExternal, linkExternalWithTracking +@docs toLinkAttributes + +-} import EventExtras import Html.Styled exposing (Attribute) From 54622b7a10af66e2af34848aa7e48527e2574734 Mon Sep 17 00:00:00 2001 From: Tessa Kelly Date: Mon, 16 May 2022 14:19:54 -0700 Subject: [PATCH 09/13] Implement the feature --- src/ClickableAttributes.elm | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/ClickableAttributes.elm b/src/ClickableAttributes.elm index aeb5bbebd..429807c9b 100644 --- a/src/ClickableAttributes.elm +++ b/src/ClickableAttributes.elm @@ -28,6 +28,8 @@ module ClickableAttributes exposing -} +import Accessibility.Styled.Role as Role +import Accessibility.Styled.Widget as Widget import EventExtras import Html.Styled exposing (Attribute) import Html.Styled.Attributes as Attributes @@ -127,7 +129,24 @@ toButtonAttributes clickableAttributes = {-| -} toLinkAttributes : { routeToString : route -> String, isDisabled : Bool } -> ClickableAttributes route msg -> ( String, List (Attribute msg) ) -toLinkAttributes { routeToString } clickableAttributes = +toLinkAttributes { routeToString, isDisabled } clickableAttributes = + let + ( linkTypeName, attributes ) = + toEnabledLinkAttributes routeToString clickableAttributes + in + ( linkTypeName + , if isDisabled then + [ Role.link + , Widget.disabled True + ] + + else + attributes + ) + + +toEnabledLinkAttributes : (route -> String) -> ClickableAttributes route msg -> ( String, List (Attribute msg) ) +toEnabledLinkAttributes routeToString clickableAttributes = let stringUrl = case ( clickableAttributes.urlString, clickableAttributes.url ) of From 0d1a9ea290f5c17039c1e0f1615abbf7bb2f5a30 Mon Sep 17 00:00:00 2001 From: Tessa Kelly Date: Mon, 16 May 2022 14:54:57 -0700 Subject: [PATCH 10/13] :skull: remove semi-abuses of button api in styleguide examples --- styleguide-app/Examples/Button.elm | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/styleguide-app/Examples/Button.elm b/styleguide-app/Examples/Button.elm index d6c9c702c..7d40e40ae 100644 --- a/styleguide-app/Examples/Button.elm +++ b/styleguide-app/Examples/Button.elm @@ -275,13 +275,7 @@ buttons model = exampleCell setStyle setSize = buttonOrLink model.label - ([ setSize - , setStyle - , Button.custom [ Html.Styled.Attributes.class "styleguide-button" ] - , Button.onClick (ShowItWorked "ButtonExample" "Button clicked!") - ] - ++ List.map Tuple.second model.attributes - ) + (setSize :: setStyle :: List.map Tuple.second model.attributes) |> List.singleton |> td [ css From 780361d8d49b4e5256f16a12e99cddef84dceb9b Mon Sep 17 00:00:00 2001 From: Tessa Kelly Date: Mon, 16 May 2022 15:50:05 -0700 Subject: [PATCH 11/13] Expand on the docs for disabled --- src/Nri/Ui/Button/V10.elm | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Nri/Ui/Button/V10.elm b/src/Nri/Ui/Button/V10.elm index 92f2bfcde..f62522bab 100644 --- a/src/Nri/Ui/Button/V10.elm +++ b/src/Nri/Ui/Button/V10.elm @@ -499,7 +499,13 @@ unfulfilled = set (\attributes -> { attributes | state = Unfulfilled }) -{-| Shows inactive styling. If a button, this attribute will disable it. +{-| Shows inactive styling. + +If a button, this attribute will disable it as you'd expect. + +If a link, this attribute will follow the pattern laid out in [Scott O'Hara's disabled links](https://www.scottohara.me/blog/2021/05/28/disabled-links.html) article, +and essentially make the anchor a disabled placeholder. + -} disabled : Attribute msg disabled = From 547f42ed3f80ba2456c3031dd4a17b8df6c306a5 Mon Sep 17 00:00:00 2001 From: Tessa Kelly Date: Mon, 16 May 2022 16:19:15 -0700 Subject: [PATCH 12/13] Fix cursor for disabled link --- src/Nri/Ui/Button/V10.elm | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Nri/Ui/Button/V10.elm b/src/Nri/Ui/Button/V10.elm index f62522bab..495eda8f8 100644 --- a/src/Nri/Ui/Button/V10.elm +++ b/src/Nri/Ui/Button/V10.elm @@ -788,6 +788,7 @@ buttonStyle = , Css.margin Css.zero , Css.hover [ Css.textDecoration Css.none ] , Css.disabled [ Css.cursor Css.notAllowed ] + , Css.Global.withAttribute "aria-disabled=true" [ Css.cursor Css.notAllowed ] , Css.display Css.inlineFlex , Css.alignItems Css.center , Css.justifyContent Css.center From c8bc517f457e32618652ed33e6842d0f06051f19 Mon Sep 17 00:00:00 2001 From: Tessa Kelly Date: Wed, 18 May 2022 16:43:24 -0700 Subject: [PATCH 13/13] Adds caveat to docs --- src/Nri/Ui/Button/V10.elm | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Nri/Ui/Button/V10.elm b/src/Nri/Ui/Button/V10.elm index 495eda8f8..181154565 100644 --- a/src/Nri/Ui/Button/V10.elm +++ b/src/Nri/Ui/Button/V10.elm @@ -506,6 +506,12 @@ If a button, this attribute will disable it as you'd expect. If a link, this attribute will follow the pattern laid out in [Scott O'Hara's disabled links](https://www.scottohara.me/blog/2021/05/28/disabled-links.html) article, and essentially make the anchor a disabled placeholder. +_Caveat!_ + +The styleguide example will NOT work correctly because of , which describes a problem where "a tags without href generate a navigation event". + +In most cases, if you're not using Browser.application, disabled links should work just fine. + -} disabled : Attribute msg disabled =