From d1f785e7de056ad54809fd8d6f411428ffa6aed7 Mon Sep 17 00:00:00 2001
From: Joffrey JAFFEUX <j.jaffeux@gmail.com>
Date: Fri, 26 May 2023 18:48:45 +0200
Subject: [PATCH] UX: closes drawer on esc if input is not focused (#21772)
The current behavior is to close drawer when pressing escape inside the input.
After this change, first escape will blur the input, and second escape will close the drawer.
This commit also refactors the whole shortcuts for drawer system spec.
---
.../discourse/components/chat-composer.js | 6 +-
.../initializers/chat-keyboard-shortcuts.js | 4 --
plugins/chat/spec/system/drawer_spec.rb | 3 +-
.../spec/system/page_objects/chat/chat.rb | 11 ++++
.../chat/spec/system/shortcuts/drawer_spec.rb | 56 ++++++++++++-------
5 files changed, 52 insertions(+), 28 deletions(-)
diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-composer.js b/plugins/chat/assets/javascripts/discourse/components/chat-composer.js
index 82a7085671d..aae58b67594 100644
--- a/plugins/chat/assets/javascripts/discourse/components/chat-composer.js
+++ b/plugins/chat/assets/javascripts/discourse/components/chat-composer.js
@@ -359,13 +359,13 @@ export default class ChatComposer extends Component {
}
}
- if (event.key === "Escape") {
+ if (event.key === "Escape" && this.isFocused) {
+ event.stopPropagation();
+
if (this.currentMessage?.inReplyTo) {
this.reset();
- return false;
} else if (this.currentMessage?.editing) {
this.composer.onCancelEditing();
- return false;
} else {
event.target.blur();
}
diff --git a/plugins/chat/assets/javascripts/discourse/initializers/chat-keyboard-shortcuts.js b/plugins/chat/assets/javascripts/discourse/initializers/chat-keyboard-shortcuts.js
index b535c4e119d..d29252b67e5 100644
--- a/plugins/chat/assets/javascripts/discourse/initializers/chat-keyboard-shortcuts.js
+++ b/plugins/chat/assets/javascripts/discourse/initializers/chat-keyboard-shortcuts.js
@@ -92,10 +92,6 @@ export default {
return;
}
- if (!isChatComposer(event.target)) {
- return;
- }
-
event.preventDefault();
event.stopPropagation();
appEvents.trigger("chat:toggle-close", event);
diff --git a/plugins/chat/spec/system/drawer_spec.rb b/plugins/chat/spec/system/drawer_spec.rb
index 0ebb40685d0..39d73599ece 100644
--- a/plugins/chat/spec/system/drawer_spec.rb
+++ b/plugins/chat/spec/system/drawer_spec.rb
@@ -79,9 +79,10 @@ RSpec.describe "Drawer", type: :system, js: true do
chat_page.open_from_header
drawer.open_channel(channel_1)
channel_page.hover_message(message_1)
+
expect(page).to have_css(".chat-message-actions-container")
- find(".chat-composer__input").send_keys(:escape)
+ drawer.close
expect(page).to have_no_css(".chat-message-actions-container")
end
diff --git a/plugins/chat/spec/system/page_objects/chat/chat.rb b/plugins/chat/spec/system/page_objects/chat/chat.rb
index 9a6d4dc1b5b..dadb5501ed6 100644
--- a/plugins/chat/spec/system/page_objects/chat/chat.rb
+++ b/plugins/chat/spec/system/page_objects/chat/chat.rb
@@ -17,6 +17,17 @@ module PageObjects
visit("/chat")
end
+ def has_drawer?(channel_id: nil, expanded: true)
+ selector = ".chat-drawer"
+ selector += ".is-expanded" if expanded
+ selector += "[data-chat-channel-id=\"#{channel_id}\"]" if channel_id
+ has_css?(selector)
+ end
+
+ def has_no_drawer?(**args)
+ !has_drawer?(**args)
+ end
+
def visit_channel(channel, message_id: nil)
visit(channel.url + (message_id ? "/#{message_id}" : ""))
has_no_css?(".chat-channel--not-loaded-once")
diff --git a/plugins/chat/spec/system/shortcuts/drawer_spec.rb b/plugins/chat/spec/system/shortcuts/drawer_spec.rb
index 6333afca295..e171f082845 100644
--- a/plugins/chat/spec/system/shortcuts/drawer_spec.rb
+++ b/plugins/chat/spec/system/shortcuts/drawer_spec.rb
@@ -6,7 +6,8 @@ RSpec.describe "Shortcuts | drawer", type: :system, js: true do
fab!(:channel_2) { Fabricate(:chat_channel) }
let(:chat_page) { PageObjects::Pages::Chat.new }
- let(:drawer) { PageObjects::Pages::ChatDrawer.new }
+ let(:channel_page) { PageObjects::Pages::ChatChannel.new }
+ let(:drawer_page) { PageObjects::Pages::ChatDrawer.new }
before do
chat_system_bootstrap(user_1, [channel_1, channel_2])
@@ -18,9 +19,9 @@ RSpec.describe "Shortcuts | drawer", type: :system, js: true do
context "when pressing dash" do
it "opens the drawer" do
- find("body").send_keys("-")
+ page.send_keys("-")
- expect(page).to have_css(".chat-drawer.is-expanded")
+ expect(chat_page).to have_drawer
end
end
end
@@ -32,44 +33,59 @@ RSpec.describe "Shortcuts | drawer", type: :system, js: true do
end
context "when pressing escape" do
- it "closes the drawer" do
- expect(page).to have_css(".chat-drawer.is-expanded")
+ context "when the composer is not focused" do
+ it "closes the drawer" do
+ expect(chat_page).to have_drawer
- drawer.open_channel(channel_1)
- find(".chat-composer__input").send_keys(:escape)
+ drawer_page.open_channel(channel_1)
+ page.send_keys(:tab) # ensures we focus out of input
+ page.send_keys(:escape)
- expect(page).to have_no_css(".chat-drawer.is-expanded")
+ expect(chat_page).to have_no_drawer
+ end
+ end
+
+ context "when the composer is focused" do
+ it "blurs the input" do
+ expect(chat_page).to have_drawer
+
+ drawer_page.open_channel(channel_1)
+ channel_page.composer.input.click
+
+ page.send_keys(:escape)
+
+ expect(chat_page).to have_drawer
+ end
end
end
context "when pressing a letter" do
it "doesn’t intercept the event" do
- drawer.open_channel(channel_1)
- find(".header-sidebar-toggle").click # simple way to ensure composer is not focused
-
+ drawer_page.open_channel(channel_1)
+ page.send_keys(:tab) # simple way to ensure composer is not focused
page.send_keys("e")
- expect(find(".chat-composer__input").value).to eq("")
+ expect(channel_page.composer.value).to eq("")
end
end
context "when using Up/Down arrows" do
it "navigates through the channels" do
- drawer.open_channel(channel_1)
+ drawer_page.open_channel(channel_1)
- expect(page).to have_selector(".chat-drawer[data-chat-channel-id=\"#{channel_1.id}\"]")
+ expect(chat_page).to have_drawer(channel_id: channel_1.id)
- find(".chat-composer__input").send_keys(%i[alt arrow_down])
+ page.send_keys(%i[alt arrow_down])
- expect(page).to have_selector(".chat-drawer[data-chat-channel-id=\"#{channel_2.id}\"]")
+ expect(chat_page).to have_drawer(channel_id: channel_2.id)
- find(".chat-composer__input").send_keys(%i[alt arrow_down])
+ page.send_keys(%i[alt arrow_down])
- expect(page).to have_selector(".chat-drawer[data-chat-channel-id=\"#{channel_1.id}\"]")
+ expect(chat_page).to have_drawer(channel_id: channel_1.id)
- find(".chat-composer__input").send_keys(%i[alt arrow_up])
+ page.send_keys(%i[alt arrow_up])
- expect(page).to have_selector(".chat-drawer[data-chat-channel-id=\"#{channel_2.id}\"]")
+ expect(chat_page).to have_drawer(channel_id: channel_2.id)
end
end
end