diff --git a/app/assets/stylesheets/terminals.css b/app/assets/stylesheets/terminals.css index c854106f3..6c8f4bfb6 100644 --- a/app/assets/stylesheets/terminals.css +++ b/app/assets/stylesheets/terminals.css @@ -69,10 +69,57 @@ .terminal--open & { display: flex; } + + .terminal--showing-help & { + display: none; + } + } + + .terminal__help { + 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; + } } .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/javascript/controllers/terminal_controller.js b/app/javascript/controllers/terminal_controller.js index 65aaca9a3..0e43293aa 100644 --- a/app/javascript/controllers/terminal_controller.js +++ b/app/javascript/controllers/terminal_controller.js @@ -3,7 +3,11 @@ 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" ] + + disconnect() { + if (this.waitingForConfirmation) { this.#reset() } + } // Actions @@ -12,6 +16,28 @@ export default class extends Controller { } executeCommand(event) { + if (this.#showHelpCommandEntered) { + this.#showHelpMenu() + event.preventDefault() + event.stopPropagation() + } else { + this.hideHelpMenu() + } + } + + hideHelpMenu() { + if (this.#showHelpCommandEntered) { this.#reset() } + 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() } else { @@ -32,6 +58,18 @@ export default class extends Controller { this.element.classList.remove(this.errorClass) } + get #showHelpCommandEntered() { + return [ "/help", "/?" ].includes(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() @@ -47,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) @@ -57,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] ` + 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/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/filter_by_tag.rb b/app/models/command/filter_by_tag.rb new file mode 100644 index 000000000..f62aca01a --- /dev/null +++ b/app/models/command/filter_by_tag.rb @@ -0,0 +1,13 @@ +class Command::FilterByTag < Command + include Command::Tags + + store_accessor :data, :params + + def title + "Filter by tag ##{tag_title}" + end + + def execute + redirect_to cards_path(**params.merge(tag_ids: [ tag.id ])) + end +end diff --git a/app/models/command/parser.rb b/app/models/command/parser.rb index 830b6c93d..c2c98fcb7 100644 --- a/app/models/command/parser.rb +++ b/app/models/command/parser.rb @@ -19,12 +19,18 @@ class Command::Parser command_name, *command_arguments = string.strip.split(" ") case command_name - 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_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 "/clear" + Command::ClearFilters.new(params: filter.as_params) + when "/close" + Command::Close.new(card_ids: cards.ids, reason: command_arguments.join(" ")) + when "/tag" + Command::Tag.new(tag_title: tag_title_from(command_arguments.join(" ")), card_ids: cards.ids) else parse_free_string(string) end @@ -44,6 +50,10 @@ class Command::Parser User.all.find { |user| user.mentionable_handles.include?(string_without_at) } end + def tag_title_from(string) + string.gsub(/^#/, "") + 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/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 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/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? diff --git a/app/views/commands/_form.html.erb b/app/views/commands/_form.html.erb index ece1cc8ee..40d04fa56 100644 --- a/app/views/commands/_form.html.erb +++ b/app/views/commands/_form.html.erb @@ -4,7 +4,8 @@ data: { controller: "form", terminal_target: "form", - action: "turbo:submit-end->terminal#focus keydown.meta+k@document->terminal#focus turbo:submit-end->terminal#executeCommand" + 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/_help.html.erb b/app/views/commands/_help.html.erb new file mode 100644 index 000000000..8c18abcf3 --- /dev/null +++ b/app/views/commands/_help.html.erb @@ -0,0 +1,28 @@ +
+

Commands you can use in Fizzy:

+
+

Navigation

+
@username
+
Navigate to a someone's profile
+
[card id]
+
Navigate 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
+
/clear
+
Clear all the filters
+ +

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
+
+
diff --git a/app/views/commands/_terminal.html.erb b/app/views/commands/_terminal.html.erb index 31395cb47..6efee7eff 100644 --- a/app/views/commands/_terminal.html.erb +++ b/app/views/commands/_terminal.html.erb @@ -2,8 +2,9 @@ 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 %> + 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" %> 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 "commands/help" %> <% end %> 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) %> 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/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 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_by_tag_test.rb b/test/models/command/filter_by_tag_test.rb new file mode 100644 index 000000000..53be19ca9 --- /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 "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 + 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 diff --git a/test/models/command/tag_test.rb b/test/models/command/tag_test.rb new file mode 100644 index 000000000..7ac9a7b66 --- /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 tag" 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