flop_phoenix icon indicating copy to clipboard operation
flop_phoenix copied to clipboard

Flop.Phoenix.pagination change to allow not to render next/prev links

Open oliverswitzer opened this issue 1 year ago • 6 comments

Description

Thanks for making this awesome library!

This PR adds the ability to specify that you do not wanter to render next and previous links.

Background: we're experiencing a styling conflict with our UI library daisyUI that causes the nubmered pages left border radius to get cut off when next / prev links are rendered (which we do not want).

image

This feature allows you to disable rendering those buttons if they aren't wanted.

Checklist

  • [ x] I added tests that cover my proposed changes.
  • [ x] I updated the documentation related to my changes (if applicable).

oliverswitzer avatar Sep 28 '23 15:09 oliverswitzer

FYI. I was able to solve the same thing with the built in options. For example, I hid the page numbers with:

def pagination_opts do
  [
    pagination_list_attrs: [class: "hidden"],
  ]
end

cadebward avatar Oct 01 '23 16:10 cadebward

FYI. I was able to solve the same thing with the built in options. For example, I hid the page numbers with:

def pagination_opts do
  [
    pagination_list_attrs: [class: "hidden"],
  ]
end

Do you still think the changes in this PR would be useful, or are you happy with the CSS solution?

woylie avatar Oct 02 '23 15:10 woylie

Coverage Status

coverage: 100.0%. remained the same when pulling 322d12101e0ebec0468e5ce054c60a9b5ec11e65 on oliverswitzer:update-pagination-component into 12e6c15d5d44243c9959f40774b242f7cdc419f7 on woylie:main.

coveralls avatar Oct 02 '23 15:10 coveralls

I am personally happy with the CSS solution, but curious to hear what @oliverswitzer thinks.

cadebward avatar Oct 03 '23 14:10 cadebward

Hey friends, sorry, been a busy couple of weeks!

To be clear I wasn't hoping to hide page numbers entirely, which seems like what the solution @cadebward was recommending (pagination_list_attrs: [class: "hidden"]) would do, but instead fix the issue shown in the screenshot where the border radius of the left most pagination number is cut off due to the presence of the "previous" button in the list.

Even if I try to hide the previous and next buttons with setting next_link_attrs={[class: "hidden"]} and previous_link_attrs={[class: "hidden"]}, this issue still persists due to the element still being present in the DOM. daisyUI's join class selector (used in their pagination component) is treating the previous button as the first element that needs a border-radius left applied, even though it is technically "display: none"... As a result the actual first element that should be visible in the list (ie the 1st page button) is not getting the proper border radius. I hope that makes sense.

I think next/prev elements need to not be rendered in the DOM entirely to fix this issue.

oliverswitzer avatar Oct 10 '23 14:10 oliverswitzer

Hey friends, sorry, been a busy couple of weeks!

To be clear I wasn't hoping to hide page numbers entirely, which seems like what the solution @cadebward was recommending (pagination_list_attrs: [class: "hidden"]) would do, but instead fix the issue shown in the screenshot where the border radius of the left most pagination number is cut off due to the presence of the "previous" button in the list.

Even if I try to hide the previous and next buttons with setting next_link_attrs={[class: "hidden"]} and previous_link_attrs={[class: "hidden"]}, this issue still persists due to the element still being present in the DOM. daisyUI's join class selector (used in their pagination component) is treating the previous button as the first element that needs a border-radius left applied, even though it is technically "display: none"... As a result the actual first element that should be visible in the list (ie the 1st page button) is not getting the proper border radius. I hope that makes sense.

I think next/prev elements need to not be rendered in the DOM entirely to fix this issue.

Alright, let's add the option. I just had two small comments.

woylie avatar Oct 12 '23 10:10 woylie