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

Use page refreshes #4

Draft
wants to merge 2 commits into
base: initial
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions app/models/project.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ class Project < ApplicationRecord

validates :name, presence: true

broadcasts_refreshes

def completed?
tasks.pending.none?
end
Expand Down
2 changes: 1 addition & 1 deletion app/models/task.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
class Task < ApplicationRecord
belongs_to :project
belongs_to :project, touch: true

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does Hey really use callbacks/touch like this in real apps, or is it only for the sake of example?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, they most likely do, since it's DRY and allows you to quickly invalidate caches as well, which they use heavily.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I can confirm we do!


scope :completed, -> { where(completed: true) }
scope :pending, -> { where(completed: false) }
Expand Down
3 changes: 3 additions & 0 deletions app/views/layouts/application.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
<%= stylesheet_link_tag "application", "data-turbo-track": "reload" %>
<link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/bulma@0.9.4/css/bulma.min.css">
<%= javascript_importmap_tags %>

<%= turbo_refreshes_with method: :morph, scroll: :preserve %>
Copy link

@bradgessler bradgessler Nov 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm experimenting with Turbo 8 via Phlex components and was surprised to see that the meta tags all use the provide :head construct instead of just emitting a tag.

If you want to make the tags accessible from outside of the content view, you might either eliminate the provide :head construct or keep that and expand out the methods like this:

  # Pages that are more likely than not to be a cache miss can skip turbo cache to avoid visual jitter.
  # Cannot be used along with +turbo_exempts_page_from_preview+.
  def turbo_exempts_page_from_cache
    provide :head, turbo_exempts_page_from_cache_tag
  end

  def turbo_exempts_page_from_cache_tag
    tag.meta(name: "turbo-cache-control", content: "no-cache")
  end

I think the reason you want to keep the provide :head tag is because the meta tags within each view file needs to put the meta tag in the head? The approach I provided above should serve both needs.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's what I ended up doing in my Phlex layout:

        # Turbo 8 has the helper method `turbo_refreshes_with method: :morph, scroll: :preserve` that
        # inserts the content in the `head` tag. Since Phlex doesn't work that way, we'll just put the tags here.
        meta(name: "turbo-refresh-method", content: "morph")
        meta(name: "turbo-refresh-scroll", content: "preserve")

Not a huge deal, but it would be nice to go through the logic that checks for argument validity if the tags are broken apart.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bradgessler from Chicago / Poll Everywhere. Funny running into you on a random PR comment thread. :)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@krschacht speaking to you live from CA, and running into similar ergonomic issues that you're running into.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened a PR at hotwired/turbo-rails#542 that decouples tag generation from the way it's served. We can continue this discussion over there.

<%= yield :head %>
</head>

<body>
Expand Down
4 changes: 4 additions & 0 deletions app/views/projects/index.html.erb
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
<% @projects.each do |project| %>
<%= turbo_stream_from project %>
<% end %>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jorgemanrubia When I open the projects index (http://localhost:3000/projects) in two separate browser windows, and in one of them I create a new project, the other one does not update with the new project. I have to manually refresh the page to see it.

I assume it's because of this block here ^, it only subscribes to the specific projects which exist at the time the page loads so it's not subscribed to a newly created project. One solution would be to introduce a new higher-level abstraction called Portfolio (a collection of projects) and broadcast refreshes from that. But that seems heavy-handed if we don't otherwise need a new model.

Is there a simpler solution to this within the Turbo 8 Morphing world?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Check this PR hotwired/turbo-rails#521. You can use the pluralized model name as the stream name for listening to additions.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But would this also work when we have scopes in place? For example like:

  <%= turbo_stream_from @projects.not_deleted.is_active.where(client_id: 1) %>

Or would it just subscribe to all the projects?


<h1 class="title">
You have <span><%= @projects.count %> </span> projects:
</h1>
Expand Down
2 changes: 2 additions & 0 deletions app/views/projects/show.html.erb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
<%= turbo_stream_from @project %>

<nav class="breadcrumb" aria-label="breadcrumbs">
<ul>
<li>
Expand Down