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

support data html options #1095

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

theforestvn88
Copy link

Rails 7 Hotwire requires many data html-options such as data-turbo-frame, data-controller (stimulus.js), ...
But it looks like Kaminari only support the data-remote option. So i think it's better to support other data html-options, too.

For example:

turbo_frame

  <%= turbo_frame_tag "tasks" do %>
    <div class="min-w-full">
      <%= render @tasks %>
    </div>
  <% end %>
  <%= paginate @tasks, data: {turbo_frame: :tasks} %>

stimulus controller

<%= paginate @tasks, data: {controller: "task", action: "click->log"} %>

@mfa777
Copy link

mfa777 commented Aug 24, 2022

After I searched issue list, I found there's a nice solution without modify Gem:
#417 (comment)

@stillhart
Copy link

This is kind of important since hotwired and turbo is default in rails 7.1. Maybe it would be even better to just pass down arguments directly using ** or having a specific parameter instead of doing it just for data. That would also solve many styling issues.

<%= paginate @tasks, link_to_attributes: {data: {}, whatever: true} %>

Right now I can't use kaminari+turbo without generating my own views.

@yuki24
Copy link
Member

yuki24 commented Feb 5, 2024

I can see this as a useful addition. I also think we can possibly deprecate the old remote option as it was a useful option back in the day, but probably not today.

@amatsuda Any thoughts on this?

@amatsuda
Copy link
Member

amatsuda commented Apr 4, 2024

I'm basically happy to merge this (and then soon cut a new gem release), but I'm still not sure how this patch actually improves your app.
Can anyone here convince me by elaborating a little bit more detaild use case, or by showing a small sample app?
Thanks!

@yuki24
Copy link
Member

yuki24 commented Apr 4, 2024

I think @theforestvn88 already explained it well in the first place with a <turbo-frame>, but implementing an example like this https://youtu.be/HURqvNJF4T0?si=fxI_kIXzSP-bGVPx&t=313 wouldn't be possible without it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants