From 50da28c0bed8b9bc3549ca8b1d3ff1e56909e5f2 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 7 May 2025 16:07:21 +0200 Subject: [PATCH 01/19] Add command to filter by tag --- app/models/command/filter_by_tag.rb | 18 ++++++++++++++++++ app/models/command/parser.rb | 7 +++++++ test/models/command/filter_by_tag_test.rb | 15 +++++++++++++++ 3 files changed, 40 insertions(+) create mode 100644 app/models/command/filter_by_tag.rb create mode 100644 test/models/command/filter_by_tag_test.rb diff --git a/app/models/command/filter_by_tag.rb b/app/models/command/filter_by_tag.rb new file mode 100644 index 000000000..8c6a49e74 --- /dev/null +++ b/app/models/command/filter_by_tag.rb @@ -0,0 +1,18 @@ +class Command::FilterByTag < Command + store_accessor :data, :tag_id, :params + + validates_presence_of :tag_id + + def title + "Filter by tag ##{tag&.title || tag_id} " + end + + def execute + redirect_to cards_path(**params.merge(tag_ids: [ tag_id ])) + end + + private + def tag + Tag.find_by_id(tag_id) + end +end diff --git a/app/models/command/parser.rb b/app/models/command/parser.rb index 830b6c93d..8fbd5ccc3 100644 --- a/app/models/command/parser.rb +++ b/app/models/command/parser.rb @@ -23,6 +23,8 @@ class Command::Parser Command::Assign.new(assignee_ids: assignees_from(command_arguments).collect(&:id), card_ids: cards.ids) when "/close" Command::Close.new(card_ids: cards.ids, reason: command_arguments.join(" ")) + when /^#/ + Command::FilterByTag.new(tag_id: tag_from(string).id, params: filter.as_params) when /^@/ Command::GoToUser.new(user_id: assignee_from(command_name)&.id) else @@ -44,6 +46,11 @@ class Command::Parser User.all.find { |user| user.mentionable_handles.include?(string_without_at) } end + def tag_from(string) + title = string.gsub(/^#/, "") + Tag.find_or_create_by!(title: title) + end + def parse_free_string(string) if cards = multiple_cards_from(string) Command::FilterCards.new(card_ids: cards.ids, params: filter.as_params) diff --git a/test/models/command/filter_by_tag_test.rb b/test/models/command/filter_by_tag_test.rb new file mode 100644 index 000000000..ef335a1d3 --- /dev/null +++ b/test/models/command/filter_by_tag_test.rb @@ -0,0 +1,15 @@ +require "test_helper" + +class Command::FilterByCardTest < ActionDispatch::IntegrationTest + include CommandTestHelper + + setup do + @tag = tags(:web) + end + + test "redirects to the cards index filtering by cards" do + result = execute_command "##{@tag.title}" + + assert_equal cards_path(indexed_by: "newest", tag_ids: [ @tag.id ]), result.url + end +end From 31d027195244c70c60c83a287334c6eaf49a56d1 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 7 May 2025 16:48:06 +0200 Subject: [PATCH 02/19] Rewrite to use infinitive in the test verbs, for consistency with other tests --- test/models/command/assign_test.rb | 4 ++-- test/models/command/close_test.rb | 22 +++++++++++----------- test/models/command/filter_cards_test.rb | 4 ++-- test/models/command/go_to_card_test.rb | 4 ++-- test/models/command/go_to_user_test.rb | 4 ++-- test/models/command/search_test.rb | 4 ++-- 6 files changed, 21 insertions(+), 21 deletions(-) diff --git a/test/models/command/assign_test.rb b/test/models/command/assign_test.rb index 705dad10e..89cda1d81 100644 --- a/test/models/command/assign_test.rb +++ b/test/models/command/assign_test.rb @@ -8,7 +8,7 @@ class Command::AssignTest < ActionDispatch::IntegrationTest @card = cards(:text) end - test "assigns card on perma" do + test "assign card on perma" do assert_difference -> { @card.assignees.count }, +2 do execute_command "/assign @kevin @david", context_url: collection_card_url(@card.collection, @card) end @@ -17,7 +17,7 @@ class Command::AssignTest < ActionDispatch::IntegrationTest assert_includes @card.assignees, users(:kevin) end - test "assigns cards on cards' index page" do + test "assign cards on cards' index page" do execute_command "/assign @kevin @david", context_url: collection_cards_url(@card.collection) cards(:logo, :text, :layout).each do |card| diff --git a/test/models/command/close_test.rb b/test/models/command/close_test.rb index abdb821b3..138e37aa4 100644 --- a/test/models/command/close_test.rb +++ b/test/models/command/close_test.rb @@ -8,7 +8,7 @@ class Command::CloseTest < ActionDispatch::IntegrationTest @card = cards(:text) end - test "closes card on perma" do + test "close card on perma" do assert_changes -> { @card.reload.closed? }, from: false, to: true do execute_command "/close", context_url: collection_card_url(@card.collection, @card) end @@ -17,7 +17,7 @@ class Command::CloseTest < ActionDispatch::IntegrationTest assert_equal users(:david), @card.closed_by end - test "closes card on perma with reason" do + test "close card on perma with reason" do assert_changes -> { @card.reload.closed? }, from: false, to: true do execute_command "/close Not now", context_url: collection_card_url(@card.collection, @card) end @@ -26,26 +26,26 @@ class Command::CloseTest < ActionDispatch::IntegrationTest assert_equal "Not now", @card.closure.reason end - test "closes cards on cards' index page" do - cards_to_check = cards(:logo, :text, :layout) - cards_to_check.each(&:reopen) + test "close cards on cards' index page" do + cards = cards(:logo, :text, :layout) + cards.each(&:reopen) execute_command "/close", context_url: collection_cards_url(@card.collection) - cards_to_check.each { it.reload.closed? } + cards.each { it.reload.closed? } end - test "undo closing" do - cards_to_check = cards(:logo, :text, :layout) - cards_to_check.each(&:reopen) + test "undo close" do + cards = cards(:logo, :text, :layout) + cards.each(&:reopen) command = parse_command "/close", context_url: collection_cards_url(@card.collection) command.execute - cards_to_check.each { it.reload.closed? } + cards.each { it.reload.closed? } command.undo - cards_to_check.each { it.reload.open? } + cards.each { it.reload.open? } end end diff --git a/test/models/command/filter_cards_test.rb b/test/models/command/filter_cards_test.rb index 0280157fe..f54cd1729 100644 --- a/test/models/command/filter_cards_test.rb +++ b/test/models/command/filter_cards_test.rb @@ -7,13 +7,13 @@ class Command::FilterCardsTest < ActionDispatch::IntegrationTest @card_ids = cards(:logo, :layout).collect(&:id) end - test "redirects to the cards index filtering by cards" do + test "redirect to the cards index filtering by cards" do result = execute_command "#{@card_ids.join(" ")}" assert_equal cards_path(indexed_by: "newest", card_ids: @card_ids), result.url end - test "respects existing filters" do + test "respect existing filters" do result = execute_command "#{@card_ids.join(",")}", context_url: "http://37signals.fizzy.localhost:3006/cards?collection_ids%5B%5D=#{collections(:writebook).id}" assert_equal cards_path(indexed_by: "newest", collection_ids: [ collections(:writebook).id ], card_ids: @card_ids), result.url diff --git a/test/models/command/go_to_card_test.rb b/test/models/command/go_to_card_test.rb index 6078bb298..fe6e13591 100644 --- a/test/models/command/go_to_card_test.rb +++ b/test/models/command/go_to_card_test.rb @@ -7,13 +7,13 @@ class Command::GoToCardTest < ActionDispatch::IntegrationTest @card = cards(:logo) end - test "redirects to the card perma" do + test "redirect to the card perma" do result = execute_command "#{@card.id}" assert_equal @card, result.url end - test "results in a regular search if the card does not exist" do + test "result in a regular search if the card does not exist" do command = parse_command "123" assert command.valid? diff --git a/test/models/command/go_to_user_test.rb b/test/models/command/go_to_user_test.rb index bb11648f5..6c6247186 100644 --- a/test/models/command/go_to_user_test.rb +++ b/test/models/command/go_to_user_test.rb @@ -3,13 +3,13 @@ require "test_helper" class Command::GoToUserTest < ActionDispatch::IntegrationTest include CommandTestHelper - test "redirects to the user perma" do + test "redirect to the user perma" do result = execute_command "@kevin" assert_equal users(:kevin), result.url end - test "results in an invalid command if the user does not exist" do + test "result in an invalid command if the user does not exist" do command = parse_command "@not_a_user" assert !command.valid? end diff --git a/test/models/command/search_test.rb b/test/models/command/search_test.rb index 57e8d900d..7a394f8e3 100644 --- a/test/models/command/search_test.rb +++ b/test/models/command/search_test.rb @@ -3,13 +3,13 @@ require "test_helper" class Command::SearchTest < ActionDispatch::IntegrationTest include CommandTestHelper - test "redirects to the cards index filtering by search terms" do + test "redirect to the cards index filtering by search terms" do result = execute_command "some text" assert_equal cards_path(indexed_by: "newest", terms: [ "some text" ]), result.url end - test "respects existing filters" do + test "respect existing filters" do result = execute_command "some text", context_url: "http://37signals.fizzy.localhost:3006/cards?collection_ids%5B%5D=#{collections(:writebook).id}" assert_equal cards_path(indexed_by: "newest", collection_ids: [ collections(:writebook).id ], terms: [ "some text" ]), result.url From b7f086d3616e17e8ec3e645db6dddb834ef1e2df Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 7 May 2025 16:48:33 +0200 Subject: [PATCH 03/19] Add command to tag cards --- app/models/command/filter_by_tag.rb | 13 ++----- app/models/command/parser.rb | 15 ++++---- app/models/command/tag.rb | 37 ++++++++++++++++++ app/models/command/tags.rb | 14 +++++++ test/models/command/filter_by_tag_test.rb | 2 +- test/models/command/tag_test.rb | 47 +++++++++++++++++++++++ 6 files changed, 111 insertions(+), 17 deletions(-) create mode 100644 app/models/command/tag.rb create mode 100644 app/models/command/tags.rb create mode 100644 test/models/command/tag_test.rb diff --git a/app/models/command/filter_by_tag.rb b/app/models/command/filter_by_tag.rb index 8c6a49e74..f62aca01a 100644 --- a/app/models/command/filter_by_tag.rb +++ b/app/models/command/filter_by_tag.rb @@ -1,18 +1,13 @@ class Command::FilterByTag < Command - store_accessor :data, :tag_id, :params + include Command::Tags - validates_presence_of :tag_id + store_accessor :data, :params def title - "Filter by tag ##{tag&.title || tag_id} " + "Filter by tag ##{tag_title}" end def execute - redirect_to cards_path(**params.merge(tag_ids: [ tag_id ])) + redirect_to cards_path(**params.merge(tag_ids: [ tag.id ])) end - - private - def tag - Tag.find_by_id(tag_id) - end end diff --git a/app/models/command/parser.rb b/app/models/command/parser.rb index 8fbd5ccc3..ea330420a 100644 --- a/app/models/command/parser.rb +++ b/app/models/command/parser.rb @@ -19,14 +19,16 @@ class Command::Parser command_name, *command_arguments = string.strip.split(" ") case command_name + when /^#/ + Command::FilterByTag.new(tag_title: tag_title_from(string), params: filter.as_params) + when /^@/ + Command::GoToUser.new(user_id: assignee_from(command_name)&.id) when "/assign", "/assignto" Command::Assign.new(assignee_ids: assignees_from(command_arguments).collect(&:id), card_ids: cards.ids) when "/close" Command::Close.new(card_ids: cards.ids, reason: command_arguments.join(" ")) - when /^#/ - Command::FilterByTag.new(tag_id: tag_from(string).id, params: filter.as_params) - when /^@/ - Command::GoToUser.new(user_id: assignee_from(command_name)&.id) + when "/tag" + Command::Tag.new(tag_title: tag_title_from(command_arguments.join(" ")), card_ids: cards.ids) else parse_free_string(string) end @@ -46,9 +48,8 @@ class Command::Parser User.all.find { |user| user.mentionable_handles.include?(string_without_at) } end - def tag_from(string) - title = string.gsub(/^#/, "") - Tag.find_or_create_by!(title: title) + def tag_title_from(string) + string.gsub(/^#/, "") end def parse_free_string(string) diff --git a/app/models/command/tag.rb b/app/models/command/tag.rb new file mode 100644 index 000000000..c6596de56 --- /dev/null +++ b/app/models/command/tag.rb @@ -0,0 +1,37 @@ +class Command::Tag < Command + include Command::Cards, Command::Tags + + store_accessor :data, :tagged_card_ids + + def title + "Tag #{cards_description} with ##{tag_title}" + end + + def execute + tagged_card_ids = [] + + transaction do + cards.find_each do |card| + unless card.tagged_with?(tag) + tagged_card_ids << card.id + card.toggle_tag_with(tag_title) + end + end + + update! tagged_card_ids: tagged_card_ids + end + end + + def undo + transaction do + tagged_cards.find_each do |card| + card.toggle_tag_with(tag_title) if card.tagged_with?(tag) + end + end + end + + private + def tagged_cards + user.accessible_cards.where(id: tagged_card_ids) + end +end diff --git a/app/models/command/tags.rb b/app/models/command/tags.rb new file mode 100644 index 000000000..448c7375c --- /dev/null +++ b/app/models/command/tags.rb @@ -0,0 +1,14 @@ +module Command::Tags + extend ActiveSupport::Concern + + included do + store_accessor :data, :tag_title + + validates_presence_of :tag_title + end + + private + def tag + Tag.find_or_create_by!(title: tag_title) + end +end diff --git a/test/models/command/filter_by_tag_test.rb b/test/models/command/filter_by_tag_test.rb index ef335a1d3..53be19ca9 100644 --- a/test/models/command/filter_by_tag_test.rb +++ b/test/models/command/filter_by_tag_test.rb @@ -7,7 +7,7 @@ class Command::FilterByCardTest < ActionDispatch::IntegrationTest @tag = tags(:web) end - test "redirects to the cards index filtering by cards" do + test "redirect to the cards index filtering by cards" do result = execute_command "##{@tag.title}" assert_equal cards_path(indexed_by: "newest", tag_ids: [ @tag.id ]), result.url diff --git a/test/models/command/tag_test.rb b/test/models/command/tag_test.rb new file mode 100644 index 000000000..7c3486b34 --- /dev/null +++ b/test/models/command/tag_test.rb @@ -0,0 +1,47 @@ +require "test_helper" + +class Command::TagTest < ActionDispatch::IntegrationTest + include CommandTestHelper + + setup do + Current.session = sessions(:david) + @card = cards(:text) + @tag = tags(:web) + end + + test "tag card on perma with existing tag" do + assert_changes -> { @card.tagged_with?(@tag) }, from: false, to: true do + execute_command "/tag #{@tag.title}", context_url: collection_card_url(@card.collection, @card) + end + end + + test "tag card on perma with new card" do + assert_difference -> { @card.tags.count }, +1 do + execute_command "/tag some-new-tag", context_url: collection_card_url(@card.collection, @card) + end + + assert_equal "some-new-tag", @card.tags.last.title + end + + test "tag several cards on cards' index page" do + cards = cards(:logo, :text, :layout) + cards.each { it.taggings.destroy_all } + + execute_command "/tag #{@tag.title}", context_url: collection_cards_url(@card.collection) + + cards.each { assert it.reload.tagged_with?(@tag) } + end + + test "undo tagged cards" do + cards = cards(:logo, :text, :layout) + cards.each { it.taggings.destroy_all } + + command = parse_command "/tag #{@tag.title}", context_url: collection_cards_url(@card.collection) + command.execute + + cards.each { assert it.reload.tagged_with?(@tag) } + + command.undo + cards.each { assert_not it.reload.tagged_with?(@tag) } + end +end From 2503524b7a0d3fbce52ec0f27fbe5175b6a5d023 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 7 May 2025 17:21:38 +0200 Subject: [PATCH 04/19] Implement help menu command to show a help menu with the commands --- app/assets/stylesheets/terminals.css | 8 ++++ .../controllers/terminal_controller.js | 31 +++++++++++++- app/views/commands/_form.html.erb | 2 +- app/views/commands/_help.html.erb | 42 +++++++++++++++++++ app/views/commands/_terminal.html.erb | 1 + app/views/commands/index.html.erb | 2 + 6 files changed, 84 insertions(+), 2 deletions(-) create mode 100644 app/views/commands/_help.html.erb diff --git a/app/assets/stylesheets/terminals.css b/app/assets/stylesheets/terminals.css index 7e82d6792..1e2becfaf 100644 --- a/app/assets/stylesheets/terminals.css +++ b/app/assets/stylesheets/terminals.css @@ -65,6 +65,14 @@ } } + .terminal__help { + display: none; + + .terminal--showing-help & { + display: block; + } + } + .terminal__item { @media (any-hover: hover) { &:where(:not(:active):hover), diff --git a/app/javascript/controllers/terminal_controller.js b/app/javascript/controllers/terminal_controller.js index a4148623c..09a2eea5a 100644 --- a/app/javascript/controllers/terminal_controller.js +++ b/app/javascript/controllers/terminal_controller.js @@ -3,7 +3,7 @@ import { HttpStatus } from "helpers/http_helpers" export default class extends Controller { static targets = [ "input", "form", "confirmation" ] - static classes = [ "error", "confirmation" ] + static classes = [ "error", "confirmation", "help" ] // Actions @@ -12,6 +12,23 @@ export default class extends Controller { } executeCommand(event) { + if (this.#hasShowHelpMenuCommand) { + this.#showHelpMenu() + event.preventDefault() + event.stopPropagation() + } else { + this.hideHelpMenu() + } + } + + hideHelpMenu() { + if (this.#isHelpMenuOpened && this.#hasShowHelpMenuCommand) { + this.#reset() + this.element.classList.remove(this.helpClass) + } + } + + handleCommandResponse(event) { if (event.detail.success) { this.#reset() } else { @@ -28,6 +45,18 @@ export default class extends Controller { } } + get #hasShowHelpMenuCommand() { + return this.inputTarget.value == "/help" || this.inputTarget.value == "/?" + } + + #showHelpMenu() { + this.element.classList.add(this.helpClass) + } + + get #isHelpMenuOpened() { + return this.element.classList.contains(this.helpClass) + } + async #handleErrorResponse(response) { const status = response.status const message = await response.text() diff --git a/app/views/commands/_form.html.erb b/app/views/commands/_form.html.erb index 5a82423ad..e6fec5b76 100644 --- a/app/views/commands/_form.html.erb +++ b/app/views/commands/_form.html.erb @@ -4,7 +4,7 @@ data: { controller: "form", terminal_target: "form", - action: "turbo:submit-end->terminal#focus keydown.meta+k@document->terminal#focus turbo:submit-end->terminal#executeCommand" + action: "turbo:submit-end->terminal#focus keydown.meta+k@document->terminal#focus keydown.enter->terminal#executeCommand keydown.esc->terminal#hideHelpMenu turbo:submit-end->terminal#handleCommandResponse" } do %> diff --git a/app/views/commands/_help.html.erb b/app/views/commands/_help.html.erb new file mode 100644 index 000000000..597aef5d9 --- /dev/null +++ b/app/views/commands/_help.html.erb @@ -0,0 +1,42 @@ +
+

Commands

+ +
+

Navigation

+
+
@username
+
Navigate to a user's page
+ +
[card id]
+
Go to a specific card by its id
+
+
+ +
+

Filtering

+
+
#tag_name
+
Filter cards by tag
+ +
[card id 1] [card id 2]...
+
Filter multiple cards by their IDs
+ +
search_term
+
Search for cards containing the term
+
+
+ +
+

Cards

+
+
/assign [username 1] [username 2]...
+
Assign selected cards to specified user(s)
+ +
/close [reason]
+
Close selected cards with an optional reason
+ +
/tag [tag name]
+
Add a tag to selected cards
+
+
+
diff --git a/app/views/commands/_terminal.html.erb b/app/views/commands/_terminal.html.erb index 31395cb47..ef93d3ed6 100644 --- a/app/views/commands/_terminal.html.erb +++ b/app/views/commands/_terminal.html.erb @@ -2,6 +2,7 @@ controller: "toggle-class terminal navigable-list", terminal_error_class: "terminal--error", terminal_confirmation_class: "terminal--confirmation", + terminal_help_class: "terminal--showing-help", toggle_class_toggle_class: "terminal--open", action: "keydown->navigable-list#navigate keydown.esc->toggle-class#remove:stop keydown.esc->terminal#focus keydown.esc->navigable-list#selectLast" } do %> <%= turbo_frame_tag :recent_commands, src: commands_path, refresh: "morph" %> diff --git a/app/views/commands/index.html.erb b/app/views/commands/index.html.erb index 49126efbe..b3369ab85 100644 --- a/app/views/commands/index.html.erb +++ b/app/views/commands/index.html.erb @@ -2,4 +2,6 @@
    <%= render partial: "commands/command", collection: @commands %>
+ + <%= render "commands/help" %> <% end %> From 970833fd48b4b92d7f8316bbf7f0b962762b9ee8 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 7 May 2025 17:36:32 +0200 Subject: [PATCH 05/19] Hide command history when showing help menu --- app/assets/stylesheets/terminals.css | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/assets/stylesheets/terminals.css b/app/assets/stylesheets/terminals.css index 1e2becfaf..f2794a8a3 100644 --- a/app/assets/stylesheets/terminals.css +++ b/app/assets/stylesheets/terminals.css @@ -63,6 +63,10 @@ .terminal--open & { display: flex; } + + .terminal--showing-help & { + display: none; + } } .terminal__help { From fe176aec7ce332289d16d34415de0ef669bc4309 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 7 May 2025 17:39:00 +0200 Subject: [PATCH 06/19] Only clear if you haven't touched the prompt --- app/javascript/controllers/terminal_controller.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/javascript/controllers/terminal_controller.js b/app/javascript/controllers/terminal_controller.js index 09a2eea5a..97e52935d 100644 --- a/app/javascript/controllers/terminal_controller.js +++ b/app/javascript/controllers/terminal_controller.js @@ -22,8 +22,8 @@ export default class extends Controller { } hideHelpMenu() { - if (this.#isHelpMenuOpened && this.#hasShowHelpMenuCommand) { - this.#reset() + if (this.#isHelpMenuOpened) { + if (this.#hasShowHelpMenuCommand) { this.#reset() } this.element.classList.remove(this.helpClass) } } From 8e4912a2b69f6d0da549935703c66f563b75aa2e Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 7 May 2025 17:40:03 +0200 Subject: [PATCH 07/19] Rename method --- app/javascript/controllers/terminal_controller.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/javascript/controllers/terminal_controller.js b/app/javascript/controllers/terminal_controller.js index 97e52935d..e60b0c4e2 100644 --- a/app/javascript/controllers/terminal_controller.js +++ b/app/javascript/controllers/terminal_controller.js @@ -12,7 +12,7 @@ export default class extends Controller { } executeCommand(event) { - if (this.#hasShowHelpMenuCommand) { + if (this.#hasHelpMenuCommand) { this.#showHelpMenu() event.preventDefault() event.stopPropagation() @@ -23,7 +23,7 @@ export default class extends Controller { hideHelpMenu() { if (this.#isHelpMenuOpened) { - if (this.#hasShowHelpMenuCommand) { this.#reset() } + if (this.#hasHelpMenuCommand) { this.#reset() } this.element.classList.remove(this.helpClass) } } @@ -45,7 +45,7 @@ export default class extends Controller { } } - get #hasShowHelpMenuCommand() { + get #hasHelpMenuCommand() { return this.inputTarget.value == "/help" || this.inputTarget.value == "/?" } From 8aecabe6a6651dc4acdfae37d05828b3c4d60774 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 7 May 2025 17:43:06 +0200 Subject: [PATCH 08/19] Fix typo --- test/models/command/tag_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/models/command/tag_test.rb b/test/models/command/tag_test.rb index 7c3486b34..7ac9a7b66 100644 --- a/test/models/command/tag_test.rb +++ b/test/models/command/tag_test.rb @@ -15,7 +15,7 @@ class Command::TagTest < ActionDispatch::IntegrationTest end end - test "tag card on perma with new card" do + test "tag card on perma with new tag" do assert_difference -> { @card.tags.count }, +1 do execute_command "/tag some-new-tag", context_url: collection_card_url(@card.collection, @card) end From 62b44cbb29a25b3b001c217d53b4aa78150e4897 Mon Sep 17 00:00:00 2001 From: Jason Zimdars Date: Wed, 7 May 2025 17:32:49 -0500 Subject: [PATCH 09/19] Polish help --- app/assets/stylesheets/terminals.css | 35 ++++++++++++++++++ app/views/commands/_form.html.erb | 2 +- app/views/commands/_help.html.erb | 54 ++++++++++------------------ 3 files changed, 55 insertions(+), 36 deletions(-) diff --git a/app/assets/stylesheets/terminals.css b/app/assets/stylesheets/terminals.css index f2794a8a3..904ca078d 100644 --- a/app/assets/stylesheets/terminals.css +++ b/app/assets/stylesheets/terminals.css @@ -73,7 +73,42 @@ display: none; .terminal--showing-help & { + border-block-end: 1px dashed; display: block; + margin-block-end: 0.5lh; + padding-block-end: 0.5lh; + } + + h2 { + border-block-end: 1px dashed; + font-size: var(--text-normal); + font-weight: normal; + margin-block: 0 0.5lh; + padding-block-end: 0.5lh; + } + + h3 { + font-size: var(--text-small); + margin-block: 0; + text-transform: uppercase; + } + + dl { + display: grid; + grid-template-columns: min-content 1fr; + gap: 0.5em 1em; + font-size: var(--text-small); + margin: 0; + } + + dt { + grid-column: 1; + margin-inline-start: var(--inline-space-double); + white-space: nowrap; + } + + dd { + grid-column: 2; } } diff --git a/app/views/commands/_form.html.erb b/app/views/commands/_form.html.erb index e6fec5b76..4fa5fb7f2 100644 --- a/app/views/commands/_form.html.erb +++ b/app/views/commands/_form.html.erb @@ -18,7 +18,7 @@ action: "keydown.up->toggle-class#add:prevent keydown.up->navigable-list#selectCurrentOrLast", turbo_permanent: true }, - placeholder: "Press ⌘+K to search or type commands…" %> + placeholder: "Press ⌘+K to search or type /commands…" %> <%= hidden_field_tag "confirmed", nil, data: { terminal_target: "confirmation" } %> <% end %> diff --git a/app/views/commands/_help.html.erb b/app/views/commands/_help.html.erb index 597aef5d9..356083278 100644 --- a/app/views/commands/_help.html.erb +++ b/app/views/commands/_help.html.erb @@ -1,42 +1,26 @@
-

Commands

- -
+

Commands you can use in Fizzy:

+

Navigation

-
-
@username
-
Navigate to a user's page
+
@username
+
Navigate to a someone's profile
+
[card id]
+
Navigate to a specific card by its id
-
[card id]
-
Go to a specific card by its id
-
-
- -

Filtering

-
-
#tag_name
-
Filter cards by tag
+
#tag_name
+
Filter cards by tag
+
[card id 1] [card id 2]...
+
Filter multiple cards by their IDs
+
search_term
+
Search for cards containing the term
-
[card id 1] [card id 2]...
-
Filter multiple cards by their IDs
- -
search_term
-
Search for cards containing the term
-
-
- -

Cards

-
-
/assign [username 1] [username 2]...
-
Assign selected cards to specified user(s)
- -
/close [reason]
-
Close selected cards with an optional reason
- -
/tag [tag name]
-
Add a tag to selected cards
-
-
+
/assign [username 1] [username 2]...
+
Assign selected cards to specified people(s)
+
/close [reason]
+
Close selected card(s) with an optional reason
+
/tag [tag name]
+
Add a tag to selected cards
+
From 280beb0db7117d3d5a12254e4d6eefeaea6a090e Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 8 May 2025 09:52:49 +0200 Subject: [PATCH 10/19] Arbitrary groups include closed cards --- app/models/filter.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/filter.rb b/app/models/filter.rb index 772e80c8b..a92b3a402 100644 --- a/app/models/filter.rb +++ b/app/models/filter.rb @@ -19,8 +19,8 @@ class Filter < ApplicationRecord def cards @cards ||= begin result = creator.accessible_cards.indexed_by(indexed_by) - result = result.where(id: card_ids) if card_ids.present? && !indexed_by.closed? - result = result.open unless indexed_by.closed? + result = result.where(id: card_ids) if card_ids.present? + result = result.open unless indexed_by.closed? || card_ids.present? result = result.by_engagement_status(engagement_status) if engagement_status.present? result = result.unassigned if assignment_status.unassigned? result = result.assigned_to(assignees.ids) if assignees.present? From 06ed269ef8731bd9ddbf9098697de7c49986c523 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 8 May 2025 09:57:45 +0200 Subject: [PATCH 11/19] Add control to show and remove card id filters --- app/views/filters/_cards.html.erb | 3 +++ app/views/filters/_settings.html.erb | 1 + 2 files changed, 4 insertions(+) create mode 100644 app/views/filters/_cards.html.erb diff --git a/app/views/filters/_cards.html.erb b/app/views/filters/_cards.html.erb new file mode 100644 index 000000000..fe9b61cd0 --- /dev/null +++ b/app/views/filters/_cards.html.erb @@ -0,0 +1,3 @@ +<% if filter.card_ids.present? %> + <%= filter_chip_tag "Cards #{filter.card_ids.join(", ")}", filter.as_params.without(:card_ids) %> +<% end %> diff --git a/app/views/filters/_settings.html.erb b/app/views/filters/_settings.html.erb index 850cd786e..9852304d8 100644 --- a/app/views/filters/_settings.html.erb +++ b/app/views/filters/_settings.html.erb @@ -4,6 +4,7 @@ <%= render "filters/assignees", filter: filter %> <%= render "filters/creators", filter: filter %> <%= render "filters/stages", filter: filter %> + <%= render "filters/cards", filter: filter %> <% filter.terms.each do |term| %> <%= filter_chip_tag %Q("#{term}"), filter.as_params_without(:terms, term) %> From d10c45772e7a05e052c8b8747480ee79dfc260af Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 8 May 2025 10:20:57 +0200 Subject: [PATCH 12/19] Add a clear command to clear filters --- app/models/command/clear_filters.rb | 20 ++++++++++++++++++++ app/models/command/parser.rb | 2 ++ app/views/commands/_help.html.erb | 2 ++ test/models/command/clear_filters_test.rb | 15 +++++++++++++++ 4 files changed, 39 insertions(+) create mode 100644 app/models/command/clear_filters.rb create mode 100644 test/models/command/clear_filters_test.rb diff --git a/app/models/command/clear_filters.rb b/app/models/command/clear_filters.rb new file mode 100644 index 000000000..90685d00e --- /dev/null +++ b/app/models/command/clear_filters.rb @@ -0,0 +1,20 @@ +class Command::ClearFilters < Command + store_accessor :data, :params + + def title + "Clear filters" + end + + def execute + redirect_to cards_path(**params.to_h.with_indifferent_access.without(*filters_to_clear)) + end + + private + FILTERS_TO_KEEP = [ :collection_ids, :indexed_by ] + + def filters_to_clear + Filter::Params::PERMITTED_PARAMS + .flat_map { |param| param.is_a?(Hash) ? param.keys : param } + .without(*FILTERS_TO_KEEP) + end +end diff --git a/app/models/command/parser.rb b/app/models/command/parser.rb index ea330420a..c2c98fcb7 100644 --- a/app/models/command/parser.rb +++ b/app/models/command/parser.rb @@ -25,6 +25,8 @@ class Command::Parser Command::GoToUser.new(user_id: assignee_from(command_name)&.id) when "/assign", "/assignto" Command::Assign.new(assignee_ids: assignees_from(command_arguments).collect(&:id), card_ids: cards.ids) + when "/clear" + Command::ClearFilters.new(params: filter.as_params) when "/close" Command::Close.new(card_ids: cards.ids, reason: command_arguments.join(" ")) when "/tag" diff --git a/app/views/commands/_help.html.erb b/app/views/commands/_help.html.erb index 356083278..8c18abcf3 100644 --- a/app/views/commands/_help.html.erb +++ b/app/views/commands/_help.html.erb @@ -14,6 +14,8 @@
Filter multiple cards by their IDs
search_term
Search for cards containing the term
+
/clear
+
Clear all the filters

Cards

/assign [username 1] [username 2]...
diff --git a/test/models/command/clear_filters_test.rb b/test/models/command/clear_filters_test.rb new file mode 100644 index 000000000..aa5fe1a5f --- /dev/null +++ b/test/models/command/clear_filters_test.rb @@ -0,0 +1,15 @@ +require "test_helper" + +class Command::FilterTest < ActionDispatch::IntegrationTest + include CommandTestHelper + + setup do + Current.session = sessions(:david) + end + + test "clear the filters keeping the selected collections" do + result = execute_command "/clear", context_url: "?card_ids%5B%5D=1&card_ids%5B%5D=2&collection_ids%5B%5D=#{collections(:writebook).id}&indexed_by=newest&terms%5B%5D=jorge" + + assert_equal cards_path(indexed_by: "newest", collection_ids: [ collections(:writebook).id ]), result.url + end +end From 86db634f7fb093adf077535a2a58cf5ab2966826 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 8 May 2025 10:41:51 +0200 Subject: [PATCH 13/19] Fix: selected item not showing up in Safari It also handles auto selecting when an element is focus. E.g: if you tabulate to the next control in the navigable list, you want selection to update accordingly. --- app/assets/stylesheets/terminals.css | 2 +- app/javascript/controllers/navigable_list_controller.js | 4 ++++ app/views/commands/_terminal.html.erb | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/terminals.css b/app/assets/stylesheets/terminals.css index 88a110dec..6c8f4bfb6 100644 --- a/app/assets/stylesheets/terminals.css +++ b/app/assets/stylesheets/terminals.css @@ -119,7 +119,7 @@ } .terminal__item { - &:where(:not(:active)):focus-visible { + &:where(:not(:active)):focus-visible, &:where(:not(:active))[aria-current] { --btn-color: var(--color-terminal-bg); background: var(--color-terminal-text); diff --git a/app/javascript/controllers/navigable_list_controller.js b/app/javascript/controllers/navigable_list_controller.js index 4951517d9..ca798e62b 100644 --- a/app/javascript/controllers/navigable_list_controller.js +++ b/app/javascript/controllers/navigable_list_controller.js @@ -13,6 +13,10 @@ export default class extends Controller { } } + select({ target }) { + this.#setCurrentFrom(target) + } + selectCurrentOrLast(event) { if (this.currentItem) { this.#setCurrentFrom(this.currentItem) diff --git a/app/views/commands/_terminal.html.erb b/app/views/commands/_terminal.html.erb index ef93d3ed6..642d3bd07 100644 --- a/app/views/commands/_terminal.html.erb +++ b/app/views/commands/_terminal.html.erb @@ -4,7 +4,7 @@ terminal_confirmation_class: "terminal--confirmation", terminal_help_class: "terminal--showing-help", toggle_class_toggle_class: "terminal--open", - action: "keydown->navigable-list#navigate keydown.esc->toggle-class#remove:stop keydown.esc->terminal#focus keydown.esc->navigable-list#selectLast" } do %> + action: "keydown->navigable-list#navigate focusin->navigable-list#select keydown.esc->toggle-class#remove:stop keydown.esc->terminal#focus keydown.esc->navigable-list#selectLast" } do %> <%= turbo_frame_tag :recent_commands, src: commands_path, refresh: "morph" %> <%= render "commands/form" %> From cb27a1378106b9778a34f11afcb87266a6f2a69b Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 8 May 2025 10:48:21 +0200 Subject: [PATCH 14/19] Simplify --- app/javascript/controllers/terminal_controller.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/javascript/controllers/terminal_controller.js b/app/javascript/controllers/terminal_controller.js index 93c5dd590..4cf2e4f51 100644 --- a/app/javascript/controllers/terminal_controller.js +++ b/app/javascript/controllers/terminal_controller.js @@ -22,10 +22,8 @@ export default class extends Controller { } hideHelpMenu() { - if (this.#isHelpMenuOpened) { - if (this.#hasHelpMenuCommand) { this.#reset() } - this.element.classList.remove(this.helpClass) - } + if (this.#hasHelpMenuCommand) { this.#reset() } + this.element.classList.remove(this.helpClass) } handleCommandResponse(event) { From c25ec9ef7b509fd63516a6c5b2c71ff546388ec2 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 8 May 2025 10:49:15 +0200 Subject: [PATCH 15/19] Rename --- app/javascript/controllers/terminal_controller.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/javascript/controllers/terminal_controller.js b/app/javascript/controllers/terminal_controller.js index 4cf2e4f51..0db83594f 100644 --- a/app/javascript/controllers/terminal_controller.js +++ b/app/javascript/controllers/terminal_controller.js @@ -12,7 +12,7 @@ export default class extends Controller { } executeCommand(event) { - if (this.#hasHelpMenuCommand) { + if (this.#showHelpCommandEntered) { this.#showHelpMenu() event.preventDefault() event.stopPropagation() @@ -22,7 +22,7 @@ export default class extends Controller { } hideHelpMenu() { - if (this.#hasHelpMenuCommand) { this.#reset() } + if (this.#showHelpCommandEntered) { this.#reset() } this.element.classList.remove(this.helpClass) } @@ -47,7 +47,7 @@ export default class extends Controller { this.element.classList.remove(this.errorClass) } - get #hasHelpMenuCommand() { + get #showHelpCommandEntered() { return this.inputTarget.value == "/help" || this.inputTarget.value == "/?" } From 05e55adbaa35016cc51fc623de7904a9a87fede4 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 8 May 2025 10:51:33 +0200 Subject: [PATCH 16/19] Prefer array --- app/javascript/controllers/terminal_controller.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/javascript/controllers/terminal_controller.js b/app/javascript/controllers/terminal_controller.js index 0db83594f..38a65a20a 100644 --- a/app/javascript/controllers/terminal_controller.js +++ b/app/javascript/controllers/terminal_controller.js @@ -48,7 +48,7 @@ export default class extends Controller { } get #showHelpCommandEntered() { - return this.inputTarget.value == "/help" || this.inputTarget.value == "/?" + return [ "/help", "/?" ].includes(this.inputTarget.value) } #showHelpMenu() { From 5cd71a769913aee2e65b849125b2f822b68ba579 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 8 May 2025 11:21:39 +0200 Subject: [PATCH 17/19] Don't dismiss on any key, only with N or ESC Also, this reworks the system to rely on stimulus event handling. This cover cases like unbinding events automatically when the controller get disconnected and such. See: https://3.basecamp.com/2914079/buckets/37331921/todos/8617430347 --- .../controllers/terminal_controller.js | 47 ++++++++++--------- app/views/commands/_form.html.erb | 4 +- app/views/commands/_terminal.html.erb | 2 +- 3 files changed, 27 insertions(+), 26 deletions(-) diff --git a/app/javascript/controllers/terminal_controller.js b/app/javascript/controllers/terminal_controller.js index 38a65a20a..0e43293aa 100644 --- a/app/javascript/controllers/terminal_controller.js +++ b/app/javascript/controllers/terminal_controller.js @@ -5,6 +5,10 @@ export default class extends Controller { static targets = [ "input", "form", "confirmation" ] static classes = [ "error", "confirmation", "help" ] + disconnect() { + if (this.waitingForConfirmation) { this.#reset() } + } + // Actions focus() { @@ -26,6 +30,13 @@ export default class extends Controller { this.element.classList.remove(this.helpClass) } + handleKeyPress(event) { + if (this.waitingForConfirmation) { + this.#handleConfirmationKey(event.key.toLowerCase()) + event.preventDefault() + } + } + handleCommandResponse(event) { if (event.detail.success) { this.#reset() @@ -74,6 +85,8 @@ export default class extends Controller { this.formTarget.reset() this.inputTarget.value = inputValue this.confirmationTarget.value = "" + this.waitingForConfirmation = false + this.originalInputValue = null this.element.classList.remove(this.errorClass) this.element.classList.remove(this.confirmationClass) @@ -84,35 +97,23 @@ export default class extends Controller { } async #requestConfirmation(message) { - const originalInputValue = this.inputTarget.value + this.originalInputValue = this.inputTarget.value this.element.classList.add(this.confirmationClass) this.inputTarget.value = `${message}? [Y/n] ` - try { - await this.#waitForConfirmation() - this.#submitWithConfirmation(originalInputValue) - } catch { - this.#reset(originalInputValue) + this.waitingForConfirmation = true + } + + #handleConfirmationKey(key) { + if (key === "enter" || key === "y") { + this.#submitWithConfirmation() + } else if (key === "escape" || key === "n") { + this.#reset(this.originalInputValue) } } - #waitForConfirmation() { - return new Promise((resolve, reject) => { - this.inputTarget.addEventListener("keydown", (event) => { - event.preventDefault() - const key = event.key.toLowerCase() - - if (key === "enter" || key === "y") { - resolve() - } else { - reject() - } - }, { once: true }) - }) - } - - #submitWithConfirmation(inputValue) { - this.inputTarget.value = inputValue + #submitWithConfirmation() { + this.inputTarget.value = this.originalInputValue this.confirmationTarget.value = "confirmed" this.formTarget.requestSubmit() } diff --git a/app/views/commands/_form.html.erb b/app/views/commands/_form.html.erb index a2f979c9c..40d04fa56 100644 --- a/app/views/commands/_form.html.erb +++ b/app/views/commands/_form.html.erb @@ -4,6 +4,7 @@ data: { controller: "form", terminal_target: "form", + turbo_permanent: true, action: "turbo:submit-end->terminal#focus keydown.meta+k@document->terminal#focus keydown.enter->terminal#executeCommand keydown.esc->terminal#hideHelpMenu turbo:submit-end->terminal#handleCommandResponse" } do %> @@ -15,8 +16,7 @@ class: "terminal__input input fill-transparent unpad", data: { terminal_target: "input", - action: "keydown.up->toggle-class#add:prevent keydown.up->navigable-list#selectCurrentOrLast terminal#hideError", - turbo_permanent: true + action: "keydown.up->toggle-class#add:prevent keydown.up->navigable-list#selectCurrentOrLast terminal#hideError" }, placeholder: "Press ⌘+K to search or type commands…", spellcheck: "false" %> diff --git a/app/views/commands/_terminal.html.erb b/app/views/commands/_terminal.html.erb index 642d3bd07..62bd7ecbf 100644 --- a/app/views/commands/_terminal.html.erb +++ b/app/views/commands/_terminal.html.erb @@ -4,7 +4,7 @@ terminal_confirmation_class: "terminal--confirmation", terminal_help_class: "terminal--showing-help", toggle_class_toggle_class: "terminal--open", - action: "keydown->navigable-list#navigate focusin->navigable-list#select keydown.esc->toggle-class#remove:stop keydown.esc->terminal#focus keydown.esc->navigable-list#selectLast" } do %> + action: "keydown->terminal#handleKeyPress keydown->navigable-list#navigate focusin->navigable-list#select keydown.esc->toggle-class#remove:stop keydown.esc->terminal#focus keydown.esc->navigable-list#selectLast" } do %> <%= turbo_frame_tag :recent_commands, src: commands_path, refresh: "morph" %> <%= render "commands/form" %> From 8c31d1f48af55f6b47220f589aef30be2de16d01 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 8 May 2025 11:29:40 +0200 Subject: [PATCH 18/19] Only consider published cards when using filter --- app/models/command/parser/context.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/command/parser/context.rb b/app/models/command/parser/context.rb index 09591ce72..fae468efb 100644 --- a/app/models/command/parser/context.rb +++ b/app/models/command/parser/context.rb @@ -10,7 +10,7 @@ class Command::Parser::Context if controller == "cards" && action == "show" user.accessible_cards.where id: params[:id] elsif controller == "cards" && action == "index" - filter.cards + filter.cards.published else Card.none end From 1662a8af8261ff7b65d16f2fde5b503d2f71dc33 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 8 May 2025 11:37:30 +0200 Subject: [PATCH 19/19] Hide terminal menu when pressing ESC This was only working when the focus was on the input --- app/views/commands/_terminal.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/commands/_terminal.html.erb b/app/views/commands/_terminal.html.erb index 62bd7ecbf..6efee7eff 100644 --- a/app/views/commands/_terminal.html.erb +++ b/app/views/commands/_terminal.html.erb @@ -4,7 +4,7 @@ terminal_confirmation_class: "terminal--confirmation", terminal_help_class: "terminal--showing-help", toggle_class_toggle_class: "terminal--open", - action: "keydown->terminal#handleKeyPress keydown->navigable-list#navigate focusin->navigable-list#select keydown.esc->toggle-class#remove:stop keydown.esc->terminal#focus keydown.esc->navigable-list#selectLast" } do %> + action: "keydown->terminal#handleKeyPress keydown->navigable-list#navigate focusin->navigable-list#select keydown.esc->toggle-class#remove:stop keydown.esc->terminal#focus keydown.esc->navigable-list#selectLast keydown.esc@document->terminal#hideHelpMenu" } do %> <%= turbo_frame_tag :recent_commands, src: commands_path, refresh: "morph" %> <%= render "commands/form" %>