From 4ece76902af66053958249220b3fc07082295003 Mon Sep 17 00:00:00 2001 From: Tom Nunn Date: Fri, 10 Mar 2023 21:24:58 +0000 Subject: [PATCH 1/3] Optimise filtering. Only filter when input/items change. --- examples/tests/ExampleTest.elm | 7 ++++ src/Internal/Model.elm | 69 ++++++++++++++++++++++++---------- src/Internal/Msg.elm | 2 +- src/Internal/Update.elm | 4 +- src/Internal/View.elm | 13 ++++++- 5 files changed, 70 insertions(+), 25 deletions(-) diff --git a/examples/tests/ExampleTest.elm b/examples/tests/ExampleTest.elm index 5ee37e2..7577484 100644 --- a/examples/tests/ExampleTest.elm +++ b/examples/tests/ExampleTest.elm @@ -84,6 +84,13 @@ exampleProgramTest = programTestWith (Select.withMinInputLength (Just 3)) |> ProgramTest.fillIn "" "Choose a country" "zzzz" |> ProgramTest.expectViewHas [ Selector.text "No matches" ] + , Test.test "Choosing an option and then focusing back on the input shows all the options again" <| + \() -> + programTest + |> ProgramTest.fillIn "" "Choose a country" "United" + |> Select.Effect.simulateClickOption simulateInputConfig "country-select" "🇬🇧 United Kingdom of Great Britain and Northern Ireland" + |> ProgramTest.simulateDomEvent (Query.find [ Selector.id (Select.toInputElementId countrySelect) ]) Test.Html.Event.focus + |> ProgramTest.expectViewHas [ Selector.text "🇦🇩 Andorra" ] ] diff --git a/src/Internal/Model.elm b/src/Internal/Model.elm index 0d23b09..b731522 100644 --- a/src/Internal/Model.elm +++ b/src/Internal/Model.elm @@ -1,6 +1,5 @@ module Internal.Model exposing ( Model - , applyFilter , blur , clear , closeMenu @@ -13,6 +12,7 @@ module Internal.Model exposing , openMenu , selectOption , setElements + , setFilteredOptions , setFocused , setInputValue , setItems @@ -55,6 +55,7 @@ type Model a type alias InternalState a = { id : String , items : List a + , filteredOptions : Maybe (List (Option a)) , selected : Maybe a , inputValue : String , highlighted : Maybe Int @@ -77,6 +78,7 @@ init id = Model { id = id , items = [] + , filteredOptions = Nothing , selected = Nothing , inputValue = "" , highlighted = Nothing @@ -129,20 +131,28 @@ toFilteredOptions minInputLength itemToString filter (Model model) = case minInputLength of Just chars -> if String.length model.inputValue >= chars then - List.map (Option.init itemToString) model.items - |> Filter.filterOptions model.inputValue filter + toFilteredOptions_ itemToString filter (Model model) else [] Nothing -> - List.map (Option.init itemToString) model.items - |> (if model.applyFilter then - Filter.filterOptions model.inputValue filter + if model.applyFilter then + toFilteredOptions_ itemToString filter (Model model) - else - identity - ) + else + List.map (Option.init itemToString) model.items + + +toFilteredOptions_ : (a -> String) -> Maybe (Filter a) -> Model a -> List (Option a) +toFilteredOptions_ itemToString filter (Model model) = + case model.filteredOptions of + Just opts -> + opts + + Nothing -> + List.map (Option.init itemToString) model.items + |> Filter.filterOptions model.inputValue filter toInputElementId : Model a -> String @@ -233,18 +243,42 @@ isRequestFailed (Model { requestState }) = setItems : List a -> Model a -> Model a -setItems items (Model d) = - Model { d | items = items } +setItems items (Model model) = + Model + { model + | items = items + , filteredOptions = + if items == model.items then + model.filteredOptions + + else + Nothing + } setSelected : Maybe a -> Model a -> Model a -setSelected a (Model d) = - Model { d | selected = a } +setSelected a (Model model) = + Model { model | selected = a } setInputValue : String -> Model a -> Model a -setInputValue v (Model d) = - Model { d | inputValue = v } +setInputValue v (Model model) = + Model + { model + | inputValue = v + , applyFilter = True + , filteredOptions = + if v == model.inputValue then + model.filteredOptions + + else + Nothing + } + + +setFilteredOptions : List (Option a) -> Model a -> Model a +setFilteredOptions opts (Model model) = + Model { model | filteredOptions = Just opts } selectOption : Option a -> Model a -> Model a @@ -307,11 +341,6 @@ setElements { container, menu } (Model model) = } -applyFilter : Bool -> Model a -> Model a -applyFilter v (Model model) = - Model { model | applyFilter = v } - - setFocused : Bool -> Model a -> Model a setFocused v (Model model) = Model { model | focused = v } diff --git a/src/Internal/Msg.elm b/src/Internal/Msg.elm index 3585729..c45ca99 100644 --- a/src/Internal/Msg.elm +++ b/src/Internal/Msg.elm @@ -5,7 +5,7 @@ import Internal.Option exposing (Option) type Msg a - = InputChanged String + = InputChanged String (List (Option a)) | OptionClicked (Option a) | InputFocused (Maybe Int) | InputClicked (Maybe Int) diff --git a/src/Internal/Update.elm b/src/Internal/Update.elm index f639ff0..cbe7de7 100644 --- a/src/Internal/Update.elm +++ b/src/Internal/Update.elm @@ -25,7 +25,7 @@ update ({ onSelect } as options) tagger msg model = update_ : UpdateOptions effect a msg -> (Msg a -> msg) -> Msg a -> Model a -> ( Model a, Effect effect msg ) update_ { request, requestMinInputLength, debounceRequest, onFocus, onLoseFocus, onInput } tagger msg model = case msg of - InputChanged val -> + InputChanged val filteredOptions -> ( model |> Model.setInputValue val |> Model.highlightIndex @@ -36,7 +36,7 @@ update_ { request, requestMinInputLength, debounceRequest, onFocus, onLoseFocus, Just 0 ) False - |> Model.applyFilter True + |> Model.setFilteredOptions filteredOptions |> Model.setSelected Nothing |> Model.setItems (if request /= Nothing && val == "" then diff --git a/src/Internal/View.elm b/src/Internal/View.elm index c45c6ee..c1f63f2 100644 --- a/src/Internal/View.elm +++ b/src/Internal/View.elm @@ -80,7 +80,10 @@ view attribs v = toElement : Model a -> ViewConfigInternal a msg -> Element msg toElement model config = - toElement_ (Model.toMenuPlacement config.menuMaxHeight config.menuPlacement model) (Model.toFilteredOptions config.minInputLength config.itemToString config.filter model) model config + toElement_ (Model.toMenuPlacement config.menuMaxHeight config.menuPlacement model) + (Model.toFilteredOptions config.minInputLength config.itemToString config.filter model) + model + config toElement_ : Placement -> List (Option a) -> Model a -> ViewConfigInternal a msg -> Element msg @@ -173,7 +176,13 @@ inputView filteredOptions model config = ] ++ inputAccessibilityAttributes filteredOptions model ) - { onChange = InputChanged >> config.onChange + { onChange = + \v -> + InputChanged v + (Model.setInputValue v model + |> Model.toFilteredOptions config.minInputLength config.itemToString config.filter + ) + |> config.onChange , text = if Model.isFocused model then Model.toInputValue model From b09648db428934e3393813e189753c937824c832 Mon Sep 17 00:00:00 2001 From: Tom Nunn Date: Fri, 10 Mar 2023 21:38:32 +0000 Subject: [PATCH 2/3] Update changelog --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cff3444..70213f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## 3.1.1 + +### Enhancements + +- Optimised filtering. Previously filtering ran every time the virtual DOM re-rendered the select. This caused performance issues with large lists of items. Now the filter only runs when the input or options are changed. +- Filter startsWithThenContains now uses a faster implementation. + ## 3.1.0 ### New features From a75b0803608ec81b09c255e199bb9c6948ff0811 Mon Sep 17 00:00:00 2001 From: Tom Nunn Date: Fri, 10 Mar 2023 21:41:43 +0000 Subject: [PATCH 3/3] Bump version 3.1.1 --- README.md | 2 +- elm.json | 2 +- package.json | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 4b111ad..b4cca79 100644 --- a/README.md +++ b/README.md @@ -87,4 +87,4 @@ view countries model = ## Limitations -There are issues when the input is placed within a parent element that has overflow scroll or auto: the menu may be clipped by the parent. This can be overcome by using [Select.withMenuPositionFixed](https://package.elm-lang.org/packages/nunntom/elm-ui-select/3.1.0/Select/#withMenuPositionFixed), but if the parent also has a transform applied, it gets clipped again. This means any parent with e.g. Element.scrollBarY + Element.moveDown/moveLeft etc. can cause issues. This is due to [a feature of the current CSS spec](https://bugs.chromium.org/p/chromium/issues/detail?id=20574). +There are issues when the input is placed within a parent element that has overflow scroll or auto: the menu may be clipped by the parent. This can be overcome by using [Select.withMenuPositionFixed](https://package.elm-lang.org/packages/nunntom/elm-ui-select/3.1.1/Select/#withMenuPositionFixed), but if the parent also has a transform applied, it gets clipped again. This means any parent with e.g. Element.scrollBarY + Element.moveDown/moveLeft etc. can cause issues. This is due to [a feature of the current CSS spec](https://bugs.chromium.org/p/chromium/issues/detail?id=20574). diff --git a/elm.json b/elm.json index 5716719..5cd9036 100644 --- a/elm.json +++ b/elm.json @@ -3,7 +3,7 @@ "name": "nunntom/elm-ui-select", "summary": "A select widget for Elm Ui", "license": "BSD-3-Clause", - "version": "3.1.0", + "version": "3.1.1", "exposed-modules": [ "Select", "Select.Filter", diff --git a/package.json b/package.json index 1792838..aa3764b 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "elm-ui-select", - "description": "A select widget for elm-ui.", - "version": "3.1.0", + "description": "A select widget for elm-ui with keyboard input, filtering, menu scrolling and requests!", + "version": "3.1.1", "scripts": { "test": "cd examples && elm-test && cd ../", "review": "elm-review",