Closed michelson closed 3 years ago
I can confirm that is working properly if I change the element to <div>
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.
I can confirm that is working properly if I change the element to
by that I meant that the turbo frame works properly on divs, I was not referring to replace the tbody with div tho
<turbo-frame>
is not a table element, so your browser hoists it out of the table to the nearest adjacency where it is permitted. So the above is invalid HTML.
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 tbody
and each tr
element? Or would it make more sense to have a separate variant of turbo_frame_tag
that used table tags and added the necessary attributes to ensure they were handled?
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 display: table-cell
and display: none
, you might be able to reuse your original form HTML (e.g. _form.html.erb
in the Rails world), instead of having a dedicated table row form. That could be a bigger win.
@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 tr
rows.
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. It would be a good idea to document this , maybe a list of supported elements. or trigger some console warn on the code ?
As I understand it, browsers that support Custom Elements also support the is
attribute:
The
is
global attribute allows you to specify that a standard HTML element should behave like a defined custom built-in element (see Using custom elements for more details).This attribute can only be used if the specified custom element name has been successfully defined in the current document, and extends the element type it is being applied to.
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 [is]
attribute mesh with how we find turbo-frame
elements? Would a <tbody id="rooms" is="turbo-frame">
compared via document.getElementById("rooms") instanceof FrameElement
evaluate to true
?
NOTE Following up on this, it appears that Safari support is incomplete, and declaring [is]
does not make an element evaluate instanceof FrameElement
to true
.
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
<table>
<thead>
<tr>
<th>Name</th>
<th>Key</th>
<th>User</th>
<th colspan="3"></th>
</tr>
</thead>
<turbo-table-body id="rooms">
<% @rooms.each do |room| %>
<%= render room %> <%# assumes that `rooms/room` renders a `<turbo-tr>`
<% end %>
</turbo-table-body>
</table>
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?
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 <tr><td>
it will be out of the table (at least chrome renders it outside, if you see the dom)
this could easily be changed, if instead/alternative to the tag a data-turbo-target
could be used:
<tbody data-turbo-target="target_id" data-turbo-target-src="...">
<tr><td colspan=9999>loading...</td></tr>
</tbody>
which could lazy load the tbody
(as suggested in https://github.com/hotwired/turbo/issues/148 )
I couldn't find a js event that get's triggered when a frame is replaced, is there any? (could be used as a work around)
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>tr>td
, by the way you can have as mucho of those as you want):
<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 tr>td
in a template:
<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. (Which I won't be testing anymore, since I just realize because of UJS, we can't really replace turbolinks with turbo...)
Is there any better solution for this?
any news on this ?
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 :smile:
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()
}
Just discovered that you can target any element in a turbo stream response - it doesn't have to be a turbo frame. So appending to a table body can be done by simply targetting it appropriately.
<turbo-stream action="append" target="devicesTableBody">
<template>
<%= render partial: 'row', collection: @devices, formats: [:html] %>
</template>
</turbo-steam>
I'm sure you used to get an error saying the target wasn't a frame :|
@defsdoor How do you use it with https://github.com/hotwired/turbo-rails syntax ?
@defsdoor How do you use it with https://github.com/hotwired/turbo-rails syntax ?
Sorry I don't fully understand what you are asking.
I manage infinite scrolling by having a hidden div that is below the table (the table has sticky headers) and use an intersection observer that spots when this div is close to being visible. Inside the div is a form that has the next page for it's URL.
This form is submitted via Rails.fire(form, 'submit')
The next page URL renders next_page.turbo_stream.erb by default and that contains -
<turbo-stream action="append" target="devicesTableBody">
<template>
<%= render partial: 'row', collection: @devices, formats: [:html] %>
</template>
</turbo-steam>
<turbo-stream action="update" target="devicesNextPage">
<template>
<%= render partial: 'next_page', formats: [:html] %>
</template>
</turbo-steam>
Which appends to the table body and updates the next page div with the new next page URL
turbo_streams target= must be provided a DOM ID, where targets= can be any class selector.
I just ran into this issue using Rails 6.1.4.1. I rebuild the table using divs with Bootstrap 5 utility classes (d-table-row, d-table-cell, etc). The div table renders perfect until I wrap it in a turbo_frame_tag...and, then the table flow breaks.
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.
The div table renders perfect until I wrap it in a turbo_frame_tag...and, then the table flow breaks.
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 display: contents
(consider Tailwind's .contents
class for an example)?
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 <turbo-frame>
is a Custom element anyway, there aren't any semantics to miscommunicate to assistive technology.
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
#project_segments
.table-custom
.table-row
.table-cell= sortable_header :name, model_class.human_attribute_name(:name)
.table-cell= sortable_header :created_at, model_class.human_attribute_name(:created_at)
.table-cell
- project_segments.each do |project_segment|
= turbo_frame_tag project_segment, class: 'table-row' do
.table-cell= link_to project_segment.name, project_project_segment_path(@project, project_segment), target: :_top
.table-cell= l project_segment.created_at, format: :long
.table-cell.text-nowrap
- if policy(project_segment).edit?
= link_to edit_project_project_segment_path(@project, project_segment), class: "btn btn-outline-primary btn-sm mx-1" do
%i.fas.fa-pen
- if policy(project_segment).destroy?
= link_to project_project_segment_path(@project, project_segment), class: "btn btn-outline-danger btn-sm mx-1",
data: { "controller": "delete", "action": "click->delete#confirm", info: project_segment.id } do
%i.fas.fa-trash
CSS
.table-custom {
@extend .d-table;
@extend .mt-2;
@extend .mb-3;
width: 100%;
font-size: 0.9rem;
font-weight: 300;
color: rgb(33, 37, 41);
.table-row:first-child {
font-weight: 500;
}
.table-row {
@extend .d-table-row;
}
.table-cell {
@extend .d-table-cell;
@extend .p-2; /* Should be 3, I think 2 is looking better. */
border-bottom: 1px solid rgb(224, 224, 224);
}
}
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.
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
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.
The rails g sacaffold ...
creates <table>s
. A strong warning should be present in the docs explaining why using Turbo with scaffold created files is not possible.
Rails 7 no longer uses tables in the scaffold templates.
@dhh: Are you sure?
Rails version: 7.0.1 (Ruby 3.1.0)
Command: rails g scaffold test titre:string commentaire:text
File: app/views/\<resource>/index.html.haml
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 (https://github.com/haml/haml-rails/issues/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 data-turbo-frame
attribute to behave as a <turbo-frame>
, e.g. <tr id="list-1" data-turbo-frame>
? This seems to me like a really elegant solution, which would allow users to get the full power of turbo frames, including complete template reuse, capable of being used regardless of the restrictions of which HTML elements are valid in a given position in the DOM.
Reopen and make awesome fix? :-)
Would it be possible to implement the solution that's been suggested in #567 of allowing an element with a
data-turbo-frame
attribute to behave as a<turbo-frame>
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:
the solution forces you to now have two different versions of your template code
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 turbo:stream-render
event, so if you're trying to integrate with something like Vue.js (as described in this video) then that approach of using the events to re-initialise your Vue (or other FE) components isn't viable, as there's no post-render event (only a before-render event) for streams.
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.
<tbody id="posts_table">
<% @posts.each do |post| %>
<%= render 'post_row', post: post %>
<% end %>
</tbody>
And in controller:
def create
@post = Post.new(post_params)
respond_to do |format|
if @post.save
format.turbo_stream do
render turbo_stream: turbo_stream.append("posts_table", partial: "posts/post_row", locals: { post: @post })
end
end
end
end
With other actions u shouldn't have problems, I think only append and prepend are the issue.
For replace, remove etc. you can just wrap <table>
in turbo frame.
This is what @inopinatus said, putting for someone who is new to hotwire.
<tbody id="posts_table"> <% @posts.each do |post| %> <%= render 'post_row', post: post %> <% end %> </tbody>
And in controller:
def create @post = Post.new(post_params) respond_to do |format| if @post.save format.turbo_stream do render turbo_stream: turbo_stream.append("posts_table", partial: "posts/post_row", locals: { post: @post }) end end end end
With other actions u shouldn't have problems, I think only append and prepend are the issue. For replace, remove etc. you can just wrap
<table>
in turbo frame.
You can also have a unique ID on each table row <tr id="unique">
and have it replaced
<turbo-stream action="replace" target="unique">
<template>
<tr>....new content</tr>
</template>
</turbo-stream>
turbo_stream.replace "unique", partial: "path/index" # view that contains the table row with same id
This solution is worked for me for CRUD operation now i can insert, delete and update with updated view without refreshing page.
<%= turbo_stream_from "rooms" %> <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>
<tr id="room_<%=room.id%>" is="turbo-frame"> <td> <%= room.name %></td> <td> <%= room.key %></td> <td> <%= room.user.name %></td> </tr>
@hannan228 Could you elaborate? Do the is="turbo-frame"
attributes make your tbody
and tr
act like turbo-frame
elements?
@manuelmeurer yes, By adding a unique id and is="turbo-frame"
to the row tag, it can be used as a Turbo-frame element.
@hannan228
I tried this, but it didn't work for me
in create.turbo_stream.erb
<%= turbo_stream.prepend "table-body" do %>
<%= render partial: 'incomes/income', locals: { income: @income } %>
<% end %>
in index.html.erb
<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 is='turbo-frame'
solution wasn't working for me.
FWIW, the missing piece for me was adding turbo_stream: true
to the edit button:
<%= button_to t('edit'), edit_something_path(something), method: :get, data: { turbo_stream: true } %>
Then, I could define app/views/something/edit.turbo_stream.erb
to replace the row. For me, that looked like
<%= 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>
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: