From 093718d9ed31463e29b30fca00d3521eaef71698 Mon Sep 17 00:00:00 2001 From: Tessa Kelly Date: Tue, 8 Sep 2020 14:35:36 -0700 Subject: [PATCH] Fix broken on-focus tooltip behavior --- src/Nri/Ui/SegmentedControl/V13.elm | 9 +++------ src/Nri/Ui/Tabs/V7.elm | 6 ++---- src/TabsInternal/V2.elm | 18 ++++++++---------- styleguide-app/Examples/SegmentedControl.elm | 20 +++++++------------- styleguide-app/Examples/Tabs.elm | 17 ++++++++--------- 5 files changed, 28 insertions(+), 42 deletions(-) diff --git a/src/Nri/Ui/SegmentedControl/V13.elm b/src/Nri/Ui/SegmentedControl/V13.elm index 41d9a9aa..70d683f5 100644 --- a/src/Nri/Ui/SegmentedControl/V13.elm +++ b/src/Nri/Ui/SegmentedControl/V13.elm @@ -153,8 +153,7 @@ type alias Option value msg = {-| - - `onSelect` : the message to produce when an option is selected by the user - - `onFocus` : the message to focus an element by id string + - `focusAndSelect` : the message to produce when an option is selected by the user - `options`: the list of options available - `selected`: the value of the currently-selected option - `positioning`: how to position and size the segmented control @@ -162,8 +161,7 @@ type alias Option value msg = -} view : - { onSelect : a -> msg - , onFocus : String -> msg + { focusAndSelect : { select : a, focus : Maybe String } -> msg , options : List (Option a msg) , selected : a , positioning : Positioning @@ -185,8 +183,7 @@ view config = { tabList, tabPanels } = TabsInternal.views - { onSelect = config.onSelect - , onFocus = config.onFocus + { focusAndSelect = config.focusAndSelect , selected = config.selected , tabs = List.map toInternalTab config.options , tabListStyles = diff --git a/src/Nri/Ui/Tabs/V7.elm b/src/Nri/Ui/Tabs/V7.elm index 0c73cce1..81877d14 100644 --- a/src/Nri/Ui/Tabs/V7.elm +++ b/src/Nri/Ui/Tabs/V7.elm @@ -112,8 +112,7 @@ view : { title : Maybe String , alignment : Alignment , customSpacing : Maybe Float - , onSelect : id -> msg - , onFocus : String -> msg + , focusAndSelect : { select : id, focus : Maybe String } -> msg , selected : id , tabs : List (Tab id msg) } @@ -133,8 +132,7 @@ view config = { tabList, tabPanels } = TabsInternal.views - { onSelect = config.onSelect - , onFocus = config.onFocus + { focusAndSelect = config.focusAndSelect , selected = config.selected , tabs = List.map toInternalTab config.tabs , tabStyles = tabStyles config.customSpacing diff --git a/src/TabsInternal/V2.elm b/src/TabsInternal/V2.elm index f211d94e..bc28c7b6 100644 --- a/src/TabsInternal/V2.elm +++ b/src/TabsInternal/V2.elm @@ -23,8 +23,7 @@ import Nri.Ui.Util exposing (dashify) {-| -} type alias Config id msg = - { onSelect : id -> msg - , onFocus : String -> msg + { focusAndSelect : { select : id, focus : Maybe String } -> msg , selected : id , tabs : List (Tab id msg) , tabListStyles : List Css.Style @@ -85,15 +84,15 @@ viewTab_ config index tab = else AttributesExtra.none - , Attributes.href href - , EventExtras.onClickPreventDefaultForLinkWithHref (config.onSelect tab.id) + , EventExtras.onClickPreventDefaultForLinkWithHref + (config.focusAndSelect { select = tab.id, focus = Nothing }) ] ) Nothing -> -- This is for a non-SPA view ( Html.button - , [ Events.onClick (config.onSelect tab.id) + , [ Events.onClick (config.focusAndSelect { select = tab.id, focus = Nothing }) ] ) @@ -107,7 +106,6 @@ viewTab_ config index tab = , Widget.selected isSelected , Role.tab , Attributes.id (tabToId tab.idString) - , Events.onFocus (config.onSelect tab.id) , Events.on "keyup" <| Json.Decode.andThen (keyEvents config tab) Events.keyCode ] @@ -132,7 +130,7 @@ viewTab_ config index tab = keyEvents : Config id msg -> Tab id msg -> Int -> Json.Decode.Decoder msg -keyEvents { onFocus, tabs } thisTab keyCode = +keyEvents { focusAndSelect, tabs } thisTab keyCode = let findAdjacentTab tab acc = case acc of @@ -140,7 +138,7 @@ keyEvents { onFocus, tabs } thisTab keyCode = acc ( True, Nothing ) -> - ( True, Just (tabToId tab.idString) ) + ( True, Just { select = tab.id, focus = Just (tabToId tab.idString) } ) ( False, Nothing ) -> ( tab.id == thisTab.id, Nothing ) @@ -158,7 +156,7 @@ keyEvents { onFocus, tabs } thisTab keyCode = -- Right case nextTab of Just next -> - Json.Decode.succeed (onFocus next) + Json.Decode.succeed (focusAndSelect next) Nothing -> Json.Decode.fail "No next tab" @@ -167,7 +165,7 @@ keyEvents { onFocus, tabs } thisTab keyCode = -- Left case previousTab of Just previous -> - Json.Decode.succeed (onFocus previous) + Json.Decode.succeed (focusAndSelect previous) Nothing -> Json.Decode.fail "No previous tab" diff --git a/styleguide-app/Examples/SegmentedControl.elm b/styleguide-app/Examples/SegmentedControl.elm index b6637120..e6667a77 100644 --- a/styleguide-app/Examples/SegmentedControl.elm +++ b/styleguide-app/Examples/SegmentedControl.elm @@ -49,8 +49,7 @@ example = , Html.p [ css [ Css.marginTop (Css.px 1) ] ] [ Html.text "Use in cases where it would also be reasonable to use Tabs." ] , SegmentedControl.view - { onSelect = SelectPage - , onFocus = Focus + { focusAndSelect = FocusAndSelectPage , selected = state.page , positioning = options.positioning , toUrl = Nothing @@ -221,14 +220,13 @@ optionsControl = (List.map (\i -> ( String.fromInt i, Control.value i )) (List.range 2 8)) ) |> Control.field "long content" (Control.bool False) - |> Control.field "tooltips" (Control.bool False) + |> Control.field "tooltips" (Control.bool True) {-| -} type Msg - = Focus String + = FocusAndSelectPage { select : Page, focus : Maybe String } | Focused (Result Dom.Error ()) - | SelectPage Page | PageTooltip Page Bool | SelectRadio Int | ChangeOptions (Control Options) @@ -238,9 +236,10 @@ type Msg update : Msg -> State -> ( State, Cmd Msg ) update msg state = case msg of - Focus id -> - ( state - , Task.attempt Focused (Dom.focus id) + FocusAndSelectPage { select, focus } -> + ( { state | page = select } + , Maybe.map (Task.attempt Focused << Dom.focus) focus + |> Maybe.withDefault Cmd.none ) Focused _ -> @@ -248,11 +247,6 @@ update msg state = , Cmd.none ) - SelectPage page -> - ( { state | page = page } - , Cmd.none - ) - PageTooltip page isOpen -> ( { state | pageTooltip = diff --git a/styleguide-app/Examples/Tabs.elm b/styleguide-app/Examples/Tabs.elm index 348b865c..d9aa7a9d 100644 --- a/styleguide-app/Examples/Tabs.elm +++ b/styleguide-app/Examples/Tabs.elm @@ -81,8 +81,7 @@ type Id type Msg - = SelectTab Id - | Focus String + = FocusAndSelectTab { select : Id, focus : Maybe String } | Focused (Result Dom.Error ()) | SetSettings (Control Settings) | ToggleTooltip Id Bool @@ -91,11 +90,12 @@ type Msg update : Msg -> State -> ( State, Cmd Msg ) update msg model = case msg of - SelectTab id -> - ( { model | selected = id }, Cmd.none ) - - Focus id -> - ( model, Task.attempt Focused (Dom.focus id) ) + FocusAndSelectTab { select, focus } -> + ( { model | selected = select } + , focus + |> Maybe.map (Dom.focus >> Task.attempt Focused) + |> Maybe.withDefault Cmd.none + ) Focused error -> ( model, Cmd.none ) @@ -146,8 +146,7 @@ example = { title = settings.title , alignment = settings.alignment , customSpacing = settings.customSpacing - , onSelect = SelectTab - , onFocus = Focus + , focusAndSelect = FocusAndSelectTab , selected = model.selected , tabs = allTabs model.openTooltip }