make the index a table again #457
Conversation
| <tr class="border-b"> | ||
| <% attributes.reject(&:password_digest?).each do |attribute| -%> | ||
| <td class="py-4 whitespace-nowrap"><%%= <%= singular_name %>.<%= attribute.column_name %> %></td> | ||
| <% end -%> | ||
| <td class="py-4 space-x-4 whitespace-nowrap"> | ||
| <%%= link_to "Show", <%= model_resource_name(singular_table_name) %>, class: "text-blue-600 hover:text-blue-900" %> | ||
| </td> | ||
| </tr> |
There was a problem hiding this comment.
we can still have this in a partial but i don't see the benefit - i don't think it'll be used somewhere else.
There was a problem hiding this comment.
I think it's fine to have it inline in this PR, if a need emerges for a partial it's easy to extract later.
| <td class="py-4 whitespace-nowrap"><%%= <%= singular_name %>.<%= attribute.column_name %> %></td> | ||
| <% end -%> | ||
| <td class="py-4 space-x-4 whitespace-nowrap"> | ||
| <%%= link_to "Show", <%= model_resource_name(singular_table_name) %>, class: "text-blue-600 hover:text-blue-900" %> |
There was a problem hiding this comment.
I kept just the link to the show action, but we can also add the Edit and Destroy links. thoughts?
There was a problem hiding this comment.
I would personally like to have the Edit and Destroy links in the table, but I suppose we could do that in a separate PR if you're unsure.
There was a problem hiding this comment.
I think it's a nice addition - added!
patriciomacadden
force-pushed
the
index-table
branch
from
c881254 to
91e8705
Compare
| </tr> | ||
| </thead> | ||
| <tbody class="divide-y divide-gray-200"> | ||
| <%% @users.each do |user| %> |
There was a problem hiding this comment.
This needs to be @<%= plural_table_name %>.each
| <%% @users.each do |user| %> | ||
| <tr class="border-b"> | ||
| <% attributes.reject(&:password_digest?).each do |attribute| -%> | ||
| <td class="py-4 whitespace-nowrap"><%%= <%= singular_name %>.<%= attribute.column_name %> %></td> |
There was a problem hiding this comment.
The partial has logic to handle rendering links to attachments, which we should probably mirror.
There was a problem hiding this comment.
good catch - added!
| <tr class="border-b"> | ||
| <% attributes.reject(&:password_digest?).each do |attribute| -%> | ||
| <td class="py-4 whitespace-nowrap"><%%= <%= singular_name %>.<%= attribute.column_name %> %></td> | ||
| <% end -%> | ||
| <td class="py-4 space-x-4 whitespace-nowrap"> | ||
| <%%= link_to "Show", <%= model_resource_name(singular_table_name) %>, class: "text-blue-600 hover:text-blue-900" %> | ||
| </td> | ||
| </tr> |
There was a problem hiding this comment.
I think it's fine to have it inline in this PR, if a need emerges for a partial it's easy to extract later.
…dcoded vars, add edit and destroy actions to table rows
|
thanks for your review @flavorjones, all issues addressed! :) |
|
I'm having second thoughts on converting to a table. I do think it's the proper way to implement a CRUD (and it's always been in Rails) but now it's not (see this comment in the PR that originated the current design). But also I'd like the scaffold generator to generate something ready to use without too many modifications. I'm closing this PR, and I'll see how can we improve the list view. Feel free to reopen if you think it's good, though. |
As per this PR, the scaffold generator stopped generating a table in the index page, and encourages css frameworks gems (such as this) to do the styling.
I don't quite like what the generator generates currently (design wise, although I'm no designer), so I thought of bringing back the table, with a basic style so it's ready to use (or as ready to use as possible).
I looked for box-style CRUD designs so we could keep using the partial but I couldn't find anything.
As always, before and after pictures: