From fc0cf3917f016e0fe00fc7c340ed1d4dff262b7f Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Thu, 17 Apr 2025 14:05:26 +0200 Subject: [PATCH 1/7] Pin without redirect Use turbo stream response to save on the redirect latency --- app/controllers/cards/pins_controller.rb | 4 ---- app/views/cards/pins/_pin.html.erb | 11 +++++++++++ app/views/cards/pins/create.turbo_stream.erb | 1 + app/views/cards/pins/destroy.turbo_stream.erb | 1 + app/views/cards/pins/show.html.erb | 12 +----------- test/controllers/cards/pins_controller_test.rb | 8 ++++---- 6 files changed, 18 insertions(+), 19 deletions(-) create mode 100644 app/views/cards/pins/_pin.html.erb create mode 100644 app/views/cards/pins/create.turbo_stream.erb create mode 100644 app/views/cards/pins/destroy.turbo_stream.erb diff --git a/app/controllers/cards/pins_controller.rb b/app/controllers/cards/pins_controller.rb index 1f6c1aaa8..c6317f095 100644 --- a/app/controllers/cards/pins_controller.rb +++ b/app/controllers/cards/pins_controller.rb @@ -6,16 +6,12 @@ class Cards::PinsController < ApplicationController def create pin = @card.pin_by Current.user - broadcast_my_new pin - redirect_to card_pin_path(@card) end def destroy pin = @card.unpin_by Current.user - broadcast_my_removed pin - redirect_to card_pin_path(@card) end private diff --git a/app/views/cards/pins/_pin.html.erb b/app/views/cards/pins/_pin.html.erb new file mode 100644 index 000000000..e08308829 --- /dev/null +++ b/app/views/cards/pins/_pin.html.erb @@ -0,0 +1,11 @@ +
+ <% if card.pinned_by? Current.user %> + <%= button_to card_pin_path(card), method: :delete, class: "btn btn--reversed" do %> + <%= icon_tag "pinned" %> Un-pin this card + <% end %> + <% else %> + <%= button_to card_pin_path(card), class: "btn" do %> + <%= icon_tag "unpinned" %> Pin this card + <% end %> + <% end %> +
diff --git a/app/views/cards/pins/create.turbo_stream.erb b/app/views/cards/pins/create.turbo_stream.erb new file mode 100644 index 000000000..752fc594d --- /dev/null +++ b/app/views/cards/pins/create.turbo_stream.erb @@ -0,0 +1 @@ +<%= turbo_stream.replace [ @card, :pin_button ], partial: "cards/pins/pin", locals: { card: @card } %> diff --git a/app/views/cards/pins/destroy.turbo_stream.erb b/app/views/cards/pins/destroy.turbo_stream.erb new file mode 100644 index 000000000..752fc594d --- /dev/null +++ b/app/views/cards/pins/destroy.turbo_stream.erb @@ -0,0 +1 @@ +<%= turbo_stream.replace [ @card, :pin_button ], partial: "cards/pins/pin", locals: { card: @card } %> diff --git a/app/views/cards/pins/show.html.erb b/app/views/cards/pins/show.html.erb index 64eb1ad5c..0343f1b68 100644 --- a/app/views/cards/pins/show.html.erb +++ b/app/views/cards/pins/show.html.erb @@ -1,13 +1,3 @@ <%= turbo_frame_tag dom_id(@card, :pin) do %> - <% if @card.pinned_by? Current.user %> - <%= button_to card_pin_path(@card), method: :delete, class: "btn btn--reversed" do %> - <%= icon_tag "pinned" %> - Un-pin this card - <% end %> - <% else %> - <%= button_to card_pin_path(@card), class: "btn" do %> - <%= icon_tag "unpinned" %> - Pin this card - <% end %> - <% end %> + <%= render "cards/pins/pin", card: @card %> <% end %> diff --git a/test/controllers/cards/pins_controller_test.rb b/test/controllers/cards/pins_controller_test.rb index 39b12a053..82f0534c5 100644 --- a/test/controllers/cards/pins_controller_test.rb +++ b/test/controllers/cards/pins_controller_test.rb @@ -9,23 +9,23 @@ class Cards::PinsControllerTest < ActionDispatch::IntegrationTest assert_changes -> { cards(:layout).pinned_by?(users(:kevin)) }, from: false, to: true do perform_enqueued_jobs do assert_turbo_stream_broadcasts([ users(:kevin), :pins ], count: 1) do - post card_pin_path(cards(:layout)) + post card_pin_path(cards(:layout)), as: :turbo_stream end end end - assert_redirected_to card_pin_path(cards(:layout)) + assert_response :success end test "destroy" do assert_changes -> { cards(:shipping).pinned_by?(users(:kevin)) }, from: true, to: false do perform_enqueued_jobs do assert_turbo_stream_broadcasts([ users(:kevin), :pins ], count: 1) do - delete card_pin_path(cards(:shipping)) + delete card_pin_path(cards(:shipping)), as: :turbo_stream end end end - assert_redirected_to card_pin_path(cards(:shipping)) + assert_response :success end end From bddd624e412c7ef7f3168d6918d20c6917b697a3 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Thu, 17 Apr 2025 14:14:03 +0200 Subject: [PATCH 2/7] Match template name to container id --- app/views/cards/pins/{_pin.html.erb => _pin_button.html.erb} | 0 app/views/cards/pins/create.turbo_stream.erb | 2 +- app/views/cards/pins/show.html.erb | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) rename app/views/cards/pins/{_pin.html.erb => _pin_button.html.erb} (100%) diff --git a/app/views/cards/pins/_pin.html.erb b/app/views/cards/pins/_pin_button.html.erb similarity index 100% rename from app/views/cards/pins/_pin.html.erb rename to app/views/cards/pins/_pin_button.html.erb diff --git a/app/views/cards/pins/create.turbo_stream.erb b/app/views/cards/pins/create.turbo_stream.erb index 752fc594d..b1dd787e9 100644 --- a/app/views/cards/pins/create.turbo_stream.erb +++ b/app/views/cards/pins/create.turbo_stream.erb @@ -1 +1 @@ -<%= turbo_stream.replace [ @card, :pin_button ], partial: "cards/pins/pin", locals: { card: @card } %> +<%= turbo_stream.replace [ @card, :pin_button ], partial: "cards/pins/pin_button", locals: { card: @card } %> diff --git a/app/views/cards/pins/show.html.erb b/app/views/cards/pins/show.html.erb index 0343f1b68..c8ac69d7e 100644 --- a/app/views/cards/pins/show.html.erb +++ b/app/views/cards/pins/show.html.erb @@ -1,3 +1,3 @@ <%= turbo_frame_tag dom_id(@card, :pin) do %> - <%= render "cards/pins/pin", card: @card %> + <%= render "cards/pins/pin_button", card: @card %> <% end %> From 8fe82410690914800f620a9a5f7be8e7cdee4e31 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Thu, 17 Apr 2025 14:14:08 +0200 Subject: [PATCH 3/7] Reuse create template --- app/views/cards/pins/destroy.turbo_stream.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/cards/pins/destroy.turbo_stream.erb b/app/views/cards/pins/destroy.turbo_stream.erb index 752fc594d..d54dea5f1 100644 --- a/app/views/cards/pins/destroy.turbo_stream.erb +++ b/app/views/cards/pins/destroy.turbo_stream.erb @@ -1 +1 @@ -<%= turbo_stream.replace [ @card, :pin_button ], partial: "cards/pins/pin", locals: { card: @card } %> +<%= render template: "cards/pins/create" %> From 1437bcb3b73b353c4bf276d53294677d29ea00a7 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Thu, 17 Apr 2025 14:29:12 +0200 Subject: [PATCH 4/7] Clearer about where these are being sent --- app/controllers/cards/pins_controller.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/controllers/cards/pins_controller.rb b/app/controllers/cards/pins_controller.rb index c6317f095..2e0d12554 100644 --- a/app/controllers/cards/pins_controller.rb +++ b/app/controllers/cards/pins_controller.rb @@ -6,20 +6,20 @@ class Cards::PinsController < ApplicationController def create pin = @card.pin_by Current.user - broadcast_my_new pin + broadcast_add_to_tray pin end def destroy pin = @card.unpin_by Current.user - broadcast_my_removed pin + broadcast_remove_from_tray pin end private - def broadcast_my_new(pin) + def broadcast_add_to_tray(pin) pin.broadcast_prepend_later_to [ Current.user, :pins ], target: "pins", partial: "my/pins/pin" end - def broadcast_my_removed(pin) + def broadcast_remove_from_tray(pin) pin.broadcast_remove_to [ Current.user, :pins ] end end From 87ce4a623b17628a44364deff8befc50c20b6f74 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Thu, 17 Apr 2025 14:31:02 +0200 Subject: [PATCH 5/7] Even clearer about where these pins are going --- app/controllers/cards/pins_controller.rb | 4 ++-- app/views/my/pins/_tray.html.erb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/cards/pins_controller.rb b/app/controllers/cards/pins_controller.rb index 2e0d12554..68430ff06 100644 --- a/app/controllers/cards/pins_controller.rb +++ b/app/controllers/cards/pins_controller.rb @@ -16,10 +16,10 @@ class Cards::PinsController < ApplicationController private def broadcast_add_to_tray(pin) - pin.broadcast_prepend_later_to [ Current.user, :pins ], target: "pins", partial: "my/pins/pin" + pin.broadcast_prepend_later_to [ Current.user, :pins_tray ], target: "pins", partial: "my/pins/pin" end def broadcast_remove_from_tray(pin) - pin.broadcast_remove_to [ Current.user, :pins ] + pin.broadcast_remove_to [ Current.user, :pins_tray ] end end diff --git a/app/views/my/pins/_tray.html.erb b/app/views/my/pins/_tray.html.erb index 70c1d5a16..ddd39fdfe 100644 --- a/app/views/my/pins/_tray.html.erb +++ b/app/views/my/pins/_tray.html.erb @@ -1,4 +1,4 @@ -<%= turbo_stream_from Current.user, :pins %> +<%= turbo_stream_from Current.user, :pins_tray %> <%= tag.dialog id: "pin-tray", class: "tray pin-tray", data: { controller: "dialog", turbo_permanent: true, dialog_modal_value: false, dialog_target: "dialog", action: "keydown.esc->dialog#close:stop click@document->dialog#closeOnClickOutside" } do %> From 4d0dddec46c09b96b402805e30a96cf0e0f771eb Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Thu, 17 Apr 2025 14:31:28 +0200 Subject: [PATCH 6/7] No need to loop through a job when this is the sole point of the action --- app/controllers/cards/pins_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/cards/pins_controller.rb b/app/controllers/cards/pins_controller.rb index 68430ff06..235393178 100644 --- a/app/controllers/cards/pins_controller.rb +++ b/app/controllers/cards/pins_controller.rb @@ -16,7 +16,7 @@ class Cards::PinsController < ApplicationController private def broadcast_add_to_tray(pin) - pin.broadcast_prepend_later_to [ Current.user, :pins_tray ], target: "pins", partial: "my/pins/pin" + pin.broadcast_prepend_to [ Current.user, :pins_tray ], target: "pins", partial: "my/pins/pin" end def broadcast_remove_from_tray(pin) From 7b5a1c0232fda05fbea87b1276bfb407d7e7ccea Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Thu, 17 Apr 2025 14:32:26 +0200 Subject: [PATCH 7/7] Fix test --- test/controllers/cards/pins_controller_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/controllers/cards/pins_controller_test.rb b/test/controllers/cards/pins_controller_test.rb index 82f0534c5..3bc569827 100644 --- a/test/controllers/cards/pins_controller_test.rb +++ b/test/controllers/cards/pins_controller_test.rb @@ -8,7 +8,7 @@ class Cards::PinsControllerTest < ActionDispatch::IntegrationTest test "create" do assert_changes -> { cards(:layout).pinned_by?(users(:kevin)) }, from: false, to: true do perform_enqueued_jobs do - assert_turbo_stream_broadcasts([ users(:kevin), :pins ], count: 1) do + assert_turbo_stream_broadcasts([ users(:kevin), :pins_tray ], count: 1) do post card_pin_path(cards(:layout)), as: :turbo_stream end end @@ -20,7 +20,7 @@ class Cards::PinsControllerTest < ActionDispatch::IntegrationTest test "destroy" do assert_changes -> { cards(:shipping).pinned_by?(users(:kevin)) }, from: true, to: false do perform_enqueued_jobs do - assert_turbo_stream_broadcasts([ users(:kevin), :pins ], count: 1) do + assert_turbo_stream_broadcasts([ users(:kevin), :pins_tray ], count: 1) do delete card_pin_path(cards(:shipping)), as: :turbo_stream end end