UX: improves composer and thread panel (#21210)

This pull request is a full overhaul of the chat-composer and contains various improvements to the thread panel. They have been grouped in the same PR as lots of improvements/fixes to the thread panel needed an improved composer. This is meant as a first step.

### New features included in this PR

- A resizable side panel
- A clear dropzone area for uploads
- A simplified design for image uploads, this is only a first step towards more redesign of this area in the future

### Notable fixes in this PR

- Correct placeholder in thread panel
- Allows to edit the last message of a thread with arrow up
- Correctly focus composer when replying to a message
- The reply indicator is added instantly in the channel when starting a thread
- Prevents a large variety of bug where the composer could bug and prevent sending message or would clear your input while it has content

### Technical notes

To achieve this PR, three important changes have been made:

- `<ChatComposer>` has been fully rewritten and is now a glimmer component
- The chat composer now takes a `ChatMessage` as input which can directly be used in other operations, it simplifies a lot of logic as we are always working a with a `ChatMessage`
- `TextareaInteractor` has been created to wrap the existing `TextareaTextManipulation` mixin, it will make future migrations easier and allow us to have a less polluted `<ChatComposer>`

Note ".chat-live-pane" has been renamed ".chat-channel"

Design for upload dropzone is from @chapoi
This commit is contained in:
Joffrey JAFFEUX
2023-04-25 10:23:03 +02:00
committed by GitHub
parent 02625d1edd
commit bf886662df
82 changed files with 1872 additions and 1337 deletions

View File

@ -10,10 +10,83 @@ RSpec.describe "Chat composer", type: :system, js: true do
before { chat_system_bootstrap }
xit "it stores draft in replies" do
end
context "when loading a channel with a draft" do
fab!(:draft_1) do
Chat::Draft.create!(
chat_channel: channel_1,
user: current_user,
data: { message: "draft" }.to_json,
)
end
xit "it stores draft" do
before do
channel_1.add(current_user)
sign_in(current_user)
end
it "loads the draft" do
chat.visit_channel(channel_1)
expect(find(".chat-composer__input").value).to eq("draft")
end
context "with uploads" do
fab!(:upload_1) do
Fabricate(
:upload,
url: "/images/logo-dark.png",
original_filename: "logo_dark.png",
width: 400,
height: 300,
extension: "png",
)
end
fab!(:draft_1) do
Chat::Draft.create!(
chat_channel: channel_1,
user: current_user,
data: { message: "draft", uploads: [upload_1] }.to_json,
)
end
it "loads the draft with the upload" do
chat.visit_channel(channel_1)
expect(find(".chat-composer__input").value).to eq("draft")
expect(page).to have_selector(".chat-composer-upload--image", count: 1)
end
end
context "when replying" do
fab!(:draft_1) do
Chat::Draft.create!(
chat_channel: channel_1,
user: current_user,
data: {
message: "draft",
replyToMsg: {
id: message_1.id,
excerpt: message_1.excerpt,
user: {
id: message_1.user.id,
name: nil,
avatar_template: message_1.user.avatar_template,
username: message_1.user.username,
},
},
}.to_json,
)
end
it "loads the draft with replied to mesage" do
chat.visit_channel(channel_1)
expect(find(".chat-composer__input").value).to eq("draft")
expect(page).to have_selector(".chat-reply__username", text: message_1.user.username)
expect(page).to have_selector(".chat-reply__excerpt", text: message_1.excerpt)
end
end
end
context "when replying to a message" do
@ -62,17 +135,17 @@ RSpec.describe "Chat composer", type: :system, js: true do
".chat-composer-message-details .chat-reply__username",
text: current_user.username,
)
expect(find(".chat-composer-input").value).to eq(message_2.message)
expect(find(".chat-composer__input").value).to eq(message_2.message)
end
context "when pressing escape" do
it "cancels editing" do
chat.visit_channel(channel_1)
channel.edit_message(message_2)
find(".chat-composer-input").send_keys(:escape)
find(".chat-composer__input").send_keys(:escape)
expect(page).to have_no_selector(".chat-composer-message-details .chat-reply__username")
expect(find(".chat-composer-input").value).to eq("")
expect(find(".chat-composer__input").value).to eq("")
end
end
@ -83,7 +156,7 @@ RSpec.describe "Chat composer", type: :system, js: true do
find(".cancel-message-action").click
expect(page).to have_no_selector(".chat-composer-message-details .chat-reply__username")
expect(find(".chat-composer-input").value).to eq("")
expect(find(".chat-composer__input").value).to eq("")
end
end
end
@ -100,7 +173,7 @@ RSpec.describe "Chat composer", type: :system, js: true do
channel.click_action_button("emoji")
find("[data-emoji='grimacing']").click(wait: 0.5)
expect(find(".chat-composer-input").value).to eq(":grimacing:")
expect(find(".chat-composer__input").value).to eq(":grimacing:")
end
it "removes denied emojis from insert emoji picker" do
@ -123,20 +196,20 @@ RSpec.describe "Chat composer", type: :system, js: true do
it "adds the emoji to the composer" do
chat.visit_channel(channel_1)
find(".chat-composer-input").fill_in(with: ":gri")
find(".chat-composer__input").fill_in(with: ":gri")
find(".emoji-shortname", text: "grimacing").click
expect(find(".chat-composer-input").value).to eq(":grimacing: ")
expect(find(".chat-composer__input").value).to eq(":grimacing: ")
end
it "doesn't suggest denied emojis and aliases" do
SiteSetting.emoji_deny_list = "peach|poop"
chat.visit_channel(channel_1)
find(".chat-composer-input").fill_in(with: ":peac")
find(".chat-composer__input").fill_in(with: ":peac")
expect(page).to have_no_selector(".emoji-shortname", text: "peach")
find(".chat-composer-input").fill_in(with: ":hank") # alias
find(".chat-composer__input").fill_in(with: ":hank") # alias
expect(page).to have_no_selector(".emoji-shortname", text: "poop")
end
end
@ -149,7 +222,7 @@ RSpec.describe "Chat composer", type: :system, js: true do
xit "prefills the emoji picker filter input" do
chat.visit_channel(channel_1)
find(".chat-composer-input").fill_in(with: ":gri")
find(".chat-composer__input").fill_in(with: ":gri")
click_link(I18n.t("js.composer.more_emoji"))
@ -158,7 +231,7 @@ RSpec.describe "Chat composer", type: :system, js: true do
it "filters with the prefilled input" do
chat.visit_channel(channel_1)
find(".chat-composer-input").fill_in(with: ":fr")
find(".chat-composer__input").fill_in(with: ":fr")
click_link(I18n.t("js.composer.more_emoji"))
@ -178,15 +251,15 @@ RSpec.describe "Chat composer", type: :system, js: true do
find("body").send_keys("b")
expect(find(".chat-composer-input").value).to eq("b")
expect(find(".chat-composer__input").value).to eq("b")
find("body").send_keys("b")
expect(find(".chat-composer-input").value).to eq("bb")
expect(find(".chat-composer__input").value).to eq("bb")
find("body").send_keys(:enter) # special case
expect(find(".chat-composer-input").value).to eq("bb")
expect(find(".chat-composer__input").value).to eq("bb")
end
end
end

View File

@ -22,7 +22,7 @@ RSpec.describe "Chat message", type: :system, js: true do
channel.hover_message(message_1)
expect(page).to have_css(
".chat-live-pane[data-id='#{channel_1.id}'] [data-id='#{message_1.id}'] .chat-message.is-active",
".chat-channel[data-id='#{channel_1.id}'] [data-id='#{message_1.id}'] .chat-message.is-active",
)
end
end

View File

@ -11,6 +11,7 @@ RSpec.describe "Chat message - channel", type: :system, js: true do
let(:cdp) { PageObjects::CDP.new }
let(:chat) { PageObjects::Pages::Chat.new }
let(:channel) { PageObjects::Pages::ChatChannel.new }
let(:thread) { PageObjects::Pages::ChatThread.new }
let(:message_1) { thread_1.chat_messages.first }
before do
@ -24,12 +25,13 @@ RSpec.describe "Chat message - channel", type: :system, js: true do
context "when hovering a message" do
it "adds an active class" do
last_message = thread_1.chat_messages.last
chat.visit_thread(thread_1)
channel.hover_message(message_1)
thread.hover_message(last_message)
expect(page).to have_css(
".chat-thread[data-id='#{thread_1.id}'] [data-id='#{message_1.id}'] .chat-message.is-active",
".chat-thread[data-id='#{thread_1.id}'] [data-id='#{last_message.id}'] .chat-message.is-active",
)
end
end

View File

@ -81,7 +81,7 @@ RSpec.describe "Drawer", type: :system, js: true do
channel_page.hover_message(message_1)
expect(page).to have_css(".chat-message-actions-container")
find(".chat-composer-input").send_keys(:escape)
find(".chat-composer__input").send_keys(:escape)
expect(page).to have_no_css(".chat-message-actions-container")
end

View File

@ -88,8 +88,15 @@ describe "Thread indicator for chat messages", type: :system, js: true do
channel_page.reply_to(message_without_thread)
channel_page.fill_composer("this is a reply to make a new thread")
channel_page.click_send_message
expect(channel_page).to have_thread_indicator(message_without_thread)
new_thread = message_without_thread.reload.thread
new_thread = nil
try_until_success(timeout: 5) do
new_thread = message_without_thread.reload.thread
expect(new_thread).to be_present
end
expect(page).not_to have_css(channel_page.message_by_id_selector(new_thread.replies.first))
end

View File

@ -19,7 +19,7 @@ module PageObjects
def visit_channel(channel, mobile: false)
visit(channel.url + (mobile ? "?mobile_view=1" : ""))
has_no_css?(".not-loaded-once")
has_no_css?(".chat-channel--not-loaded-once")
has_no_css?(".chat-skeleton")
end

View File

@ -4,25 +4,25 @@ module PageObjects
module Pages
class ChatChannel < PageObjects::Pages::Base
def type_in_composer(input)
find(".chat-composer-input--channel").click # makes helper more reliable by ensuring focus is not lost
find(".chat-composer-input--channel").send_keys(input)
find(".chat-channel .chat-composer__input").click # makes helper more reliable by ensuring focus is not lost
find(".chat-channel .chat-composer__input").send_keys(input)
end
def fill_composer(input)
find(".chat-composer-input--channel").click # makes helper more reliable by ensuring focus is not lost
find(".chat-composer-input--channel").fill_in(with: input)
find(".chat-channel .chat-composer__input").click # makes helper more reliable by ensuring focus is not lost
find(".chat-channel .chat-composer__input").fill_in(with: input)
end
def click_composer
find(".chat-composer-input--channel").click # ensures autocomplete is closed and not masking anything
find(".chat-channel .chat-composer__input").click # ensures autocomplete is closed and not masking anything
end
def click_send_message
find(".chat-composer .send-btn:enabled").click
find(".chat-composer--send-enabled .chat-composer__send-btn").click
end
def message_by_id_selector(id)
".chat-live-pane .chat-messages-container .chat-message-container[data-id=\"#{id}\"]"
".chat-channel .chat-messages-container .chat-message-container[data-id=\"#{id}\"]"
end
def message_by_id(id)

View File

@ -24,17 +24,17 @@ module PageObjects
end
def type_in_composer(input)
find(".chat-composer-input--thread").click # makes helper more reliable by ensuring focus is not lost
find(".chat-composer-input--thread").send_keys(input)
find(".chat-thread .chat-composer__input").click # makes helper more reliable by ensuring focus is not lost
find(".chat-thread .chat-composer__input").send_keys(input)
end
def fill_composer(input)
find(".chat-composer-input--thread").click # makes helper more reliable by ensuring focus is not lost
find(".chat-composer-input--thread").fill_in(with: input)
find(".chat-thread .chat-composer__input").click # makes helper more reliable by ensuring focus is not lost
find(".chat-thread .chat-composer__input").fill_in(with: input)
end
def click_composer
find(".chat-composer-input--thread").click # ensures autocomplete is closed and not masking anything
find(".chat-thread .chat-composer__input").click # ensures autocomplete is closed and not masking anything
end
def send_message(id, text = nil)
@ -45,7 +45,9 @@ module PageObjects
end
def click_send_message(id)
find(thread_selector_by_id(id)).find(".chat-composer .send-btn:enabled").click
find(thread_selector_by_id(id)).find(
".chat-composer--send-enabled .chat-composer__send-btn",
).click
end
def has_message?(thread_id, text: nil, id: nil)
@ -73,6 +75,18 @@ module PageObjects
)
end
end
def hover_message(message)
message_by_id(message.id).hover
end
def message_by_id(id)
find(message_by_id_selector(id))
end
def message_by_id_selector(id)
".chat-thread .chat-messages-container .chat-message-container[data-id=\"#{id}\"]"
end
end
end
end

View File

@ -36,7 +36,7 @@ module PageObjects
end
def has_open_channel?(channel)
has_css?("#{VISIBLE_DRAWER} .chat-live-pane[data-id='#{channel.id}']")
has_css?("#{VISIBLE_DRAWER} .chat-channel[data-id='#{channel.id}']")
end
end
end

View File

@ -23,7 +23,7 @@ RSpec.describe "Shortcuts | chat composer", type: :system, js: true do
it "adds bold text" do
chat.visit_channel(channel_1)
composer = find(".chat-composer-input")
composer = find(".chat-composer__input")
composer.send_keys([key_modifier, "b"])
expect(composer.value).to eq("**strong text**")
@ -34,7 +34,7 @@ RSpec.describe "Shortcuts | chat composer", type: :system, js: true do
it "adds italic text" do
chat.visit_channel(channel_1)
composer = find(".chat-composer-input")
composer = find(".chat-composer__input")
composer.send_keys([key_modifier, "i"])
expect(composer.value).to eq("_emphasized text_")
@ -45,7 +45,7 @@ RSpec.describe "Shortcuts | chat composer", type: :system, js: true do
it "adds preformatted text" do
chat.visit_channel(channel_1)
composer = find(".chat-composer-input")
composer = find(".chat-composer__input")
composer.send_keys([key_modifier, "e"])
expect(composer.value).to eq("`indent preformatted text by 4 spaces`")
@ -71,8 +71,8 @@ RSpec.describe "Shortcuts | chat composer", type: :system, js: true do
chat.visit_channel(channel_1)
channel_page.message_thread_indicator(thread.original_message).click
composer = find(".chat-composer-input--channel")
thread_composer = find(".chat-composer-input--thread")
composer = find(".chat-channel .chat-composer__input")
thread_composer = find(".chat-thread .chat-composer__input")
composer.send_keys([key_modifier, "i"])
expect(composer.value).to eq("_emphasized text_")
@ -98,7 +98,7 @@ RSpec.describe "Shortcuts | chat composer", type: :system, js: true do
chat.visit_channel(channel_1)
expect(channel_page).to have_message(id: message_1.id)
find(".chat-composer-input").send_keys(:arrow_up)
find(".chat-composer__input").send_keys(:arrow_up)
expect(page.find(".chat-composer-message-details")).to have_content(message_1.message)
end
@ -111,7 +111,7 @@ RSpec.describe "Shortcuts | chat composer", type: :system, js: true do
page.driver.browser.network_conditions = { offline: true }
channel_page.send_message("Hello world")
find(".chat-composer-input").send_keys(:arrow_up)
find(".chat-composer__input").send_keys(:arrow_up)
expect(page).to have_no_css(".chat-composer-message-details")
end

View File

@ -36,7 +36,7 @@ RSpec.describe "Shortcuts | drawer", type: :system, js: true do
expect(page).to have_css(".chat-drawer.is-expanded")
drawer.open_channel(channel_1)
find(".chat-composer-input").send_keys(:escape)
find(".chat-composer__input").send_keys(:escape)
expect(page).to have_no_css(".chat-drawer.is-expanded")
end
@ -49,7 +49,7 @@ RSpec.describe "Shortcuts | drawer", type: :system, js: true do
page.send_keys("e")
expect(find(".chat-composer-input").value).to eq("")
expect(find(".chat-composer__input").value).to eq("")
end
end
@ -59,15 +59,15 @@ RSpec.describe "Shortcuts | drawer", type: :system, js: true do
expect(page).to have_selector(".chat-drawer[data-chat-channel-id=\"#{channel_1.id}\"]")
find(".chat-composer-input").send_keys(%i[alt arrow_down])
find(".chat-composer__input").send_keys(%i[alt arrow_down])
expect(page).to have_selector(".chat-drawer[data-chat-channel-id=\"#{channel_2.id}\"]")
find(".chat-composer-input").send_keys(%i[alt arrow_down])
find(".chat-composer__input").send_keys(%i[alt arrow_down])
expect(page).to have_selector(".chat-drawer[data-chat-channel-id=\"#{channel_1.id}\"]")
find(".chat-composer-input").send_keys(%i[alt arrow_up])
find(".chat-composer__input").send_keys(%i[alt arrow_up])
expect(page).to have_selector(".chat-drawer[data-chat-channel-id=\"#{channel_2.id}\"]")
end

View File

@ -19,7 +19,7 @@ RSpec.describe "Shortcuts | full page", type: :system, js: true do
page.send_keys("e")
expect(find(".chat-composer-input").value).to eq("e")
expect(find(".chat-composer__input").value).to eq("e")
end
end
end

View File

@ -182,7 +182,7 @@ RSpec.describe "Sidebar navigation menu", type: :system, js: true do
visit("/")
expect(sidebar_page.dms_section.find(".channel-#{dm_channel_1.id}")["title"]).to eq(
"Chat with @&lt;script&gt;alert(&#x27;hello&#x27;)&lt;/script&gt;",
"Chat in @&lt;script&gt;alert(&#x27;hello&#x27;)&lt;/script&gt;",
)
end
end

View File

@ -27,10 +27,10 @@ RSpec.describe "Silenced user", type: :system, js: true do
)
end
it "removes the send button" do
it "disables the send button" do
chat.visit_channel(channel_1)
expect(page).to have_css(".send-btn[disabled]")
expect(page).to have_css(".chat-composer__send-btn[disabled]")
end
it "prevents reactions" do

View File

@ -25,7 +25,6 @@ describe "Uploading files in chat messages", type: :system, js: true do
end
expect(page).to have_css(".chat-composer-upload .preview .preview-img")
expect(page).to have_content(File.basename(file_path))
channel.send_message("upload testing")
@ -61,7 +60,6 @@ describe "Uploading files in chat messages", type: :system, js: true do
channel.click_action_button("chat-upload-btn")
end
expect(page).to have_content(File.basename(file_path))
expect(find(".chat-composer-upload")).to have_content("Processing")
# image processing clientside is slow! here we are waiting for processing
@ -101,10 +99,11 @@ describe "Uploading files in chat messages", type: :system, js: true do
it "allows deleting uploads" do
chat.visit_channel(channel_1)
channel.open_edit_message(message_2)
find(".chat-composer-upload").find(".remove-upload").click
find(".chat-composer-upload").hover
find(".chat-composer-upload__remove-btn").click
channel.click_send_message
expect(channel.message_by_id(message_2.id)).not_to have_css(".chat-uploads")
expect(message_2.reload.uploads).to be_empty
try_until_success(timeout: 5) { expect(message_2.reload.uploads).to be_empty }
end
it "allows adding more uploads" do
@ -118,13 +117,13 @@ describe "Uploading files in chat messages", type: :system, js: true do
end
expect(page).to have_css(".chat-composer-upload .preview .preview-img", count: 2)
expect(page).to have_content(File.basename(file_path))
channel.click_send_message
expect(page).not_to have_css(".chat-composer-upload")
expect(page).to have_css(".chat-img-upload", count: 2)
expect(message_2.reload.uploads.count).to eq(2)
try_until_success(timeout: 5) { expect(message_2.reload.uploads.count).to eq(2) }
end
end