From 060daed760639468e8307ced1e5feb85e99ef110 Mon Sep 17 00:00:00 2001 From: Marek Fajkus Date: Sun, 29 Nov 2020 00:02:59 +0100 Subject: [PATCH] Refactor the loading and request management (#230) --- src/Main.elm | 154 ++++++++++++++--------------- src/Search.elm | 262 ++++++++++++++++++++++++++----------------------- src/index.less | 16 +++ 3 files changed, 225 insertions(+), 207 deletions(-) diff --git a/src/Main.elm b/src/Main.elm index b7d9152..d0a1f86 100644 --- a/src/Main.elm +++ b/src/Main.elm @@ -108,50 +108,68 @@ updateWith toPage toMsg model ( subModel, subCmd ) = ) -submitQuery : - Model - -> ( Model, Cmd Msg ) - -> ( Model, Cmd Msg ) -submitQuery old ( new, cmd ) = +attemptQuery : ( Model, Cmd Msg ) -> ( Model, Cmd Msg ) +attemptQuery (( model, _ ) as pair) = let - triggerSearch _ newModel msg makeRequest = - if - (newModel.query /= Nothing) - && (newModel.query /= Just "") - && List.member newModel.channel Search.channels - then - ( new - , Cmd.batch - [ cmd - , makeRequest - new.elasticsearch - newModel.channel - (Maybe.withDefault "" newModel.query) - newModel.from - newModel.size - newModel.sort - |> Cmd.map msg - ] + -- We intentially throw away Cmd + -- because we don't want to perform any effects + -- in this cases where route itself doesn't change + noEffects = + Tuple.mapSecond (always Cmd.none) + + submitQuery msg makeRequest searchModel = + Tuple.mapSecond + (\cmd -> + Cmd.batch + [ cmd + , Cmd.map msg <| + makeRequest + model.elasticsearch + searchModel.channel + (Maybe.withDefault "" searchModel.query) + searchModel.from + searchModel.size + searchModel.sort + ] ) + pair + in + case model.page of + Packages searchModel -> + if Search.shouldLoad searchModel then + submitQuery PackagesMsg Page.Packages.makeRequest searchModel else - ( new, cmd ) - in - case ( old.page, new.page ) of - ( Packages oldModel, Packages newModel ) -> - triggerSearch oldModel newModel PackagesMsg Page.Packages.makeRequest + noEffects pair - ( NotFound, Packages newModel ) -> - triggerSearch newModel newModel PackagesMsg Page.Packages.makeRequest + Options searchModel -> + if Search.shouldLoad searchModel then + submitQuery OptionsMsg Page.Options.makeRequest searchModel - ( Options oldModel, Options newModel ) -> - triggerSearch oldModel newModel OptionsMsg Page.Options.makeRequest + else + noEffects pair - ( NotFound, Options newModel ) -> - triggerSearch newModel newModel OptionsMsg Page.Options.makeRequest + _ -> + pair - ( _, _ ) -> - ( new, cmd ) + +pageMatch : Page -> Page -> Bool +pageMatch m1 m2 = + case ( m1, m2 ) of + ( NotFound, NotFound ) -> + True + + ( Home _, Home _ ) -> + True + + ( Packages _, Packages _ ) -> + True + + ( Options _, Options _ ) -> + True + + _ -> + False changeRouteTo : @@ -159,49 +177,6 @@ changeRouteTo : -> Url.Url -> ( Model, Cmd Msg ) changeRouteTo currentModel url = - let - attempteQuery ( newModel, cmd ) = - let - -- We intentially throw away Cmd - -- because we don't want to perform any effects - -- in this cases where route itself doesn't change - noEffects = - ( newModel, Cmd.none ) - in - case ( currentModel.route, newModel.route ) of - ( Route.Packages arg1, Route.Packages arg2 ) -> - if - (arg1.channel /= arg2.channel) - || (arg1.query /= arg2.query) - || (arg1.from /= arg2.from) - || (arg1.size /= arg2.size) - || (arg1.sort /= arg2.sort) - then - submitQuery newModel ( newModel, cmd ) - - else - noEffects - - ( Route.Options arg1, Route.Options arg2 ) -> - if - (arg1.channel /= arg2.channel) - || (arg1.query /= arg2.query) - || (arg1.from /= arg2.from) - || (arg1.size /= arg2.size) - || (arg1.sort /= arg2.sort) - then - submitQuery newModel ( newModel, cmd ) - - else - noEffects - - ( a, b ) -> - if a /= b then - submitQuery newModel ( newModel, cmd ) - - else - noEffects - in case Route.fromUrl url of Nothing -> ( { currentModel | page = NotFound } @@ -212,6 +187,13 @@ changeRouteTo currentModel url = let model = { currentModel | route = route } + + avoidReinit ( newModel, cmd ) = + if pageMatch currentModel.page newModel.page then + ( model, Cmd.none ) + + else + ( newModel, cmd ) in case route of Route.NotFound -> @@ -234,7 +216,8 @@ changeRouteTo currentModel url = in Page.Packages.init searchArgs modelPage |> updateWith Packages PackagesMsg model - |> attempteQuery + |> avoidReinit + |> attemptQuery Route.Options searchArgs -> let @@ -248,7 +231,8 @@ changeRouteTo currentModel url = in Page.Options.init searchArgs modelPage |> updateWith Options OptionsMsg model - |> attempteQuery + |> avoidReinit + |> attemptQuery update : Msg -> Model -> ( Model, Cmd Msg ) @@ -263,7 +247,13 @@ update msg model = Browser.External href -> ( model - , Browser.Navigation.load href + , case href of + -- ignore links with no `href` attribute + "" -> + Cmd.none + + _ -> + Browser.Navigation.load href ) ( ChangedUrl url, _ ) -> diff --git a/src/Search.elm b/src/Search.elm index d1ef331..e11bc96 100644 --- a/src/Search.elm +++ b/src/Search.elm @@ -12,6 +12,7 @@ module Search exposing , init , makeRequest , makeRequestBody + , shouldLoad , update , view ) @@ -148,10 +149,25 @@ init args model = |> fromSortId |> Maybe.withDefault Relevance } + |> ensureLoading , Browser.Dom.focus "search-query-input" |> Task.attempt (\_ -> NoOp) ) +shouldLoad : Model a -> Bool +shouldLoad model = + model.result == RemoteData.Loading + + +ensureLoading : Model a -> Model a +ensureLoading model = + if model.query /= Nothing && model.query /= Just "" && List.member model.channel channels then + { model | result = RemoteData.Loading } + + else + model + + -- --------------------------- -- UPDATE @@ -166,6 +182,7 @@ type Msg a | QueryInputSubmit | QueryResponse (RemoteData.WebData (SearchResult a)) | ShowDetails String + | ChangePage Int update : @@ -186,19 +203,15 @@ update toRoute navKey msg model = | sort = fromSortId sortId |> Maybe.withDefault Relevance , from = 0 } + |> ensureLoading |> pushUrl toRoute navKey ChannelChange channel -> { model | channel = channel - , result = - if model.query == Nothing || model.query == Just "" then - RemoteData.NotAsked - - else - RemoteData.Loading , from = 0 } + |> ensureLoading |> pushUrl toRoute navKey QueryInput query -> @@ -207,10 +220,8 @@ update toRoute navKey msg model = ) QueryInputSubmit -> - { model - | result = RemoteData.Loading - , from = 0 - } + { model | from = 0 } + |> ensureLoading |> pushUrl toRoute navKey QueryResponse result -> @@ -229,6 +240,11 @@ update toRoute navKey msg model = } |> pushUrl toRoute navKey + ChangePage from -> + { model | from = from } + |> ensureLoading + |> pushUrl toRoute navKey + pushUrl : Route.SearchRoute -> Browser.Navigation.Key -> Model a -> ( Model a, Cmd msg ) pushUrl toRoute navKey model = @@ -487,20 +503,26 @@ view { toRoute, categoryName } title model viewSuccess outMsg = ] ] ] - , case model.result of - RemoteData.NotAsked -> - div [] [ text "" ] + , div [] <| + case model.result of + RemoteData.NotAsked -> + [ text "" ] - RemoteData.Loading -> - div [ class "loader" ] [ text "Loading..." ] + RemoteData.Loading -> + [ p [] [ em [] [ text "Searching..." ] ] + , div [] + [ viewSortSelection outMsg model + , viewPager outMsg model 0 toRoute + ] + , div [ class "loader-wrapper" ] [ div [ class "loader" ] [ text "Loading..." ] ] + , viewPager outMsg model 0 toRoute + ] - RemoteData.Success result -> - if result.hits.total.value == 0 then - div [] + RemoteData.Success result -> + if result.hits.total.value == 0 then [ h4 [] [ text <| "No " ++ categoryName ++ " found!" ] ] - else - div [] + else [ p [] [ em [] [ text @@ -525,138 +547,128 @@ view { toRoute, categoryName } title model viewSuccess outMsg = ) ] ] - , form [ class "form-horizontal pull-right" ] - [ div - [ class "control-group" - ] - [ label [ class "control-label" ] [ text "Sort by:" ] - , div - [ class "controls" ] - [ select - [ onInput (\x -> outMsg (SortChange x)) - ] - (List.map - (\sort -> - option - [ selected (model.sort == sort) - , value (toSortId sort) - ] - [ text <| toSortTitle sort ] - ) - sortBy - ) - ] - ] + , div [] + [ viewSortSelection outMsg model + , viewPager outMsg model result.hits.total.value toRoute ] - , viewPager outMsg model result toRoute , viewSuccess model.channel model.show result - , viewPager outMsg model result toRoute + , viewPager outMsg model result.hits.total.value toRoute ] - RemoteData.Failure error -> - let - ( errorTitle, errorMessage ) = - case error of - Http.BadUrl text -> - ( "Bad Url!", text ) + RemoteData.Failure error -> + let + ( errorTitle, errorMessage ) = + case error of + Http.BadUrl text -> + ( "Bad Url!", text ) - Http.Timeout -> - ( "Timeout!", "Request to the server timeout." ) + Http.Timeout -> + ( "Timeout!", "Request to the server timeout." ) - Http.NetworkError -> - ( "Network Error!", "A network request bonsaisearch.net domain failed. This is either due to a content blocker or a networking issue." ) + Http.NetworkError -> + ( "Network Error!", "A network request bonsaisearch.net domain failed. This is either due to a content blocker or a networking issue." ) - Http.BadStatus code -> - ( "Bad Status", "Server returned " ++ String.fromInt code ) + Http.BadStatus code -> + ( "Bad Status", "Server returned " ++ String.fromInt code ) - Http.BadBody text -> - ( "Bad Body", text ) - in - div [ class "alert alert-error" ] - [ h4 [] [ text errorTitle ] - , text errorMessage + Http.BadBody text -> + ( "Bad Body", text ) + in + [ div [ class "alert alert-error" ] + [ h4 [] [ text errorTitle ] + , text errorMessage + ] ] ] +viewSortSelection : (Msg a -> b) -> Model a -> Html b +viewSortSelection outMsg model = + form [ class "form-horizontal pull-right sort-form" ] + [ div [ class "control-group sort-group" ] + [ label [ class "control-label" ] [ text "Sort by:" ] + , div + [ class "controls" ] + [ select + [ onInput (\x -> outMsg (SortChange x)) + ] + (List.map + (\sort -> + option + [ selected (model.sort == sort) + , value (toSortId sort) + ] + [ text <| toSortTitle sort ] + ) + sortBy + ) + ] + ] + ] + + viewPager : (Msg a -> b) -> Model a - -> SearchResult a + -> Int -> Route.SearchRoute -> Html b -viewPager _ model result toRoute = - ul [ class "pager" ] - [ li - [ classList - [ ( "disabled", model.from == 0 ) - ] - ] - [ a - [ href <| - if model.from == 0 then - "" +viewPager outMsg model total toRoute = + Html.map outMsg <| + ul [ class "pager" ] + [ li [ classList [ ( "disabled", model.from == 0 ) ] ] + [ a + [ Html.Events.onClick <| + if model.from == 0 then + NoOp - else - createUrl toRoute { model | from = 0 } + else + ChangePage 0 + ] + [ text "First" ] ] - [ text "First" ] - ] - , li - [ classList - [ ( "disabled", model.from == 0 ) - ] - ] - [ a - [ href <| - if model.from - model.size < 0 then - "" + , li [ classList [ ( "disabled", model.from == 0 ) ] ] + [ a + [ Html.Events.onClick <| + if model.from - model.size < 0 then + NoOp - else - createUrl toRoute { model | from = model.from - model.size } + else + ChangePage <| model.from - model.size + ] + [ text "Previous" ] ] - [ text "Previous" ] - ] - , li - [ classList - [ ( "disabled", model.from + model.size >= result.hits.total.value ) - ] - ] - [ a - [ href <| - if model.from + model.size >= result.hits.total.value then - "" + , li [ classList [ ( "disabled", model.from + model.size >= total ) ] ] + [ a + [ Html.Events.onClick <| + if model.from + model.size >= total then + NoOp - else - createUrl toRoute { model | from = model.from + model.size } + else + ChangePage <| model.from + model.size + ] + [ text "Next" ] ] - [ text "Next" ] - ] - , li - [ classList - [ ( "disabled", model.from + model.size >= result.hits.total.value ) - ] - ] - [ a - [ href <| - if model.from + model.size >= result.hits.total.value then - "" + , li [ classList [ ( "disabled", model.from + model.size >= total ) ] ] + [ a + [ Html.Events.onClick <| + if model.from + model.size >= total then + NoOp - else - let - remainder = - if remainderBy model.size result.hits.total.value == 0 then - 1 + else + let + remainder = + if remainderBy model.size total == 0 then + 1 - else - 0 - in - createUrl toRoute - { model | from = ((result.hits.total.value // model.size) - remainder) * model.size } + else + 0 + in + ChangePage <| ((total // model.size) - remainder) * model.size + ] + [ text "Last" ] ] - [ text "Last" ] ] - ] diff --git a/src/index.less b/src/index.less index 2bb9be9..d197cb3 100644 --- a/src/index.less +++ b/src/index.less @@ -1,6 +1,7 @@ body { position: relative; min-height: 100vh; + overflow-y: scroll; } #content { @@ -89,6 +90,21 @@ header .navbar.navbar-static-top { } } +.sort-form, +.sort-form > .sort-group { + margin-bottom: 0; +} +.pager { + & > li > a { + cursor: pointer; + margin: 0 2px; + } +} + +.loader-wrapper { + height: 200px; + overflow: hidden; +} .loader, .loader:before, .loader:after {