Pagination path :page not encoding safe

[mrdotb]

Hello,

I found a small inconsistency issue with the pagination component path attribute. :page when encoded with URI.encode_query became %3Apage breaking the code below.

https://github.com/petalframework/petal_components/blob/main/lib/petal_components/pagination.ex#L129C1-L131C6

  defp get_path(path, page_number, current_page) when is_binary(path) do
    get_path(&String.replace(path, ":page", Integer.to_string(&1)), page_number, current_page)
  end

Let's say we want to conserve a map of active param filters while paging and use the sigil p which use URI.encode_query

params = %{"filter" => "test", "page" => "1"}
# put :page
params = Map.put(params, "page", ":page")
# "filter=test&page=%3Apage"
<.pagination
  link_type="live_patch"
  path={~p"/teams?#{@params}"}
  current_page={@current_page}
  total_pages={@total_pages}
/>

[mplatts]

Yes I've had that issue. For now I do this:

  <.pagination
      link_type="live_patch"
      class="my-5"
      path={build_paginated_path(@search_params)}
      current_page={@paginated_entries.current_page}
      total_pages={@paginated_entries.total_pages}
    />

  defp build_paginated_path(params) do
    ~p"/?#{Map.put(params, "page", ":page")}"
    |> String.replace("%3Apage", ":page")
  end

Where @search_params are all the params from the URL.

But yeah, would be good to fix this. Open to ideas on how to do it

[mrdotb]

:smile: I did the same fix with a String.replace and wrap the pagination in my own component.

So about the root cause I will say that using : is a mistake because it's not url safe. Those chars are url safe !, -, _ and don't have a special meaning.

Without a breaking change the easy solution is to URI.encode path in the get_path function and replace on %3A instead of :. This way it's possible to pass sigil_p or a string with : non encoded.

Happy to write the PR if you agree.

[mplatts]

Thanks yeah I agree.. a PR would be very helpful thanks.

[mrdotb]

Fixed in #263