From d729cf59f975fd2f7486fdf1f0854d0b9afa632c Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Thu, 4 Dec 2025 16:14:52 -0800 Subject: [PATCH] Beautify board watchers list (#1946) * Beautify board_watchers_list by escaping the concat jungle * Correct it and clean it * Latest Rails to get the content tag fix * Test against original collection --- Gemfile.lock | 8 ++--- app/helpers/accesses_helper.rb | 59 +++++++++------------------------- app/helpers/avatars_helper.rb | 4 +++ 3 files changed, 24 insertions(+), 47 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index e24a38a82..b9c3489b5 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -6,7 +6,7 @@ GIT GIT remote: https://github.com/rails/rails.git - revision: b22cb0c0cb39b103d816a66d560991acf0a57163 + revision: 3c3f6c8a253c8a0f346695374f927c9ab32fbefb branch: main specs: actioncable (8.2.0.alpha) @@ -216,7 +216,7 @@ GEM actionview (>= 7.0.0) activesupport (>= 7.0.0) jmespath (1.6.2) - json (2.16.0) + json (2.17.1) jwt (3.1.2) base64 kamal (2.9.0) @@ -351,7 +351,7 @@ GEM nokogiri (>= 1.15.7, != 1.16.7, != 1.16.6, != 1.16.5, != 1.16.4, != 1.16.3, != 1.16.2, != 1.16.1, != 1.16.0.rc1, != 1.16.0) rainbow (3.1.1) rake (13.3.1) - rdoc (6.16.0) + rdoc (6.16.1) erb psych (>= 4.0.0) tsort @@ -443,7 +443,7 @@ GEM ostruct stimulus-rails (1.3.4) railties (>= 6.0.0) - stringio (3.1.8) + stringio (3.1.9) thor (1.4.0) thruster (0.1.16) thruster (0.1.16-aarch64-linux) diff --git a/app/helpers/accesses_helper.rb b/app/helpers/accesses_helper.rb index 2d1b5522e..6ba7b4d3c 100644 --- a/app/helpers/accesses_helper.rb +++ b/app/helpers/accesses_helper.rb @@ -1,6 +1,4 @@ module AccessesHelper - MAX_DISPLAYED_WATCHERS = 8 - def access_menu_tag(board, **options, &) tag.menu class: [ options[:class], { "toggler--toggled": board.all_access? } ], data: { controller: "filter toggle-class navigable-list", @@ -27,45 +25,28 @@ module AccessesHelper end def board_watchers_list(board) - watchers = board.watchers.with_avatars + watchers = board.watchers.with_avatars.load - displayed_watchers = watchers.limit(MAX_DISPLAYED_WATCHERS) - overflow_count = watchers.count - MAX_DISPLAYED_WATCHERS + displayed_watchers = watchers.first(8) + overflow_count = watchers.size - 8 - safe_join([ - tag.strong(displayed_watchers.any? ? "Watching for new cards" : "No one is watching for new cards", class: "txt-uppercase"), - tag.div(class: "board-tools__watching") do - safe_join([ - safe_join(displayed_watchers.map { |watcher| avatar_tag(watcher) }), - (tag.div(data: { controller: "dialog", action: "keydown.esc->dialog#close click@document->dialog#closeOnClickOutside" }) do - concat tag.button("+#{overflow_count}", class: "overflow-count btn btn--circle borderless", data: { action: "dialog#open" }, aria: { label: "Show #{overflow_count} more watchers" }) - concat(tag.dialog(class: "board-tools__watching-dialog dialog panel", data: { dialog_target: "dialog" }, aria: { hidden: "true" }) do - safe_join(watchers.map { |watcher| avatar_tag(watcher) }) - end) - end if overflow_count > 0) - ].compact) - end - ]) + tag.strong(watchers.any? ? "Watching for new cards" : "No one is watching for new cards", class: "txt-uppercase") + + tag.div(avatar_tags(displayed_watchers), class: "board-tools__watching") do + tag.div(data: { controller: "dialog", action: "keydown.esc->dialog#close click@document->dialog#closeOnClickOutside" }) do + tag.button("+#{overflow_count}", class: "overflow-count btn btn--circle borderless", data: { action: "dialog#open" }, aria: { label: "Show #{overflow_count} more watchers" }) + + tag.dialog(avatar_tags(watchers), class: "board-tools__watching-dialog dialog panel", data: { dialog_target: "dialog" }, aria: { hidden: "true" }) + end if overflow_count > 0 + end end def involvement_button(board, access, show_watchers, icon_only) - involvement_label_id = dom_id(board, :involvement_label) - button_to( - board_involvement_path(board), - method: :put, - aria: { labelledby: involvement_label_id }, - title: involvement_access_label(access), - class: class_names("btn", { "btn--reversed": access.watching? && icon_only }), - params: { show_watchers: show_watchers, involvement: next_involvement(access.involvement), icon_only: icon_only } - ) do - safe_join([ - icon_tag("notification-bell-#{icon_only ? 'reverse-' : nil}#{access.involvement.dasherize}"), - tag.span( - class: class_names("txt-nowrap txt-uppercase", "for-screen-reader": icon_only), - id: involvement_label_id - ) - ]) + board_involvement_path(board), method: :put, + params: { show_watchers: show_watchers, involvement: next_involvement(access.involvement), icon_only: icon_only }, + aria: { labelledby: dom_id(board, :involvement_label) }, title: access.access_only? ? "Watch this" : "Stop watching", + class: class_names("btn", { "btn--reversed": access.watching? && icon_only })) do + icon_tag("notification-bell-#{icon_only ? 'reverse-' : nil}#{access.involvement.dasherize}") + + tag.span(class: class_names("txt-nowrap txt-uppercase", "for-screen-reader": icon_only), id: dom_id(board, :involvement_label)) end end @@ -74,12 +55,4 @@ module AccessesHelper order = %w[ watching access_only ] order[(order.index(involvement.to_s) + 1) % order.size] end - - def involvement_access_label(access) - if access.access_only? - "Watch this" - else - "Stop watching" - end - end end diff --git a/app/helpers/avatars_helper.rb b/app/helpers/avatars_helper.rb index 8fb42448a..7c064a650 100644 --- a/app/helpers/avatars_helper.rb +++ b/app/helpers/avatars_helper.rb @@ -19,6 +19,10 @@ module AvatarsHelper end end + def avatar_tags(users, **options) + users.collect { avatar_tag(it, **options) }.join.html_safe + end + def mail_avatar_tag(user, size: 48, **options) if user.avatar.attached? image_tag user_avatar_url(user), alt: user.name, class: "avatar", size: size, **options