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

@@ -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)

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

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