-
Notifications
You must be signed in to change notification settings - Fork 436
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
turbo_frame_tag on Table element #48
Comments
I can confirm that is working properly if I change the element to |
Table éléments only accept a set of elements as children. It's not valid html to put a div inside a table or a tr directly. I've had the same problem, my solution was to fake a table by using divs and the correct display attributes such as table-row or table-cell. This is not ideal though, maybe we could have a way of telling turbo that the whole tr / tbody should be a frame via a data attribute or something, having forms inside tables that update live on change is a useful pattern so it's a shame that it's a hassle to implement with turbo. |
Only table elements are welcome inside tables. You can use streams to target replacement of individual ids though. |
I would like to understand why this does not work on tbody.
by that I meant that the turbo frame works properly on divs, I was not referring to replace the tbody with div tho |
If you want to replace the tbody, then instead give the tbody an id attribute, and use a turbo streams response to target that. |
@sstephenson I understand that HTML has constraints over what types of elements are allowed within tables, but the big picture idea here has merit. i.e. it would be very useful for turbo to be able to replace table rows in a simple, intuitive way Excluding this capability adds a major limitation that is not intuitive to new users and, to be honest, doesn't really make sense. I think this is too important to not consider, and I think many more people will be confused and wonder why it doesn't "just work" when they try to update tables using turbo. I had this exact same problem a few days ago. The first thing I tried to do with turbo was set it up to auto update a table of data, and it didn't work. I tried to implement a few ways and couldn't get the table data to actually replace. When I saw this issue and looked through the code (and the tags it inserts into the page) I understood why it didn't work and changed to using a div arrangement with table-like styling. But I would prefer to use table markup for the data if possible. Could this be implemented either by wrapping a turbo frame around the whole table, or by adding some type of data attributes to I'm happy to help with implementation, but I understand it may not be a top priority right now. |
I agree that editing table rows a la a spreadsheet is a very compelling use case, and it would be nice to use traditional table markup. But, with judicious application of |
@jonathanhefner I'm not the original author of this issue, so my use case was a little different. (i.e. not a form) I mostly wanted to highlight the value add of getting this to work with tabular data with actual |
issue author here: imo, as the primary integration of turbo is Rails oriented I think that many users will be hit by this because Rails scaffolds generates tables for listings, so I guess it would be a nice one to tackle this. Anyways, it seems that @inopinatus suggestion allows to use tables with turbo, so I can live with that. |
As I understand it, browsers that support Custom Elements also support the
I wonder if the original example could make use of turbo-frame elements through this mechanism: <table>
<thead>
<tr>
<th>Name</th>
<th>Key</th>
<th>User</th>
<th colspan="3"></th>
</tr>
</thead>
<tbody id="rooms" is="turbo-frame">
<% @rooms.each do |room| %>
<%= render room %> <%# assumes that `rooms/room` renders a `<tr>`
<% end %>
</tbody>
</table> @sstephenson does the NOTE Following up on this, it appears that Safari support is incomplete, and declaring |
I've just spent an hour hitting this exact issue too. Perhaps one solution would be to add some extra custom types that extend the tbody and tr elements with turbo-enabled elements that behave exactly like turbo-frame. This would allow to easily use something like
Not as neat as having the ability to use turbo_frame_tag everywhere but it's also an easy compromise to get such a common feature working until all major browsers support the is attribute. And much less hassle than having to target individual ids in responses. |
FWIW I had a similar issue in my lazy loading gem, I made multiple elements and just let the user decide which element to use: https://github.com/julianrubisch/futurism/tree/master/javascript/elements it’s probably not so easy in turbo, but maybe doable? |
This PR abstracts the FrameElement class into an anonymous class factory and matching interface type, permitting mixin to (almost) any element, and enabling registration of customized built-in elements as frames in addition to the standard autonomous custom element. Supports treating elements as a Turbo Frame that cannot otherwise carry one due to their content model, such as <tbody>. Set up with: Turbo.defineCustomFrameElement('tbody') and then use like: <table> <tbody id="tbody" is="turbo-frame-tbody"> <tr><td>Table content</td></tr> </tbody> </table> The response frame must match by the element name and both the is and id attributes. Implements: hotwired#48.
Ran into this myself as well. Given Rails' default scaffolding behavior of generating tables for CRUD interfaces, it seems reasonable to assume that tables would be supported by core interactive functionality. Tables are still valid, and often a preferred, markup structure that would see a lot of value in proper turbo_frame_tag support. |
This issue is real :( |
urgh. also knew about limitations with the nesting of tags in table. but was surprised that there are no solution for this limitation 😩 |
Also ran into this issue. First time using turbo, which lead me down many rabbit holes before determining it was the table. Realise turbo is in beta, but as @etcook stated, tables are a default, it'd be nice if turbo clearly documented/highlighted this limitation as not to waste dev cycles. Similar to how UJS compatibility is mentioned: https://github.com/hotwired/turbo-rails#compatibility-with-rails-ujs Beyond that, loving the concepts behind turbo. Backends are back! |
If you add anything other than this could easily be changed, if instead/alternative to the tag a
which could lazy load the |
I have a workaround in place to load "parts" of a table lazily (load once!). But you'll need quite a few pieces (I've simplified it "a lot" so is easier to see what's happening): The idea is to place the tag where it is expected (within a <tbody id="my_frame_anchor">
<tr><td>
<turbo-frame id="my_frame" data-anchor="my_frame_anchor" src="a_nice_path"></turbo-frame>
</td></tr>
</tbody> The response need to be valid to, so you need to pack the <turbo-frame id="my_frame">
<template>
<%= render "rows" %>
</template>
</turbo-frame> The last piece is some js to watch for the insertion (sadly no event, so...) and rearrange the dom: // get the content of the template in the turbo-frame and rewrite the tbody-anchor contents with it
var observer = new MutationObserver(function( mutations ) {
var $turboTarget = $(mutations[0].target);
var anchor = $('#' + $turboTarget.data('anchor'));
$anchor.html($turboTarget.find('template').html());
});
observer.observe($('#my_frame')[0], { childList: true}); This is as ugly as it gets, but might give some ideas on how to cope with this issue, or the missing puzzle pieces. |
Is there any better solution for this? |
any news on this ? |
This PR abstracts the FrameElement class into an anonymous class factory and matching interface type, permitting mixin to (almost) any element, and enabling registration of customized built-in elements as frames in addition to the standard autonomous custom element. Supports treating elements as a Turbo Frame that cannot otherwise carry one due to their content model, such as <tbody>. Set up with: Turbo.defineCustomFrameElement('tbody') and then use like: <table> <tbody id="tbody" is="turbo-frame-tbody"> <tr><td>Table content</td></tr> </tbody> </table> The response frame must match by the element name and both the is and id attributes. Implements: hotwired#48.
This PR abstracts the FrameElement class into an anonymous class factory and matching interface type, permitting mixin to (almost) any element, and enabling registration of customized built-in elements as frames in addition to the standard autonomous custom element. Supports treating elements as a Turbo Frame that cannot otherwise carry one due to their content model, such as <tbody>. Set up with: Turbo.defineCustomFrameElement('tbody') and then use like: <table> <tbody id="tbody" is="turbo-frame-tbody"> <tr><td>Table content</td></tr> </tbody> </table> The response frame must match by the element name and both the is and id attributes. Implements: hotwired#48.
My solution for this was to add a hidden turbo frame outside of the table and use a MutationObserver to append its contents to the table whenever they changed. In some ways it seems like a dirty hack, and in others quite elegant 😄 connect() {
let thisController = this;
this.observer = new MutationObserver(function(mutations) {
let tbody = thisController.entriesTarget
let source = thisController.newContentTarget;
while (source.hasChildNodes() ) {
tbody.appendChild(source.removeChild(source.firstChild))
}
})
observer.observe(this.newContentTarget, { childList: true })
}
disconnect() {
this.observer.disconnect()
} |
turbo frames effect the document flow so you need to style them accordingly - for example if you have a frame inside a hierarchy of flex containers, you will need to style the frame as flex also. It would be nice if you could style them to be "transparent" to the rendering of a page and have child elements pretend they don't exist, and react (display wise) within the outer container. |
@atstockland @defsdoor have either of you tried It's worth noting that there are some bugs in browsers that pose accessibility concerns for the element itself (but not for its descendants). However, since |
I'd previously googled as many different terms as I could think of to see if there was a way to do this and came up blank. I've just tried this in my app and it seems to work - brilliant - thank you ;) Is there any mention of this in the turbo documentation ? I've not seen it - it's essential to know imho. |
I knew before trying turbo about the table limitation. I went ahead and replaced my tables using an home made table: HTML
CSS
and it kinda works but it is still bad. Since I trying to mimic a table behavior, the turbo_frame will only fill the space of 1 column. It feels like there is not really any perfect approach I can think of at the moment. However, this workaround might help in finding a workaround for the op, but still, inserting any random tag within this custom table might break the table/table-row/table-cell behavior aswell. |
You need to wrap the entire table in a turbo_frame_tag. Give each row or element you want to target a uniq DOM id. Each element you want to target needs to originate from a partial. Then, in your controller action that responds to turbo_stream simply target the DOM id (not the turbo_frame_tag id) using the same source partial (be sure to pass in all the same locals). This will work. I typically target the row id, and turbo_stream.replace "uniq_row_id" if I want to manipulate existing data in a row. If you have other links in the table be sure to use data: {turbo_frame: "_top"} since the entire table is now wrapped in turbo. You could give the tbody a unique id and then turbo_stream.prepend to insert a new row on top of existing rows. Each row needs to originate from a partial and then use that same partial to render a new row via turbo_stream. Placing the turbo_frame_tag anywhere inside the table is not valid... your browser actually attempts to move that tag outside the table on render... which breaks everything. |
Here you’re using turbo streams which require you to have a specific handling in your controllers, you don’t need a turbo frame tag at all . However if you want to benefit from the frame capabilities ( no need to change anything in the controller , scoped navigation , etc ) you can’t proceed as such |
The |
Rails 7 no longer uses tables in the scaffold templates. |
@dhh: Are you sure? Rails version: 7.0.1 (Ruby 3.1.0) Results: Actually, the problem is in the haml-rails gem |
haml is not part of Rails itself. So looks like those templates are being generated by the HAML gem, and that gem may still be using tables. Rails itself no longer does. |
I have opened an issue over there (haml/haml-rails#179) and I am working to find a solution. What I'd like to make is a solution that used plain vanilla css. I don't use bootstrap and I can not assume that everyone uses my own CSS Framework choice (Zurb-Foundation) |
It's not clear to me why this issue has been closed. It seems that it's still a problem without a fully satisfactory workaround. There's a suggested solution using turbo streams on ticket 567, but as is clear from the example code, the solution forces you to now have two different versions of your template code, which immediately makes the whole Turbo thing less useful. As someone using Turbo for the first time, this is quite a major snag. Would it be possible to implement the solution that's been suggested in #567 of allowing an element with a Reopen and make awesome fix? :-) |
There is a mechanism in HTML to perform this upgrade; it's called Custom Built-in Elements. Unfortunately, this mechanism isn't ready for general use, because Webkit (i.e. Safari) refuses to implement it. Nevertheless I wrote up a treatment that generalizes Turbo to allow custom built-ins, see #131. Slightly stale now but was working on Firefox and Chrome, and could be updated readily for current Turbo if the Webkit situation changes. In the meantime I recommend revisiting the turbo-stream solution in #567, with the following nuance:
I'm loathe to turn a github issue into a tutorial ... but ... you can still DRY this up, refactoring to render the table rows using a partial, and letting Rails select the appropriate container e.g. with formats and (my favourite) partial layouts. |
Thanks @inopinatus, and nice work on #131, that looks very promising. Re. code reuse, I'm actually using Django, but you're right in that it's technically possible to keep things DRY - the code isn't repeated, there's just now more of it 😀 . It works, but it's starting to feel like I'm making my application more complicated, not less. Anyway, given that a solution to this is being worked on, and there are clearly lots of people encountering this problem, could someone with the relevant permissions re-open this ticket? |
Another point worth noting on the turbo-stream solution is that there doesn't appear to be a Maybe that could be fixed separately, but as a current limitation it's another good reason for pushing towards getting a solution to allow Turbo Frames to work inside tables. |
This is what @inopinatus said, putting for someone who is new to hotwire.
And in controller:
With other actions u shouldn't have problems, I think only append and prepend are the issue. |
You can also have a unique ID on each table row
|
This solution is worked for me for CRUD operation now i can insert, delete and update with updated view without refreshing page. index.html.erb
_room.html.erb
|
@hannan228 Could you elaborate? Do the |
@manuelmeurer yes, By adding a unique id and |
@hannan228 <%= turbo_stream.prepend "table-body" do %>
<%= render partial: 'incomes/income', locals: { income: @income } %>
<% end %> in <tbody id="table-body" is="turbo-frame" class="bg-white divide-y divide-gray-200 dark:bg-gray-800 dark:divide-gray-700">
<%= render(TableNoItemsComponent.new(show: @collection.blank?)) %>
<%= render(partial: @partial, collection: @collection) %>
</tbody> |
@thamdavies Please convert the table rows into turbo-frames. Please note that turbo-frames won't work on the tbody tag, but they will work on tr tags. If you want to prepend the entire table, then enclose tbody in a new div. I believe this should make it work. thanks |
Many of these examples target creation, but for me the issue was editing the row I was on and, for whatever reason (not using safari), the FWIW, the missing piece for me was adding <%= button_to t('edit'), edit_something_path(something), method: :get, data: { turbo_stream: true } %> Then, I could define <%= turbo_stream.replace @something, partial: '', locals: {} %> |
Late to the party, non rails user here... Just encountered this issue, and was able to get row edit working with this simple setup table.html <turbo-frame id="edit-target"></turbo-frame>
<table class="table">
<tbody>
<tr id="row-1">
<td>
<a href="/edit.html" data-turbo-frame="edit-target"> <!-- target the dummy frame -->
Edit me
</a>
</td>
</tr>
</tbody>
</table> edit.html <turbo-frame id="edit-target">
<turbo-stream action="replace" target="row-1"> <!-- replacing row to edit with stream -->
<template>
<tr id="row-1">
<td>
<input value="Ok"></input>
</td>
</tr>
</template>
</turbo-stream>
</turbo-frame> |
Wow! Thanks a lot! I did not know you can put turbo_stream tag inside turbo_frame. This solved all the issues with tables. |
Have turbo stream taget a tbody table
tbody id='newrows'
= render partial: 'a_row'
---
= turbo_stream.append 'newrows' do
= render partial: 'a_row' |
Here is my simple way of adding a row to a table. Maybe not best practice, but works. The view is the document index page at
Documents are broadcast to the index page using the trigger of callback:
Happy to hear any improvements, just noting this basic version for anyone who finds this thread. |
Hi , I'm trying Turbo , simple crud, table list:
example code:
but the frame tag is rendered separate from the list , why ?
note the red line is the frame tag border style:
The text was updated successfully, but these errors were encountered: