From d420d8d8ac34b7709337ac8eaa5279d3fdafca97 Mon Sep 17 00:00:00 2001
From: Jason Zimdars <jz@37signals.com>
Date: Thu, 26 Jun 2025 16:56:27 -0500
Subject: [PATCH] Add keyboard navigation and filtering to quick filters

---
 app/assets/stylesheets/filters.css     |   2 +-
 app/assets/stylesheets/popup.css       |   2 +-
 app/views/filters/_assignees.html.erb  | 108 ++++++++++++++++---------
 app/views/filters/_creators.html.erb   |  90 +++++++++++----------
 app/views/filters/_indexed_by.html.erb |  56 +++++++------
 app/views/filters/_tags.html.erb       |  75 ++++++++++-------
 6 files changed, 194 insertions(+), 139 deletions(-)

diff --git a/app/assets/stylesheets/filters.css b/app/assets/stylesheets/filters.css
index d89ad57f1..45be28002 100644
--- a/app/assets/stylesheets/filters.css
+++ b/app/assets/stylesheets/filters.css
@@ -98,7 +98,7 @@
   }
 
   .quick-filter {
-    &:has(input:not(.default-value):checked) {
+    &:not(.default-value):has(.checked) {
       .input--select {
         --input-background: var(--color-selected);
       }
diff --git a/app/assets/stylesheets/popup.css b/app/assets/stylesheets/popup.css
index 0d1c27807..addd87f2d 100644
--- a/app/assets/stylesheets/popup.css
+++ b/app/assets/stylesheets/popup.css
@@ -94,7 +94,7 @@
     }
 
     &:has(input:checked)  {
-      --btn-background: var(--color-ink-lightest);
+      --btn-background: transparent;
       --btn-color: var(--color-ink);
 
       .icon {
diff --git a/app/views/filters/_assignees.html.erb b/app/views/filters/_assignees.html.erb
index 9dc4a87a0..1a1e33926 100644
--- a/app/views/filters/_assignees.html.erb
+++ b/app/views/filters/_assignees.html.erb
@@ -5,53 +5,81 @@
         <% if filter.assignment_status.unassigned? %>
           No one
         <% elsif filter.assignees.any? %>
-          Assigned to <%= filter.assignees.map(&:familiar_name).to_sentence %>
+          Assigned to <%= filter.assignees.map(&:familiar_name).to_sentence(two_words_connector: " or ", last_word_connector: ", or ") %>
         <% else %>
           Assigned to…
         <% end %>
       </span>
     </button>
 
-    <dialog class="margin-block-start-half popup panel flex-column align-start gap-half fill-white shadow" data-dialog-target="dialog" data-action="turbo:before-cache@document->dialog#close">
-      <strong class="popup__title margin-block-start-half pad-inline-half overflow-ellipsis">Assigned to…</strong>
-      <%= form_with url: cards_path, method: :get, class: "popup__list",
-            data: { controller: "form" } do |form| %>
-        <% filter.as_params.except(:assignee_ids, :assignment_status).each do |key, value| %>
-          <%= filter_hidden_field_tag key, value %>
-        <% end %>
+    <%= tag.dialog class: "margin-block-start-half popup panel flex-column align-start gap-half fill-white shadow txt-small", data: {
+				action: "turbo:before-cache@document->dialog#close keydown->navigable-list#navigate filter:changed->navigable-list#reset toggle->filter#filter",
+        aria: { label: "Assigned to…", aria_description: "Assigned to…" },
+				controller: "filter navigable-list",
+				dialog_target: "dialog",
+				navigable_list_focus_on_selection_value: false,
+				navigable_list_actionable_items_value: true } do %>
+      <strong class="popup__title txt-nowrap">Assigned to…</strong>
 
-        <% if @expand_all %>
-          <%= hidden_field_tag :expand_all, @expand_all %>
-        <% end %>
-
-        <%= link_to cards_path(filter.as_params.except(:assignee_ids, :assignment_status).merge(expand_all: @expand_all)), class: "btn popup__item" do %>
-          <span class="overflow-ellipsis">Deselect all</span>
-        <% end %>
-
-        <div class="btn popup__item">
-          <%= form.check_box "assignment_status", {
-            checked: filter.assignment_status.unassigned?,
-            data: { action: "change->form#submit" },
-            include_hidden: false,
-          }, "unassigned" %>
-
-          <%= form.label :assignment_status, "No one", class: "overflow-ellipsis" %>
-          <%= icon_tag "check", class: "checked flex-item-justify-end" %>
-        </div>
-
-        <% User.active.order(:name).each do |user| %>
-          <div class="btn popup__item">
-            <%= form.check_box "assignee_ids[]", {
-              checked: filter.assignees.include?(user),
-              data: { action: "change->form#submit" },
-              include_hidden: false,
-            }, user.id %>
-
-            <%= form.label "assignee_ids[]", user.name, for: dom_id(user, :filter), class: "overflow-ellipsis" %>
-            <%= icon_tag "check", class: "checked flex-item-justify-end" %>
-          </div>
-        <% end %>
+      <% if User.active.many? %>
+        <%= text_field_tag :search, nil, placeholder: "Filter…", class: "input input--transparent txt-small", autofocus: true,
+              type: "search", autocorrect: "off", autocomplete: "off", data: { "1p-ignore": "true", filter_target: "input", action: "input->filter#filter" } %>
       <% end %>
-    </dialog>
+
+      <ul class="popup__list" data-filter-target="list">
+        <li class="popup__group flex align-center" data-value="no one" data-filter-target="item" data-navigable-list-target="item">
+          <%= form_with url: cards_path, method: :get, class: "full-width", data: { controller: "form" } do |form| %>
+            <% filter.as_params.except(:assignee_ids, :assignment_status).each do |key, value| %>
+              <%= filter_hidden_field_tag key, value %>
+            <% end %>
+
+            <% unless filter.assignment_status.unassigned? %>
+              <%= hidden_field_tag :assignment_status, "unassigned" %>
+            <% end %>
+
+            <% if @expand_all %>
+              <%= hidden_field_tag :expand_all, @expand_all %>
+            <% end %>
+
+            <%= form.button type: "submit", class: "btn popup__item min-width full-width unpad-inline" do %>
+              <span class="overflow-ellipsis flex-item-grow">No one</span>
+              <% if filter.assignment_status.unassigned? %>
+                <%= icon_tag "check", class: "checked flex-item-justify-end" %>
+              <% end %>
+            <% end %>
+          <% end %>
+        </li>
+        <% User.active.order(:name).each do |user| %>
+          <li class="popup__group flex align-center" data-value="<%= user.name.downcase %>" data-filter-target="item" data-navigable-list-target="item">
+            <%= form_with url: cards_path, method: :get, class: "full-width", data: { controller: "form" } do |form| %>
+              <% filter.as_params.except(:assignee_ids, :assignment_status).each do |key, value| %>
+                <%= filter_hidden_field_tag key, value %>
+              <% end %>
+
+              <% filter.assignees.each do |existing_assignee| %>
+                <% unless existing_assignee == user %>
+                  <%= hidden_field_tag "assignee_ids[]", existing_assignee.id %>
+                <% end %>
+              <% end %>
+
+              <% unless filter.assignees.include?(user) %>
+                <%= hidden_field_tag "assignee_ids[]", user.id %>
+              <% end %>
+
+              <% if @expand_all %>
+                <%= hidden_field_tag :expand_all, @expand_all %>
+              <% end %>
+
+              <%= form.button type: "submit", class: "btn popup__item min-width full-width unpad-inline" do %>
+                <span class="overflow-ellipsis flex-item-grow"><%= user.name %></span>
+                <% if filter.assignees.include?(user) %>
+                  <%= icon_tag "check", class: "checked flex-item-justify-end" %>
+                <% end %>
+              <% end %>
+            <% end %>
+          </li>
+        <% end %>
+      </ul>
+    <% end %>
   </div>
 <% end %>
diff --git a/app/views/filters/_creators.html.erb b/app/views/filters/_creators.html.erb
index 3db8d63d6..9ba8954e7 100644
--- a/app/views/filters/_creators.html.erb
+++ b/app/views/filters/_creators.html.erb
@@ -4,57 +4,59 @@
     <button class="btn input input--select flex-inline txt-x-small" data-action="click->dialog#open:stop">
       <span class="overflow-ellipsis">
         <% if filter.creators.any? %>
-          <%= "Added by #{filter.creators.map(&:familiar_name).to_sentence}" %>
+          <%= "Added by #{filter.creators.map(&:familiar_name).to_sentence(two_words_connector: " or ", last_word_connector: ", or ")}" %>
         <% else %>
           Added by…
         <% end %>
       </span>
     </button>
 
-    <dialog class="margin-block-start-half popup panel flex-column align-start gap-half fill-white shadow"
-        aria-label="Added by…" aria-description="Added by…"
-        data-dialog-target="dialog" data-action="turbo:before-cache@document->dialog#close">
-      <strong class="popup__title margin-block-start-half pad-inline-half">Added by…</strong>
-      <%= form_with url: cards_path, method: :get, class: "popup__list",
-            data: { controller: "form" } do |form| %>
-        <% filter.as_params.except(:creator_ids).each do |key, value| %>
-          <%= filter_hidden_field_tag key, value %>
-        <% end %>
+    <%= tag.dialog class: "margin-block-start-half popup panel flex-column align-start gap-half fill-white shadow txt-small", data: {
+				action: "turbo:before-cache@document->dialog#close keydown->navigable-list#navigate filter:changed->navigable-list#reset toggle->filter#filter",
+        aria: { label: "Added by…", aria_description: "Added by…" },
+				controller: "filter navigable-list",
+				dialog_target: "dialog",
+				navigable_list_focus_on_selection_value: false,
+				navigable_list_actionable_items_value: true } do %>
+      <strong class="popup__title txt-nowrap">Added by…</strong>
 
-        <% if @expand_all %>
-          <%= hidden_field_tag :expand_all, @expand_all %>
-        <% end %>
-
-        <%= link_to cards_path(filter.as_params.except(:creator_ids).merge(expand_all: @expand_all)), class: "btn popup__item" do %>
-          <span class="overflow-ellipsis">Deselect all</span>
-        <% end %>
-
-        <div class="btn popup__item">
-          <%= form.check_box :creator_ids, {
-            multiple: true,
-            checked: filter.creators.include?(Current.user),
-            data: { action: "change->form#submit" },
-            include_hidden: false,
-          }, Current.user.id %>
-
-          <%= form.label :creator_ids, "Me", for: form.field_id(:creator_ids, Current.user.id), class: "overflow-ellipsis" %>
-          <%= icon_tag "check", class: "checked flex-item-justify-end" %>
-        </div>
-
-        <% User.active.without(Current.user).order(:name).each do |user| %>
-          <div class="btn popup__item">
-            <%= form.check_box :creator_ids, {
-              multiple: true,
-              checked: filter.creators.include?(user),
-              data: { action: "change->form#submit" },
-              include_hidden: false,
-            }, user.id %>
-
-            <%= form.label :creator_ids, user.name, for: form.field_id(:creator_ids, user.id), class: "overflow-ellipsis" %>
-            <%= icon_tag "check", class: "checked flex-item-justify-end" %>
-          </div>
-        <% end %>
+      <% if User.active.many? %>
+        <%= text_field_tag :search, nil, placeholder: "Filter…", class: "input input--transparent txt-small", autofocus: true,
+              type: "search", autocorrect: "off", autocomplete: "off", data: { "1p-ignore": "true", filter_target: "input", action: "input->filter#filter" } %>
       <% end %>
-    </dialog>
+
+      <ul class="popup__list" data-filter-target="list">
+        <% User.active.order(:name).each do |user| %>
+          <li class="popup__group flex align-center" data-value="<%= user.name.downcase %>" data-filter-target="item" data-navigable-list-target="item">
+            <%= form_with url: cards_path, method: :get, class: "full-width", data: { controller: "form" } do |form| %>
+              <% filter.as_params.except(:creator_ids).each do |key, value| %>
+                <%= filter_hidden_field_tag key, value %>
+              <% end %>
+
+              <% filter.creators.each do |existing_creators| %>
+                <% unless existing_creators == user %>
+                  <%= hidden_field_tag "creator_ids[]", existing_creators.id %>
+                <% end %>
+              <% end %>
+
+              <% unless filter.creators.include?(user) %>
+                <%= hidden_field_tag "creator_ids[]", user.id %>
+              <% end %>
+
+              <% if @expand_all %>
+                <%= hidden_field_tag :expand_all, @expand_all %>
+              <% end %>
+
+              <%= form.button type: "submit", class: "btn popup__item min-width full-width unpad-inline" do %>
+                <span class="overflow-ellipsis flex-item-grow"><%= user.name %></span>
+                <% if filter.creators.include?(user) %>
+                  <%= icon_tag "check", class: "checked flex-item-justify-end" %>
+                <% end %>
+              <% end %>
+            <% end %>
+          </li>
+        <% end %>
+      </ul>
+    <% end %>
   </div>
 <% end %>
diff --git a/app/views/filters/_indexed_by.html.erb b/app/views/filters/_indexed_by.html.erb
index 091252cf5..37a22388f 100644
--- a/app/views/filters/_indexed_by.html.erb
+++ b/app/views/filters/_indexed_by.html.erb
@@ -1,36 +1,44 @@
 <% if !filter.indexed_by.latest? || @expand_all %>
-  <div class="quick-filter position-relative" data-controller="dialog" data-action="keydown.esc->dialog#close click@document->dialog#closeOnClickOutside">
+  <div class="quick-filter position-relative <%= "default-value" if filter.indexed_by.latest? %>" data-controller="dialog" data-action="keydown.esc->dialog#close click@document->dialog#closeOnClickOutside">
     <button class="btn input input--select flex-inline txt-x-small" data-action="click->dialog#open:stop">
       <span class="overflow-ellipsis">
         <%= filter.indexed_by.humanize %>
       </span>
     </button>
 
-    <dialog class="margin-block-start-half popup panel flex-column align-start gap-half fill-white shadow"
-        aria-label="Sort by…" aria-description="Sort by…"
-        data-dialog-target="dialog" data-action="turbo:before-cache@document->dialog#close">
-      <strong class="popup__title margin-block-start-half pad-inline-half">Sort by…</strong>
-      <%= form_with url: cards_path, method: :get, class: "popup__list",
-            data: { controller: "form" } do |form| %>
-        <% filter.as_params.except(:indexed_by).each do |key, value| %>
-          <%= filter_hidden_field_tag key, value %>
-        <% end %>
-
-        <% if @expand_all %>
-          <%= hidden_field_tag :expand_all, @expand_all %>
-        <% end %>
-
+    <%= tag.dialog class: "margin-block-start-half popup panel flex-column align-start gap-half fill-white shadow txt-small", data: {
+				action: "turbo:before-cache@document->dialog#close keydown->navigable-list#navigate",
+        aria: { label: "Sort by…", aria_description: "Sort by…" },
+				controller: "navigable-list",
+				dialog_target: "dialog",
+				navigable_list_focus_on_selection_value: false,
+				navigable_list_actionable_items_value: true } do %>
+      <strong class="popup__title txt-nowrap">Sort by…</strong>
+      
+      <ul class="popup__list" data-filter-target="list">
         <% Filter::INDEXES.each do |index| %>
-          <div class="btn popup__item">
-            <%= form.radio_button :indexed_by, index,
-                checked: filter.indexed_by == index,
-                data: { action: "change->form#submit" } %>
+          <li class="popup__group flex align-center" data-navigable-list-target="item">
+            <%= form_with url: cards_path, method: :get, class: "full-width", data: { controller: "form" } do |form| %>
+              <% filter.as_params.except(:indexed_by).each do |key, value| %>
+                <%= filter_hidden_field_tag key, value %>
+              <% end %>
 
-            <%= form.label :indexed_by, index.humanize, value: index, class: "overflow-ellipsis" %>
-            <%= icon_tag "check", class: "checked flex-item-justify-end" %>
-          </div>
+              <%= hidden_field_tag :indexed_by, index %>
+
+              <% if @expand_all %>
+                <%= hidden_field_tag :expand_all, @expand_all %>
+              <% end %>
+
+              <%= form.button type: "submit", class: "btn popup__item min-width full-width unpad-inline" do %>
+                <span class="overflow-ellipsis flex-item-grow"><%= index.humanize %></span>
+                <% if filter.indexed_by == index %>
+                  <%= icon_tag "check", class: "checked flex-item-justify-end" %>
+                <% end %>
+              <% end %>
+            <% end %>
+          </li>
         <% end %>
-      <% end %>
-    </dialog>
+      </ul>
+    <% end %>
   </div>
 <% end %>
diff --git a/app/views/filters/_tags.html.erb b/app/views/filters/_tags.html.erb
index 2cc76c340..665d1f048 100644
--- a/app/views/filters/_tags.html.erb
+++ b/app/views/filters/_tags.html.erb
@@ -3,42 +3,59 @@
     <button class="btn input input--select flex-inline txt-x-small" data-action="click->dialog#open:stop">
       <span class="overflow-ellipsis">
         <% if filter.tags.any? %>
-          <%= filter.tags.map(&:hashtag).to_sentence %>
+          <%= filter.tags.map(&:hashtag).to_sentence(two_words_connector: " or ", last_word_connector: ", or ") %>
         <% else %>
           Tagged…
         <% end %>
       </span>
     </button>
 
-    <dialog class="margin-block-start-half popup panel flex-column align-start gap-half fill-white shadow" data-dialog-target="dialog" data-action="turbo:before-cache@document->dialog#close">
-      <strong class="popup__title margin-block-start-half pad-inline-half overflow-ellipsis">Tagged…</strong>
-      <%= form_with url: cards_path, method: :get, class: "popup__list",
-            data: { controller: "form" } do |form| %>
-        <% filter.as_params.except(:tag_ids).each do |key, value| %>
-          <%= filter_hidden_field_tag key, value %>
-        <% end %>
+    <%= tag.dialog class: "margin-block-start-half popup panel flex-column align-start gap-half fill-white shadow txt-small", data: {
+				action: "turbo:before-cache@document->dialog#close keydown->navigable-list#navigate filter:changed->navigable-list#reset toggle->filter#filter",
+        aria: { label: "Tagged…", aria_description: "Tagged…" },
+				controller: "filter navigable-list",
+				dialog_target: "dialog",
+				navigable_list_focus_on_selection_value: false,
+				navigable_list_actionable_items_value: true } do %>
+      <strong class="popup__title txt-nowrap">Tagged…</strong>
 
-        <% if @expand_all %>
-          <%= hidden_field_tag :expand_all, @expand_all %>
-        <% end %>
-
-        <%= link_to cards_path(filter.as_params.except(:tag_ids).merge(expand_all: @expand_all)), class: "btn popup__item" do %>
-          <span class="overflow-ellipsis">Clear all</span>
-        <% end %>
-
-        <% Tag.order(:title).each do |tag| %>
-          <div class="btn popup__item">
-            <%= form.check_box "tag_ids[]", {
-              checked: filter.tags.include?(tag),
-              data: { action: "change->form#submit" },
-              include_hidden: false,
-            }, tag.id %>
-
-            <%= form.label "tag_ids[]", tag.hashtag, for: dom_id(tag, :filter), class: "overflow-ellipsis" %>
-            <%= icon_tag "check", class: "checked flex-item-justify-end" %>
-          </div>
-        <% end %>
+      <% if Tag.many? %>
+        <%= text_field_tag :search, nil, placeholder: "Filter…", class: "input input--transparent txt-small", autofocus: true,
+              type: "search", autocorrect: "off", autocomplete: "off", data: { "1p-ignore": "true", filter_target: "input", action: "input->filter#filter" } %>
       <% end %>
-    </dialog>
+      
+      <ul class="popup__list" data-filter-target="list">
+        <% Tag.order(:title).each do |tag| %>
+          <li class="popup__group flex align-center" data-value="<%= tag.title.downcase %>" data-filter-target="item" data-navigable-list-target="item">
+            <%= form_with url: cards_path, method: :get, class: "full-width", data: { controller: "form" } do |form| %>
+              <% filter.as_params.except(:tag_ids).each do |key, value| %>
+                <%= filter_hidden_field_tag key, value %>
+              <% end %>
+
+              <% filter.tags.each do |existing_tag| %>
+                <% unless existing_tag == tag %>
+                  <%= hidden_field_tag "tag_ids[]", existing_tag.id %>
+                <% end %>
+              <% end %>
+
+              <% unless filter.tags.include?(tag) %>
+                <%= hidden_field_tag "tag_ids[]", tag.id %>
+              <% end %>
+
+              <% if @expand_all %>
+                <%= hidden_field_tag :expand_all, @expand_all %>
+              <% end %>
+
+              <%= form.button type: "submit", class: "btn popup__item min-width full-width unpad-inline" do %>
+                <span class="overflow-ellipsis flex-item-grow"><%= tag.hashtag %></span>
+                <% if filter.tags.include?(tag) %>
+                  <%= icon_tag "check", class: "checked flex-item-justify-end" %>
+                <% end %>
+              <% end %>
+            <% end %>
+          </li>
+        <% end %>
+      </ul>
+    <% end %>
   </div>
 <% end %>