Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move Pagination to CSS framework #1157

Merged
merged 29 commits into from
Oct 22, 2021
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 16 additions & 6 deletions IHP/Pagination/ViewFunctions.hs
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,13 @@ import IHP.Controller.Param (paramOrNothing)
import IHP.View.Classes
import qualified Network.Wai as Wai
import qualified Network.HTTP.Types.URI as Query
import IHP.ViewSupport (theRequest)
import IHP.ViewSupport (theRequest, theCSSFramework)
import qualified Data.Containers.ListUtils as List
import IHP.View.Types (PaginationView(..), styledPagination)
import IHP.View.CSSFramework


-- | Render a navigation for your pagination. This is to be used in your view whenever
-- | Render a navigation for your pagination. This is to be used in your view whenever
-- to allow users to change pages, including "Next" and "Previous".
renderPagination :: (?context::ControllerContext) => Pagination -> Html
renderPagination pagination@Pagination {currentPage, window, pageSize} =
Expand Down Expand Up @@ -53,15 +55,23 @@ renderPagination pagination@Pagination {currentPage, window, pageSize} =
</div>
</div>
</div>

|]
where
paginationView = PaginationView
amitaibu marked this conversation as resolved.
Show resolved Hide resolved
{ cssFramework = theCSSFramework
, pagination = pagination
, previousPageUrl = pageUrl $ currentPage - 1
, nextPageUrl = pageUrl $ currentPage + 1
}

renderedHtml = styledPagination theCSSFramework theCSSFramework paginationView

maxItemsGenerator = let
oneOption :: Int -> Html
oneOption n = [hsx|<option value={show n} selected={n == pageSize} data-url={itemsPerPageUrl n}>{n} items per page</option>|]
in
[hsx|{forEach [10,20,50,100,200] oneOption}|]

nextClass = classes ["page-item", ("disabled", not $ hasNextPage pagination)]
prevClass = classes ["page-item", ("disabled", not $ hasPreviousPage pagination)]

Expand Down Expand Up @@ -198,10 +208,10 @@ setQueryValue name value queryString =
)
Nothing -> queryString <> [(name, Just value)]

-- | Removes a query item, specificed by the name
-- | Removes a query item, specified by the name
--
-- >>> removeQueryItem "filter" [("filter", Just "test")]
-- []
--
removeQueryItem :: ByteString -> Query.Query -> Query.Query
removeQueryItem name queryString = queryString |> filter (\(queryItemName, _) -> queryItemName /= name)
removeQueryItem name queryString = queryString |> filter (\(queryItemName, _) -> queryItemName /= name)
43 changes: 43 additions & 0 deletions IHP/View/CSSFramework.hs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ import qualified Text.Blaze.Html5 as H
import Text.Blaze.Html5 ((!), (!?))
import qualified Text.Blaze.Html5.Attributes as A
import IHP.ModelSupport
import IHP.Pagination.Helpers
import IHP.Pagination.Types


-- | Provides an unstyled CSSFramework
--
Expand All @@ -37,6 +40,7 @@ instance Default CSSFramework where
, styledFormGroupClass
, styledValidationResult
, styledValidationResultClass
, styledPagination
}
where
styledFlashMessages cssFramework flashMessages = forEach flashMessages (styledFlashMessage cssFramework cssFramework)
Expand Down Expand Up @@ -155,6 +159,45 @@ instance Default CSSFramework where

styledSubmitButtonClass = ""

styledPagination :: CSSFramework -> PaginationView -> Blaze.Html
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mpscholten this default implementation is very much Bootstrap framework, however I don't see much value at having a stripped of classes version, as it will never be used as-is. Do you agree?

So bootstrap function will have no change, only tailwind will

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another option that might be better is to break it down into smaller pieces, so bootsrap and tailwind will just have to define the classes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this default implementation is very much Bootstrap framework, however I don't see much value at having a stripped of classes version, as it will never be used as-is. Do you agree?

Agree

Another option that might be better is to break it down into smaller pieces, so bootsrap and tailwind will just have to define the classes.

I just checked the tailwind docs and it looks like tailwind has a completly different dom structure for pagination. So I'd go with the first approach and just make bootstrap the default

styledPagination _ paginationView =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a couple of tests to the new functions? https://github.com/digitallyinduced/ihp/blob/master/Test/View/CSSFrameworkSpec.hs :)

[hsx| <div>
{liPrevious}

<div></div>

{liNext}



</div>|]
where
pagination@Pagination {currentPage} = get #pagination paginationView
previousPageUrl = get #previousPageUrl paginationView
nextPageUrl = get #nextPageUrl paginationView
prevClass = classes ["page-item", ("disabled", not $ hasPreviousPage pagination)]
nextClass = classes ["page-item", ("disabled", not $ hasNextPage pagination)]

liPrevious :: Blaze.Html
liPrevious = [hsx|
<li class={prevClass}>
<a class="page-link" href={previousPageUrl} aria-label="Previous">
<span aria-hidden="true">&laquo;</span>
<span class="sr-only">Previous</span>
</a>
</li>
|]

liNext = [hsx|
<li class={nextClass}>
<a class="page-link" href={nextPageUrl} aria-label="Previous">
<span aria-hidden="true">&raquo;</span>
<span class="sr-only">Next</span>
</a>
</li>
|]


bootstrap :: CSSFramework
bootstrap = def { styledFlashMessage, styledSubmitButtonClass, styledFormGroupClass, styledFormFieldHelp, styledInputClass, styledInputInvalidClass, styledValidationResultClass }
where
Expand Down
15 changes: 15 additions & 0 deletions IHP/View/Types.hs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ module IHP.View.Types
, FormContext (..)
, InputType (..)
, CSSFramework (..)
, PaginationView(..)
, HtmlWithContext
, Layout
)
Expand All @@ -21,6 +22,9 @@ import qualified Text.Blaze.Html5 as Blaze
import IHP.FlashMessages.Types
import IHP.ModelSupport (Violation)

-- @todo: Move PaginationView to IHP.Pagination.Types?
import IHP.Pagination.Types


type HtmlWithContext context = (?context :: context) => Blaze.Html

Expand Down Expand Up @@ -97,6 +101,15 @@ data InputType
| FileInput


-- | Options for customizing the render of a pagination
data PaginationView =
PaginationView
{ cssFramework :: !CSSFramework
Copy link
Collaborator Author

@amitaibu amitaibu Oct 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need a cssFramework property if we also call styledPagination with CSSFramework? Seems that te pattern also with styledFormField

we later end up calling renderedHtml = styledPagination theCSSFramework theCSSFramework paginationView. How many theCSSFramework do we need? 😅

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's a bit tricky :) Basically we simulate late-binding from OOP here.

This code:

renderedHtml = styledPagination theCSSFramework theCSSFramework paginationView

Can also be written using get:

renderedHtml = (get #styledPagination theCSSFramework) theCSSFramework paginationView

That's important to understand here. We get a styledPagination function using get #styledPagination theCSSFramework.

Then we apply theCSSFramework to that function. We apply the theCSSFramework to that function because the styledPagination function internally might want to call other functions of the CSSFramework type. But you might want to override some functions of the CSSFramework.

Here's an example what would if we don't pass this a second time:

data CSSFramework = CSSFramework { styledPagination :: Html, styledButton :: Html }

bootstrapCSS = CSSFramework { styledPagination, styledButton }
  where
    styledPagination = [hsx|<div>{styledButton}</div>|]
    styledButton = [hsx|<button style="color: red">button</button>|]]

myPage = [hsx|{styledPagination bootstrapCSS}|]

All good. But now as a user we want to override the button styling:

customCSS = bootstrapCSS { styledButton = [hsx|<button style="color: green">button</button>|]]  }

myPage = [hsx|{styledPagination customCSS}|]

When we now render the myPage we will get <div><button style="color: red">button</button></div> (a red button, while our customCSS specified it should be green).

We can fix this by late-binding the calls by manually passing around a CSSFramework. Here I added a CSSFramework to all functions:

data CSSFramework = CSSFramework { styledPagination :: CSSFramework -> Html, styledButton :: CSSFramework -> Html }

bootstrapCSS = CSSFramework { styledPagination, styledButton }
  where
    styledPagination cssFramework = [hsx|<div>{get #styledButton cssFramework}</div>|]
    styledButton cssFramework = [hsx|<button style="color: red">button</button>|]]

myPage = [hsx|{styledPagination bootstrapCSS bootstrapCSS}|]

Now we can customize it again:

customCSS = bootstrapCSS { styledButton = \cssFramework -> [hsx|<button style="color: green">button</button>|]]  }

myPage = [hsx|{styledPagination customCSS customCSS}|]

I hope this clears things up.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super helpful, thanks - In fact I'll copy/ paste this explanation to CSSFramework.hs

, pagination :: !Pagination
, previousPageUrl :: !ByteString
, nextPageUrl :: !ByteString
}

-- | Render functions to render with bootstrap etc.
--
-- We call this functions with the cssFramework passed to have late binding (like from OOP languages)
Expand All @@ -121,4 +134,6 @@ data CSSFramework = CSSFramework
, styledValidationResult :: CSSFramework -> FormField -> Blaze.Html
-- | Class name for container of validation error message
, styledValidationResultClass :: Text
-- | Renders a pager
, styledPagination :: CSSFramework -> PaginationView -> Blaze.Html
}
10 changes: 10 additions & 0 deletions Main.hs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,16 @@ instance FrontController RootApplication where
instance Controller DemoController where
action DemoAction = renderPlain "Hello World!"

-- @todo: Without this I got an error
-- * No instance for (Worker RootApplication)
-- arising from a use of `IHP.Server.run'
-- * In the expression: IHP.Server.run config
-- In an equation for `main': main = IHP.Server.run config
-- |
-- 31 | main = IHP.Server.run config
instance Worker RootApplication where
amitaibu marked this conversation as resolved.
Show resolved Hide resolved
workers _ = []

config :: ConfigBuilder
config = do
option Development
Expand Down