From 10cdb8c1a83a41529f3a4c8b6912c26a06cf900f Mon Sep 17 00:00:00 2001
From: Bruno Prieto <bruno@37signals.com>
Date: Fri, 14 Mar 2025 02:29:44 +0100
Subject: [PATCH] Improve accessibility of comments and reactions

---
 app/helpers/avatars_helper.rb                   |  6 ++++--
 app/helpers/messages_helper.rb                  |  1 +
 .../controllers/reaction_delete_controller.js   | 12 +++++++-----
 app/views/comments/_body.html.erb               | 14 +++++++++-----
 app/views/comments/_comment.html.erb            |  6 +++---
 app/views/comments/reactions/_reaction.html.erb | 17 +++++++++--------
 .../comments/reactions/_reactions.html.erb      |  4 ++--
 app/views/comments/reactions/new.html.erb       | 11 ++++++-----
 8 files changed, 41 insertions(+), 30 deletions(-)

diff --git a/app/helpers/avatars_helper.rb b/app/helpers/avatars_helper.rb
index a3ebd619a..2a6d927f4 100644
--- a/app/helpers/avatars_helper.rb
+++ b/app/helpers/avatars_helper.rb
@@ -10,8 +10,10 @@ module AvatarsHelper
     AVATAR_COLORS[Zlib.crc32(user.to_param) % AVATAR_COLORS.size]
   end
 
-  def avatar_tag(user, **options)
-    link_to user_path(user), title: user.name, class: "btn avatar", data: { turbo_frame: "_top" } do
+  def avatar_tag(user, hidden_for_screen_reader: false, **options)
+    link_to user_path(user), title: user.name, class: "btn avatar", data: { turbo_frame: "_top" },
+        aria: { hidden: hidden_for_screen_reader, label: user.name },
+        tabindex: hidden_for_screen_reader ? -1 : nil do
       avatar_image_tag(user, **options)
     end
   end
diff --git a/app/helpers/messages_helper.rb b/app/helpers/messages_helper.rb
index 0fc940b37..bac9af789 100644
--- a/app/helpers/messages_helper.rb
+++ b/app/helpers/messages_helper.rb
@@ -3,6 +3,7 @@ module MessagesHelper
     turbo_frame_tag dom_id(bubble, :messages),
       class: "comments align-center center borderless margin flex flex-column gap-half",
       style: "--bubble-color: <%= bubble.color %>",
+      role: "group", aria: { label: "Messages" },
       data: {
         controller: "created-by-current-user",
         created_by_current_user_mine_class: "comment--mine"
diff --git a/app/javascript/controllers/reaction_delete_controller.js b/app/javascript/controllers/reaction_delete_controller.js
index f4672d693..3c89ec00d 100644
--- a/app/javascript/controllers/reaction_delete_controller.js
+++ b/app/javascript/controllers/reaction_delete_controller.js
@@ -6,14 +6,15 @@ export default class extends Controller {
   static values = { reacterId: Number }
 
   connect() {
-    if (this.#currentUserIsreacter) {
+    if (this.#currentUserIsReacter) {
       this.#setAccessibleAttributes()
     }
   }
 
   reveal() {
-    if (this.#currentUserIsreacter) {
+    if (this.#currentUserIsReacter) {
       this.element.classList.toggle(this.revealClass)
+      this.contentTarget.ariaExpanded = this.element.classList.contains(this.revealClass)
       this.buttonTarget.focus()
     }
   }
@@ -23,11 +24,12 @@ export default class extends Controller {
   }
 
   #setAccessibleAttributes() {
-    this.contentTarget.setAttribute('tabindex', '0')
-    this.contentTarget.setAttribute('aria-describedby', 'delete_reaction_accessible_label')
+    this.contentTarget.role = "button"
+    this.contentTarget.tabIndex = 0
+    this.contentTarget.ariaExpanded = false
   }
 
-  get #currentUserIsreacter() {
+  get #currentUserIsReacter() {
     return Current.user.id === this.reacterIdValue
   }
 }
diff --git a/app/views/comments/_body.html.erb b/app/views/comments/_body.html.erb
index 14b976386..c9cebe0ff 100644
--- a/app/views/comments/_body.html.erb
+++ b/app/views/comments/_body.html.erb
@@ -2,12 +2,16 @@
   <%= turbo_frame_tag dom_id(comment) do %>
     <div class="comment__content flex-inline flex-column full-width border-radius">
       <div class="comment__author flex align-center gap-half">
-        <strong><%= link_to comment.creator.name, user_path(comment.creator), class: "txt-ink btn btn--plain fill-transparent", data: { turbo_frame: "_top" } %></strong>
-        <%= link_to bucket_bubble_path(comment.bubble.bucket, comment.bubble, anchor: "comment_#{comment.id}"), class: "txt-undecorated" do %>
-          <%= local_datetime_tag comment.created_at, style: :shortdate, class: "comment__timestamp" %>
-        <% end %>
+        <h3 contents>
+          <strong><%= link_to comment.creator.name, user_path(comment.creator), class: "txt-ink btn btn--plain fill-transparent", data: { turbo_frame: "_top" } %></strong>
+          <%= link_to bucket_bubble_path(comment.bubble.bucket, comment.bubble, anchor: "comment_#{comment.id}"), class: "txt-undecorated" do %>
+            <%= local_datetime_tag comment.created_at, style: :shortdate, class: "comment__timestamp" %>
+          <% end %>
+        </h3>
 
-        <%= link_to edit_bucket_bubble_comment_path(comment.bubble.bucket, comment.bubble, comment), class: "btn txt-small btn--plain fill-transparent comment__edit", style: "font-size: 0.4em; opacity: 0.5;" do %>
+        <%= link_to edit_bucket_bubble_comment_path(comment.bubble.bucket, comment.bubble, comment), role: "button",
+              class: "btn txt-small btn--plain fill-transparent comment__edit",
+              style: "font-size: 0.4em; opacity: 0.5;" do %>
           <%= image_tag "menu-dots-horizontal.svg", class: "icon colorize--black" %>
           <span class="for-screen-reader">Edit</span>
         <% end %>
diff --git a/app/views/comments/_comment.html.erb b/app/views/comments/_comment.html.erb
index 932a5095b..4bbe0d96b 100644
--- a/app/views/comments/_comment.html.erb
+++ b/app/views/comments/_comment.html.erb
@@ -1,7 +1,7 @@
 <%= comment_tag comment do %>
-  <figure class="comment__avatar flex-item-no-shrink">
-    <%= avatar_tag comment.creator, loading: :lazy %>
+  <figure class="comment__avatar flex-item-no-shrink" aria-hidden="true">
+    <%= avatar_tag comment.creator, hidden_for_screen_reader: true, loading: :lazy %>
   </figure>
 
   <%= render "comments/body", comment: comment %>
-<% end %>
\ No newline at end of file
+<% end %>
diff --git a/app/views/comments/reactions/_reaction.html.erb b/app/views/comments/reactions/_reaction.html.erb
index 4004725b3..84d5f9b99 100644
--- a/app/views/comments/reactions/_reaction.html.erb
+++ b/app/views/comments/reactions/_reaction.html.erb
@@ -5,14 +5,16 @@
         data-reaction-delete-reacter-id-value="<%= reaction.reacter.id %>"
         data-controller="reaction-delete" data-reaction-delete-perform-class="reaction--deleting" 
         data-reaction-delete-reveal-class="expanded" data-reaction-delete-reacter-id-value="<%= reaction.reacter.id %>">
-    <figure class="reaction__avatar margin-none flex-item-no-shrink">
-      <%= avatar_tag reaction.reacter, aria: { label: "#{reaction.reacter.name} reacted #{reaction.content}" }, loading: :lazy %>
+    <figure class="reaction__avatar margin-none flex-item-no-shrink" aria-hidden="true">
+      <%= avatar_tag reaction.reacter, hidden_for_screen_reader: true, loading: :lazy %>
     </figure>
 
-    <%= tag.span reaction.content, role: "button",
-          class: [ "txt-small", { "txt-medium": reaction.all_emoji? } ],
-          data: { action: "click->reaction-delete#reveal keydown.enter->reaction-delete#reveal:prevent", reaction_delete_target: "content" } %>
-    
+    <%= tag.span reaction.content, class: [ "txt-small", { "txt-medium": reaction.all_emoji? } ],
+          data: {
+            action: "click->reaction-delete#reveal keydown.enter->reaction-delete#reveal:prevent",
+            reaction_delete_target: "content" } do %>
+      <span class="for-screen-reader"><%= "#{reaction.reacter.name} reacted #{reaction.content}" %></span>
+    <% end %>
     <%= button_to bucket_bubble_comment_reaction_path(comment.bubble.bucket, comment.bubble, comment, reaction),
           method: :delete,
           class: "btn btn--negative flex-item-justify-end reaction__delete",
@@ -21,5 +23,4 @@
       <span class="for-screen-reader">Delete this reaction</span>
     <% end %>
   </div>
-  <span id="delete_reaction_accessible_label" class="for-screen-reader">Press enter to delete this reaction</span>
-<% end %> 
\ No newline at end of file
+<% end %> 
diff --git a/app/views/comments/reactions/_reactions.html.erb b/app/views/comments/reactions/_reactions.html.erb
index 9f4fe4b4b..67bee5e9c 100644
--- a/app/views/comments/reactions/_reactions.html.erb
+++ b/app/views/comments/reactions/_reactions.html.erb
@@ -7,7 +7,7 @@
     
     <%= turbo_frame_tag comment, :new_reaction do %>
       <div class="flex-inline" data-controller="soft-keyboard">
-        <%= link_to new_bucket_bubble_comment_reaction_path(comment.bubble.bucket, comment.bubble, comment), 
+        <%= link_to new_bucket_bubble_comment_reaction_path(comment.bubble.bucket, comment.bubble, comment), role: "button",
               class: "txt-small btn reaction__action", action: "soft-keyboard#open" do %>
           <%= image_tag "reaction.svg", size: 20, aria: { hidden: "true" } %>
           <span class="for-screen-reader">Add a reaction</span>
@@ -15,4 +15,4 @@
       </div>
     <% end %>
   </div> 
-<% end %>
\ No newline at end of file
+<% end %>
diff --git a/app/views/comments/reactions/new.html.erb b/app/views/comments/reactions/new.html.erb
index e2ca711d3..e22cd1db8 100644
--- a/app/views/comments/reactions/new.html.erb
+++ b/app/views/comments/reactions/new.html.erb
@@ -1,14 +1,15 @@
 <%= turbo_frame_tag dom_id(@comment, :new_reaction) do %>
   <%= form_with url: bucket_bubble_comment_reactions_path(@comment.bubble.bucket, @comment.bubble, @comment), 
         class: "reaction reaction__form flex-inline postion--relative max-width align-center fill-white gap expanded",
+        html: { aria: { label: "New reaction" } },
         data: { controller: "form", turbo_frame: dom_id(@comment, :reacting), action: "keydown.esc->form#cancel" } do |form| %>
-    <label class="reaction__form-label flex gap" style="--column-gap: 0.7ch;" role="button" tabindex="0" aria-label="Add a reaction">
+    <label class="reaction__form-label flex gap" style="--column-gap: 0.7ch;">
       <figure class="reaction__avatar margin-none flex-item-no-shrink">
         <%= avatar_tag Current.user %>
-        <span class="for-screen-reader"><%= Current.user.name %></span>
       </figure>
       <%= form.text_field :content, name: "reaction[content]", autofocus: true, autocomplete: "off", autocorrect: "off", maxlength: 16,
-              required: true, pattern: /\S+.*/, class: "input reaction__input txt-small" %>
+            required: true, pattern: /\S+.*/, class: "input reaction__input txt-small",
+            aria: { label: "Add a reaction" } %>
     </label>
 
     <%= form.button class: "btn btn--reversed", type: "submit" do %>
@@ -16,10 +17,10 @@
       <span class="for-screen-reader">Submit</span>
     <% end %>
 
-    <%= link_to bucket_bubble_comment_reactions_path(@comment.bubble.bucket, @comment.bubble, @comment), 
+    <%= link_to bucket_bubble_comment_reactions_path(@comment.bubble.bucket, @comment.bubble, @comment), role: "button",
           data: { turbo_frame: dom_id(@comment, :reactions), form_target: "cancel" }, class: "btn btn--negative" do %>
       <%= image_tag "minus.svg", aria: { hidden: "true" } %>
       <span class="for-screen-reader">Cancel</span>
     <% end %>
   <% end %>
-<% end %>
\ No newline at end of file
+<% end %>