In the past this was happening on scroll so we needed to be very conservative here. Also, if we wait too much theres a visible element flashing so this PR attempts to compute right away, and a second time 100ms later in case the first one happened too early.
This will avoid the messages actions floating around while scrolling. Note it's not testing the thread counterpart yet as I have a plan in mind to tests channels and threads in a clean way in the near future.
Prior to this fix uploads event could end up in the wrong textarea. This will most importantly allow pasting an image in the thread composer.
Also fixes a minor padding issue on thread when uploads are associated to it.
`.chat-channel` had `300px` min width, when `.chat-drawer` was `250px`, resulting in overflowing channel when in drawer. This commits ensures the limits are always set at `250px`.
- using BEM notation
- making animation linear instead of default ease
- small tweaks to composer state (disabled/send-disabled/send-enabled)
- fixing bug with disabled composer on mobile
1. `this.chat.activeChannel = null` was being done in twice
2. using `willTransition()` and checking transition.to.name prefix for route cleanup rather than using `deactivate()` was unnecessarily verbose and could be premature (if something aborted the transition you'd end up in a broken state)
3. `activeChannel` on Chat service can be null, check for that before accessing
It's very hard to repro but under specific circumstances I suspect it was possible for this sequence to happen:
- set message TEXT
- cooking starts
- set message COOKED through another mean (like a message bus)
- the cooking started sooner finished and erases the cooked set at the step before causing the message to have the incorrect cooked
After removing `TextareaTextManipulation` from `ChatComposer` and using `TextareaInteractor` as a proxy, one function has been forgotten: `paste(event)` which is not available in glimmer components anymore, and even less avaiable now that the mixin is not tied to a component anymore but a real DOM node. As a solution we now add a manual paste event listener which will call `paste(event)`.
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 was reverted in 38cebd3ed5.
The issue was that I was using Discourse.redis.delete_prefixed
which does a slow redis KEYS lookup, which is not advised in
production. This commit removes that, and also ensures the periodical
thread count update only happens if threading is enabled.
I changed to use a redis INCR/DECR for reply count
cache. This avoids a round trip to redis to GET the current
count, and also avoids multi-process issues, where
if there's two processes trying to increment at the
same time, they may both receive the same value, add one
to it, then both write the same value back.
Then, it's only n+1 instead of n+2.
This also prevents almost all chat scheduled jobs from
running if chat is disabled, the only one remaining is
the message retention job.
This commit moves the category channel creation out
of the Chat::Api::Channel controller and into a
dedicated CreateCategoryChannel service. A follow up
commit will move the DM channel creation out of
the old DirectMessageChannelCreator service.
Also includes a new on_model_errors helper
for chat service class usage, that collects model
validation errors to present in a nice way.
---------
Co-authored-by: Loïc Guitaut <loic@discourse.org>
This commit fixes an issue where if you pressed a format
shortcut (e.g. bold, italic, code) for the composer and
you had the thread panel open as well, the shortcut would
trigger in both composers, not just the one that was focused.
We currently don't have a nice UI to show unread messages for the thread,
and it will take some time to create one. For now, this commit makes it so
new messages inside a thread do not count towards a chat channel's unread
counts, and new messages sent in a thread do not update a user's `last_read_message_id`
for a channel.
In addition, this PR refactors the `Chat::ChannelFetcher` to use the `Chat::ChannelUnreadsQuery`
query class for consistency, and made said class able to return zeroed-out records
for channels the user is not a member of.
Finally, a small bug is fixed here where if a user's `last_read_message_id` for
a channel was a thread's OM ID, then the thread OM would not show in the
main channel stream for them until another reply to the channel was posted.
This commit introduces a redis cache over the top of the thread
replies_count DB cache, so that we can quickly and accurately
increment/decrement the reply count for all users and not have
to constantly update the database-level count. This is done so
the UI can have a count that is displayed to the users on each
thread indicator, that appears to live update on each chat
message create/trash/recover inside the thread.
This commit also introduces the `Chat::RestoreMessage` service
and moves the restore endpoint into the `Api::ChannelMessages`
controller as part of incremental migrations to move things out
of ChatController.
Finally, this commit refactors `Chat::Publisher` to be less repetitive
with its `MessageBus` sending code.
Followup to bd5c5c4b5f,
this commit hooks up the bulk delete events for chat
messages inside the thread panel, by fanning out the
deleted message IDs based on whether they belong to
a thread or not.
Also adds a system spec to cover this case, as previously
the bulk delete event would have been broken with an incorrect
`typ` rather than `type` hash key.
This error was only happening on mobile, note we also already have a (mobile) test (plugins/chat/spec/system/transcript_spec.rb:184) for this which was passing as it's only happening at a specific speed. I don't want to complicate the test too much for this case, will reconsider if it regresses again.
This regression happened in bd5c5c4b5f and is due to `message_bus_targets = calculate_publish_targets(chat_channel, chat_message)` expecting a `chat_channel` which was only defined after.
Example exception in logs:
```
Job exception: undefined local variable or method `chat_channel' for Chat::Publisher:Module
/var/www/discourse/plugins/chat/app/services/chat/publisher.rb:91:in `publish_processed!'
/var/www/discourse/plugins/chat/app/jobs/regular/chat/process_message.rb:21:in `block in execute'
/var/www/discourse/lib/distributed_mutex.rb:53:in `block in synchronize'
/var/www/discourse/lib/distributed_mutex.rb:49:in `synchronize'
/var/www/discourse/lib/distributed_mutex.rb:49:in `synchronize'
/var/www/discourse/lib/distributed_mutex.rb:34:in `synchronize'
/var/www/discourse/plugins/chat/app/jobs/regular/chat/process_message.rb:7:in `execute'
/var/www/discourse/app/jobs/base.rb:249:in `block (2 levels) in perform'
```
This commit also:
- adds a spec to ensure oneboxing is not regressing anymore
- increment the version on message processed to ensure callbacks are correctly ran
Note we should also have more tests in `Chat::Publisher`, this will be done when we move it to a proper service.
<!-- NOTE: All pull requests should have tests (rspec in Ruby, qunit in JavaScript). If your code does not include test coverage, please include an explanation of why it was omitted. -->
This codepath was responsible to scroll to the first emoji of a section, however `scrollIntoView` was not super reliable and was also causing the whole page to scroll with drawer. This is also simply not necessary code as native focus behavior will scroll to the element.
This commit introduces a ChatChannelPaneSubscriptionsManager
and a ChatChannelThreadPaneSubscriptionsManager that inherits
from the first service that handle MessageBus subscriptions
for the main channel and the thread panel respectively.
This necessitated a change to Chat::Publisher to be able to
send MessageBus messages to multiple channels based on whether
a message was an OM for a thread, a thread reply, or a regular
channel message.
An initial change to update the thread indicator with new replies
has been done too, but that will be improved in future as we have
more data to update on the indicators.
Still remaining is to fully move over the handleSentMessage
functionality which includes scrolling and new message indicator
things.
Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
- Back button in drawer will bring you back to channel
- Larger font for thread indicator
- Prevents screen flashing due to clearing messages when they were already loaded
- Fixes a bug where did-update params were inverted causing an error when expanding/collapsing drawer
This feature will allow sites to define which emoji are not allowed. Emoji in this list should be excluded from the set we show in the core emoji picker used in the composer for posts when emoji are enabled. And they should not be allowed to be chosen to be added to messages or as reactions in chat.
This feature prevents denied emoji from appearing in the following scenarios:
- topic title and page title
- private messages (topic title and body)
- inserting emojis into a chat
- reacting to chat messages
- using the emoji picker (composer, user status etc)
- using search within emoji picker
It also takes into account the various ways that emojis can be accessed, such as:
- emoji autocomplete suggestions
- emoji favourites (auto populates when adding to emoji deny list for example)
- emoji inline translations
- emoji skintones (ie. for certain hand gestures)
This commit introduces a new thread indicator for channels with `threading_enabled`
set to true and the `enable_exp` site setting set to true. In addition, in the main channel
stream we now hide all messages that are linked to threads except for the original message,
disabling the concept of an "echo mode" for now, we may revisit this in future. We also
remove the jigsaw puzzle "Open Thread" button for message actions, since the thread
indicator can just be used instead.
This also stops the `Chat::Publisher` from sending any messages related to chat
messages that are linked to a thread, unless that chat message is the OM of the
thread. A subsequent PR will link up all MessageBus events within the thread panel,
and for the message indicators.
Another subsequent PR will add the excerpt of the latest message in each thread,
as well as the avatars of the users messaging in the thread.
Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
- rounded active style for messages
- better active state on chat message actions rows
- ensures long press on a message is not selecting text
- slightly improved messages actions animation and background fading
- ensures chat emoji picker is not cutoff on right side
- removes old legacy code related to hovered message
Followup to c1dc6a2db4,
this commit just missed removing one of the @computed
decorators which was causing multiple active channels
to show in the sidebar. Fix the issue and introduce a
system spec to catch this.
- clicking empty area on the header will toggle collapse/expand it
- applies a background on hover of the channel title
- active state for small buttons
- the back button now has the correct icon color when hovered
- adds missing focus state for heade buttons icons
When hovering the chat message actions we are technically not hovering the message anymore, which was removing the background and is slightly unexpected. This commit ensures we keep this background until closing the message actions.
This PR primarily fixes this case:
- USER A message
- USER B message
- USER B reply to USER A message <-- not showing user info when it should
Moreover, this commit also improves the spec to correctly test more cases.
This commit is a major overhaul of how chat message actions work, to make it so they are reusable between the main chat channel and the chat thread panel, as well as many improvements and fixes for the thread panel.
There are now several new classes and concepts:
* ChatMessageInteractor - This is initialized from the ChatMessage, ChatMessageActionsDesktop, and ChatMessageActionsMobile components. This handles permissions about what actions can be done for each
message based on the context (thread or channel), handles the actions themselves (e.g. copyLink, delete, edit),
and interacts with the pane of the current context to modify the UI
* ChatChannelThreadPane and ChatChannelPane services - This represents the UI context which contains the
messages, and are mostly used for state management for things like message selection.
* ChatChannelThreadComposer and ChatChannelComposer - This handles interaction between the pane, the
message actions, and the composer, dealing with reply and edit message state.
* Scrolling logic for the messages has now been moved to a helper so it can be shared between the main channel pane and the thread pane
* Various improvements with the emoji picker on both mobile and desktop. The DOM node of each component is now located outside of the message which prevents a large range of issues.
The thread panel now also works in the chat drawer, and the thread messages have less
actions than the main panel, since some do not make sense there (e.g. moving messages to
a different channel). The thread panel title, excerpt, and message sender have also been removed
for now to save space.
This gives us a solid base to keep expanding on and fixing up threads. Subsequent PRs will
make the thread MessageBus subscriptions work and disable echo mode
for the initial release of threads.
Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
This commit adds a system to generate CSS variables and classes for categories
and hashtags, which will be used in an effort to remove baked icons for hashtags
and add color to those icons.
This is in two parts. First I added an initializer generate a category color CSS
variable style tag in the head tag that looks like this:
```css
:root {
--category-1-color: #0088CC;
--category-2-color: #808281;
--category-3-color: #E45735;
--category-4-color: #A461EF;
--category-5-color: #ee56c9;
--category-6-color: #da28c2;
--category-7-color: #ab8b0a;
--category-8-color: #45da37;
...
}
```
The number is the category ID. This only generates CSS variables for categories
the user can access based on `site.categories`. If you need the parent color variable
you can just use the `category.parentCategory.id` to get it.
Then, I added an initializer to generate a hashtag CSS style tag using these variables.
Only the category and channel hashtags need this, the category one generates the
background-gradient needed for the swatch, and the channel just generates a color
for the icon. This is done in an extendable way using the new `api.registerHashtagType`
JS plugin API:
```css
hashtag-color--category-1 {
background: linear-gradient(90deg, var(--category-1-color) 50%, var(--category-1-color) 50%);
}
hashtag-color--category-2 {
background: linear-gradient(90deg, var(--category-2-color) 50%, var(--category-2-color) 50%);
}
hashtag-color--category-5 {
background: linear-gradient(90deg, var(--category-5-color) 50%, var(--category-4-color) 50%);
}
...
.hashtag-color--channel-4 {
color: var(--category-12-color);
}
.hashtag-color--channel-92 {
color: var(--category-24-color);
}
```
Note if a category has a parent, its color is used in the gradient correctly. The numbers
here are again IDs (e.g. channel ID, category ID) and the channel’s chatable ID is used
to find the category color variable.