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/src/ClickableAttributes.elm b/src/ClickableAttributes.elm index 5d0ad1537..429807c9b 100644 --- a/src/ClickableAttributes.elm +++ b/src/ClickableAttributes.elm @@ -1,19 +1,35 @@ 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 Accessibility.Styled.Role as Role +import Accessibility.Styled.Widget as Widget import EventExtras import Html.Styled exposing (Attribute) import Html.Styled.Attributes as Attributes @@ -112,8 +128,25 @@ 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, 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 diff --git a/src/Nri/Ui/Button/V10.elm b/src/Nri/Ui/Button/V10.elm index 980c77d77..181154565 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 = @@ -476,7 +499,19 @@ 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. + +_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 = @@ -557,32 +592,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 +611,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) @@ -775,6 +794,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 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 diff --git a/styleguide-app/Examples/Button.elm b/styleguide-app/Examples/Button.elm index fa6098358..7d40e40ae 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 ) @@ -276,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 diff --git a/tests/Spec/ClickableAttributes.elm b/tests/Spec/ClickableAttributes.elm new file mode 100644 index 000000000..366f8927d --- /dev/null +++ b/tests/Spec/ClickableAttributes.elm @@ -0,0 +1,180 @@ +module Spec.ClickableAttributes exposing (suite) + +import Accessibility.Aria as Aria +import Accessibility.Role as Role +import ClickableAttributes exposing (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" + [ linkAttributes, disabledLinkAttributes ] + + +linkAttributes : Test +linkAttributes = + describe "link attributes" + [ test "with an href" <| + \() -> + ClickableAttributes.href "some-route" + |> setupLinkTest + |> Expect.all + [ Query.has [ Selector.attribute (href "some-route") ] + , -- should not include a redundant role + Query.hasNot [ Selector.attribute Role.link ] + , Query.hasNot [ Selector.attribute (Aria.disabled True) ] + ] + , test "with an external href" <| + \() -> + ClickableAttributes.linkExternal "some-route" + |> setupLinkTest + |> Expect.all + [ Query.has [ Selector.attribute (href "some-route") ] + , -- 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" <| + \() -> + ClickableAttributes.linkExternalWithTracking { track = "track it!", url = "some-route" } + |> setupLinkTest + |> Expect.all + [ Query.has [ Selector.attribute (href "some-route") ] + , -- should not include a redundant role + Query.hasNot [ Selector.attribute Role.link ] + , Query.hasNot [ Selector.attribute (Aria.disabled True) ] + ] + , test "with a SPA link" <| + \() -> + ClickableAttributes.linkSpa "some-route" + |> setupLinkTest + |> Expect.all + [ Query.has [ Selector.attribute (href "some-route") ] + , -- 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" <| + \() -> + ClickableAttributes.linkWithMethod { method = "the right way", url = "some-route" } + |> setupLinkTest + |> Expect.all + [ Query.has [ Selector.attribute (href "some-route") ] + , -- 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" <| + \() -> + ClickableAttributes.linkWithTracking { track = "track it!", url = "some-route" } + |> setupLinkTest + |> Expect.all + [ Query.has [ Selector.attribute (href "some-route") ] + , -- should not include a redundant role + Query.hasNot [ Selector.attribute Role.link ] + , Query.hasNot [ Selector.attribute (Aria.disabled True) ] + ] + ] + + +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") ] + , -- 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" <| + \() -> + ClickableAttributes.linkExternal "some-route" + |> 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 ] + , -- should be marked as disabled for screenreader users + Query.has [ Selector.attribute (Aria.disabled True) ] + ] + , 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") ] + , -- 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" <| + \() -> + ClickableAttributes.linkSpa "some-route" + |> 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 ] + , -- should be marked as disabled for screenreader users + Query.has [ Selector.attribute (Aria.disabled True) ] + ] + , 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") ] + , -- 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" <| + \() -> + ClickableAttributes.linkWithTracking { track = "track it!", url = "some-route" } + |> 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 ] + , -- should be marked as disabled for screenreader users + Query.has [ Selector.attribute (Aria.disabled True) ] + ] + ] + + +setupLinkTest : (ClickableAttributes String msg -> ClickableAttributes String msg) -> Query.Single msg +setupLinkTest withLink = + 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 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