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

fix: unify pagination to resolve problems with inconsistent pagination #2720

Merged
merged 1 commit into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/ocamlorg_frontend/components/pagination.eml
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
type t = {
total_page_count : int;
page_number : int;
base_url : string;
queries : ( string * string ) list;
}

let rec construct_query_string = function
| h :: t -> "&" ^ (fst h) ^ "=" ^ (snd h) ^ construct_query_string t
| [] -> ""
Expand Down Expand Up @@ -26,7 +33,7 @@ let right_arrow ~base_url page ~queries =
<%s! Icons.chevron_right "h-5 w-5" %>
</a>

let render ~total_page_count ~page_number ~base_url ~queries =
let render { total_page_count; page_number; base_url; queries } =
<div class="pt-16 flex items-center justify-center space-x-2.5">
<% if total_page_count > 1 then ( %>
<%s! if page_number != 1 then (left_arrow ~base_url page_number ~queries) else "" %>
Expand Down
7 changes: 3 additions & 4 deletions src/ocamlorg_frontend/components/tutorial_search.eml
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
let render
~search
~total
~number_of_pages
~page
~(pagination_info : Pagination.t)
(documents : Data.Tutorial.search_document list) =
Learn_layout.single_column_layout
~title:"OCaml Tutorial Documents Search"
Expand Down Expand Up @@ -40,7 +39,7 @@ Learn_layout.single_column_layout

<% if List.length documents > 0 then ( %>
<div class="text-content dark:text-dark-content mb-2 lg:mb-4">
Showing <%i ((page - 1) * 50 + 1) %> - <%i (min (page * 50) total) %>
Showing <%i ((pagination_info.page_number - 1) * 50 + 1) %> - <%i (min (pagination_info.page_number * 50) total) %>
</div>
<ol class="flex flex-col">
<% documents |> List.iter (fun (doc : Data.Tutorial.search_document) -> %>
Expand All @@ -62,7 +61,7 @@ Learn_layout.single_column_layout
<% ); %>
</ol>
<% ); %>
<%s! Pagination.render ~total_page_count:number_of_pages ~page_number:page ~base_url:Url.tutorial_search ~queries:[("q", search)] %>
<%s! Pagination.render pagination_info %>
</div>
</div>
</div>
Expand Down
1 change: 1 addition & 0 deletions src/ocamlorg_frontend/ocamlorg_frontend.ml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ module Package_overview = Package_overview
module Navmap = Navmap
module Toc = Toc
module Package = Package
module Pagination = Pagination

let about = About.render
let academic_users = Academic_users.render
Expand Down
41 changes: 12 additions & 29 deletions src/ocamlorg_frontend/pages/academic_institutions.eml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ let render_tag tag show =
in
<span class="px-3 inline-block max-w-max rounded-3xl font-mono text-sm text-center text-white <%s _cls %> <%s if show then "" else "hidden" %>"><%s name %></span>

let render ?(search = "") ?(continent = "All") ?(resource_type = "All") ~pages_number ~page (institutions : Data.Academic_institution.t list) =
let render ?(search = "") ?(continent = "All") ?(resource_type = "All") ~pagination_info (institutions : Data.Academic_institution.t list) =
Layout.render
~title:"Academic Institutions"
~description:"Many more academic institutions teach OCaml"
Expand Down Expand Up @@ -51,11 +51,11 @@ Layout.render
</form>
<% institutions |> List.iter (fun (item : Data.Academic_institution.t) -> let logo = match item.logo with | Some x -> Ocamlorg_static.Media.url x | None -> "" in %>
<div class="flex flex-col md:flex-row items-start lg:items-center border-b border-separator_20 dark:border-dark-separator_30 py-4">
<div class="w-44 h-24 rounded-lg flex justify-center items-center border border-separator_20 dark:border-dark-separator_30 bg-white overflow-hidden p-4 mr-5 mb-4">
<a href="<%s item.url %>" class="w-44 h-24 rounded-lg flex justify-center items-center border border-separator_20 dark:border-dark-separator_30 bg-white overflow-hidden p-4 mr-5 mb-4">
<img class="max-h-16 m-auto" src="<%s logo %>" alt="<%s item.name %> logo">
</div>
</a>
<div class="flex flex-col flex-grow">
<p class="font-bold text-lg text-title dark:text-dark-title mb-2"><%s item.name %></p>
<a href="<%s item.url %>" class="font-bold text-lg text-title dark:text-dark-title mb-2 hover:underline"><%s item.name %></a>
<ul class="w-full flex flex-col">
<% item.courses
|> List.filter (fun (course : Data.Academic_institution.course) -> match resource_type with
Expand All @@ -75,9 +75,11 @@ Layout.render
<%s! Icons.link "h-5 mr-2 text-primary dark:text-dark-white group-hover:text-primary dark:group-hover:text-dark-primary w-4 h-5" %>
<%s course.name %>
</a>
<%s! render_tag "lecture_notes" course.lecture_notes %>
<%s! render_tag "exercises" course.exercises %>
<%s! render_tag "video_recordings" course.video_recordings %>
<div class="flex gap-2">
<%s! render_tag "lecture_notes" course.lecture_notes %>
<%s! render_tag "exercises" course.exercises %>
<%s! render_tag "video_recordings" course.video_recordings %>
</div>
</li>
<% ); %>
</ul>
Expand All @@ -86,27 +88,8 @@ Layout.render
<% ); %>
</div>
<div class="container-fluid">
<div class="pt-16 flex items-center justify-center space-x-2.5 w-full">
<a href="<%s Url.academic_institutions %>?p=<%i page - 1 %>"
class="w-10 h-10 lg:w-14 lg:h-14 border border-body-100 rounded-lg flex items-center justify-center text-content hover:bg-gray-50 hover:no-underline">
<%s! Icons.chevron_left "h-5 w-5" %>
</a>
<% (for i = 1 to (pages_number - 1) do
if i = page then ( %>
<a href="<%s Url.academic_institutions %>?p=<%i i %>"
class="w-10 h-10 lg:w-14 lg:h-14 border-2 border-current rounded-lg flex items-center justify-center text-primary">
<%i i %>
</a>
<% ) else ( %>
<a href="<%s Url.academic_institutions %>?p=<%i i %>"
class="w-10 h-10 lg:w-14 lg:h-14 border border-body-100 rounded-lg flex items-center justify-center text-content hover:bg-gray-50 hover:no-underline">
<%i i %>
</a>
<% ) done); %>
<a href="<%s Url.academic_institutions %>?p=<%i page + 1 %>"
class="w-10 h-10 lg:w-14 lg:h-14 border border-body-100 rounded-lg flex items-center justify-center text-content hover:bg-gray-50 hover:no-underline">
<%s! Icons.chevron_right "h-5 w-5" %>
</a>
</div>
<div class="flex items-start">
<%s! Pagination.render pagination_info %>
</div>
</div>
</div>
6 changes: 3 additions & 3 deletions src/ocamlorg_frontend/pages/changelog.eml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
let render ~tags ?current_tag ~number_of_pages ~current_page (news : Data.Changelog.t list) =
let render ~tags ?current_tag ~(pagination_info : Pagination.t) (news : Data.Changelog.t list) =
News_layout.single_column_layout
~title:"OCaml Changelog"
~canonical:(Url.changelog ^ if current_page = 1 then "" else "?p=" ^ string_of_int current_page)
~canonical:(Url.changelog ^ if pagination_info.page_number = 1 then "" else "?p=" ^ string_of_int pagination_info.page_number)
~description:"Read the latest releases and updates from the OCaml ecosystem."
~current:Changelog @@
<div class="intro-section-simple dark:dark-intro-section-simple py-12">
Expand Down Expand Up @@ -71,7 +71,7 @@ News_layout.single_column_layout
</div>
</article>
<% ); %>
<%s! Pagination.render ~total_page_count:number_of_pages ~page_number:current_page ~base_url:Url.changelog ~queries:(match current_tag with Some current_tag -> [("t", current_tag)] | None -> [])%>
<%s! Pagination.render pagination_info %>
</div>
</div>
<div class="prose dark:prose-invert max-w-none mt-8">
Expand Down
25 changes: 3 additions & 22 deletions src/ocamlorg_frontend/pages/news.eml
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
let render ~pages_number ~page (news : Data.News.t list) =
let render ~(pagination_info : Pagination.t) (news : Data.News.t list) =
News_layout.single_column_layout
~title:"News from the OCaml teams"
~description:"Read the latest news, releases, and updates from the OCaml compiler and platform developers."
~canonical:(Url.news ^ if page = 1 then "" else "?p=" ^ string_of_int page)
~canonical:(Url.news ^ if pagination_info.page_number = 1 then "" else "?p=" ^ string_of_int pagination_info.page_number)
~current:Newsletters @@
<div class="intro-section-simple dark:dark-intro-section-simple py-12">
<div class="container-fluid">
Expand Down Expand Up @@ -41,26 +41,7 @@ News_layout.single_column_layout
<% ); %>

<div class="pt-16 flex items-center justify-center space-x-2.5">
<a href="<%s Url.news %>?p=<%i page - 1 %>"
class="w-10 h-10 lg:w-14 lg:h-14 border border-body-100 rounded-lg flex items-center justify-center text-content hover:bg-gray-50 hover:no-underline">
<%s! Icons.chevron_left "h-5 w-5" %>
</a>
<% (for i = 1 to (pages_number - 1) do
if i = page then ( %>
<a href="<%s Url.news %>?p=<%i i %>"
class="w-10 h-10 lg:w-14 lg:h-14 border-2 border-current rounded-lg flex items-center justify-center text-primary">
<%i i %>
</a>
<% ) else ( %>
<a href="<%s Url.news %>?p=<%i i %>"
class="w-10 h-10 lg:w-14 lg:h-14 border border-body-100 rounded-lg flex items-center justify-center text-content hover:bg-gray-50 hover:no-underline">
<%i i %>
</a>
<% ) done); %>
<a href="<%s Url.news %>?p=<%i page + 1 %>"
class="w-10 h-10 lg:w-14 lg:h-14 border border-body-100 rounded-lg flex items-center justify-center text-content hover:bg-gray-50 hover:no-underline">
<%s! Icons.chevron_right "h-5 w-5" %>
</a>
<%s! Pagination.render pagination_info %>
</div>
</div>
</div>
Expand Down
7 changes: 4 additions & 3 deletions src/ocamlorg_frontend/pages/ocaml_planet.eml
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,12 @@ let render_blog_post (post : Data.Planet.entry) =
<% ); %>
</div>
</div>
let render ?(category = "All") ~planet_pages_number ~planet_page (planet : Data.Planet.entry list) =

let render ?(category = "All") ~(pagination_info : Pagination.t) (planet : Data.Planet.entry list) =
News_layout.single_column_layout
~title:"The OCaml Planet"
~description:"The RSS aggregator for the OCaml community."
~canonical:(Url.ocaml_planet ^ if planet_page = 1 then "" else "?p=" ^ string_of_int planet_page)
~canonical:(Url.ocaml_planet ^ if pagination_info.page_number = 1 then "" else "?p=" ^ string_of_int pagination_info.page_number)
~alternate:{ href = "/planet.xml"; title = "The OCaml Planet"; type_ = "application/atom+xml" }
~current:OCamlPlanet @@
<div class="bg-white dark:bg-dark-background py-12">
Expand All @@ -76,7 +77,7 @@ News_layout.single_column_layout
|> String.concat "\n" ; %>
</div>
<div class="flex items-start">
<%s! Pagination.render ~total_page_count:planet_pages_number ~page_number:planet_page ~base_url:Url.ocaml_planet ~queries:[("category", category)]%>
<%s! Pagination.render pagination_info %>
</div>

</div>
Expand Down
6 changes: 3 additions & 3 deletions src/ocamlorg_frontend/pages/packages_search.eml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
let render ~total ~search ~page ~number_of_pages (packages : Package.package list) = Layout.render ~title:"OCaml Packages · Search Result"
let render ~total ~search ~(pagination_info : Pagination.t) (packages : Package.package list) = Layout.render ~title:"OCaml Packages · Search Result"
~description:"Find the package you need to build your application in the thousands of available OCaml packages."
~canonical:(Url.packages_search ^ "?q=" ^ search)
~active_top_nav_item:Header.Packages @@
Expand Down Expand Up @@ -43,7 +43,7 @@ let render ~total ~search ~page ~number_of_pages (packages : Package.package lis

<% if List.length packages > 0 then ( %>
<div class="text-title dark:text-dark-content mb-4">
Showing <%i ((page - 1) * 50 + 1) %> - <%i (min (page * 50) total) %>
Showing <%i ((pagination_info.page_number - 1) * 50 + 1) %> - <%i (min (pagination_info.page_number * 50) total) %>
</div>
<ol class="flex flex-col">
<% packages |> List.iter (fun (package : Package.package) -> %>
Expand Down Expand Up @@ -122,7 +122,7 @@ let render ~total ~search ~page ~number_of_pages (packages : Package.package lis
<% ); %>
</ol>
<% ); %>
<%s! Pagination.render ~total_page_count:number_of_pages ~page_number:page ~base_url:Url.packages_search ~queries:[("q", search)] %>
<%s! Pagination.render pagination_info %>
</div>
</div>
</div>
Expand Down
104 changes: 83 additions & 21 deletions src/ocamlorg_web/lib/handler.ml
Original file line number Diff line number Diff line change
Expand Up @@ -250,16 +250,30 @@ let paginate ~req ~n items =
in
(page, number_of_pages, current_items)

let query_param ~name value =
match value with None -> [] | Some v -> [ (name, v) ]

let learn_documents_search req =
let q = Dream.query req "q" |> Option.value ~default:"" in
let search_results = Data.Tutorial.search_documents q in
let page, number_of_pages, current_items =
paginate ~req ~n:50 search_results
let q = Dream.query req "q" in
let search_results =
Data.Tutorial.search_documents (q |> Option.value ~default:"")
in
let total = List.length search_results in
let page_number, total_page_count, current_items =
paginate ~req ~n:50 search_results
in
let pagination_info =
Ocamlorg_frontend.Pagination.
{
total_page_count;
page_number;
base_url = Url.tutorial_search;
queries = query_param ~name:"q" q;
}
in
Dream.html
(Ocamlorg_frontend.tutorial_search current_items ~total ~page
~number_of_pages ~search:q)
(Ocamlorg_frontend.tutorial_search current_items ~total ~pagination_info
~search:(q |> Option.value ~default:""))

let changelog req =
let current_tag = Dream.query req "t" in
Expand All @@ -277,10 +291,23 @@ let changelog req =
List.exists (( = ) tag) change.tags)
Data.Changelog.all
in
let page, number_of_pages, current_changes = paginate ~req ~n:30 changes in

let page_number, total_page_count, current_changes =
paginate ~req ~n:50 changes
in
let pagination_info =
Ocamlorg_frontend.Pagination.
{
total_page_count;
page_number;
base_url = Url.changelog;
queries = query_param ~name:"t" current_tag;
}
in

Dream.html
(Ocamlorg_frontend.changelog ?current_tag ~tags ~number_of_pages
~current_page:page current_changes)
(Ocamlorg_frontend.changelog ?current_tag ~tags ~pagination_info
current_changes)

let changelog_entry req =
let slug = Dream.param req "id" in
Expand Down Expand Up @@ -411,13 +438,24 @@ let academic_institutions req =
(fun institution -> matches_criteria institution continent resource_type)
users
in
let page, number_of_pages, institutions =
let page_number, total_page_count, institutions =
paginate ~req ~n:10 filtered_institutions
in

let pagination_info =
Ocamlorg_frontend.Pagination.
{
total_page_count;
page_number;
base_url = Url.academic_institutions;
queries =
query_param ~name:"q" query
@ query_param ~name:"resource_type" resource_type
@ query_param ~name:"continent" continent;
}
in
Dream.html
(Ocamlorg_frontend.academic_institutions ?search:query ?continent
?resource_type ~page ~pages_number:number_of_pages institutions)
?resource_type ~pagination_info institutions)

let about _req = Dream.html (Ocamlorg_frontend.about ())

Expand Down Expand Up @@ -542,19 +580,31 @@ let ocaml_planet req =
let filtered_entries =
Data.Planet.all |> List.filter (fun item -> matches_criteria item category)
in
let page, number_of_pages, current_items =
let page_number, total_page_count, current_items =
paginate ~req ~n:10 filtered_entries
in
let pagination_info =
Ocamlorg_frontend.Pagination.
{
total_page_count;
page_number;
base_url = Url.ocaml_planet;
queries = query_param ~name:"category" category;
}
in

Dream.html
(Ocamlorg_frontend.ocaml_planet ~planet_page:page
~planet_pages_number:number_of_pages ?category current_items)
(Ocamlorg_frontend.ocaml_planet ~pagination_info ?category current_items)

let news req =
let page, number_of_pages, current_items =
let page_number, total_page_count, current_items =
paginate ~req ~n:10 Data.News.all
in
Dream.html
(Ocamlorg_frontend.news ~page ~pages_number:number_of_pages current_items)
let pagination_info =
Ocamlorg_frontend.Pagination.
{ total_page_count; page_number; base_url = Url.news; queries = [] }
in
Dream.html (Ocamlorg_frontend.news ~pagination_info current_items)

let news_post req =
let slug = Dream.param req "id" in
Expand Down Expand Up @@ -982,18 +1032,30 @@ let packages_search t req =
| None -> Ocamlorg_package.all_latest t
in
let total = List.length packages in
let page, number_of_pages, current_items = paginate ~req ~n:50 packages in

let search =
Dream.from_percent_encoded
(match Dream.query req "q" with Some search -> search | None -> "")
in

let page_number, total_page_count, current_items =
paginate ~req ~n:50 packages
in
let pagination_info =
Ocamlorg_frontend.Pagination.
{
total_page_count;
page_number;
base_url = Url.packages_search;
queries = [ ("q", search) ];
}
in

let open Lwt.Syntax in
let* results = prepare_search_result_packages t current_items in

Dream.html
(Ocamlorg_frontend.packages_search ~total ~search ~page ~number_of_pages
results)
(Ocamlorg_frontend.packages_search ~total ~search ~pagination_info results)

let packages_autocomplete_fragment t req =
match Dream.query req "q" with
Expand Down
Loading