When a thread is created / a new message is created in the
thread, we want to make sure that the original message user
has a membership for that thread, otherwise they will not
receive unread indicators for messages in the thread.
This commit attempts to fix the case where the messages loaded initially don't fill the screen. It would prevent user to scroll and as a result to load more.
There are multiple fixes in this commit:
- the main fix is removing this code which was preventing the actual fill:
```javascript
// prevents an edge case where user clicks bottom arrow
// just after scrolling to top
if (loadingPast && this.#isAtBottom()) {
return;
}
```
- ensures we always give a page site to the `chatApi.channel(...)` call if we have one, in the current state when `fetchFromLastRead` was `true` we would not set `args.page_size`
- ensures the `query_paginated_messages` is having a valid page size, which is not nil and not > `MAX_PAGE_SIZE`
- write a spec for the autofill, it was a challenging spec to write but it should give us the confidence we need here
Since 5cce829 and the new
channel view builder, we have no need of these obsolete
routes which have way too much logic in the controller, which
has been superseded by the view builder anyway.
Remove the routes and update the channel message loading to use it.
* Moved the settings cog from thread list to thread and
put it in a new header component
* Remove thread original message component, no longer needed
and the list item and thread indicator styles/content
will be quite different
* Start adding content (unread indicator etc.) to the thread
list item and changing structure to be more like designs
* Serialize the last thread reply when opening the thread index,
show in list and update with message bus
In some cases activeChannel can be null so this will error,
also it is limiting to have this code in chatApi. Instead
move to the threads manager, and also lean on channelsManager.find
to get the channel from the cache instead, which will not error.
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.
#### FIX: Do not use client lastReadMessageId when fetching channel messages
We had an issue where the following happened:
1. User opened channel and saw the last message, and we set the
lastReadMessageId on the server and the client
2. User navigated to another channel
3. Another user deleted the message in the original channel
4. The first user navigated back to the original channel before
the MessageBus event for the deleted message arrived, and got
a 404 error because we were sending the deleted lastReadMessageId as
target_message_id to the channel controller.
Instead of this which is a bit flaky and is hard to cover all
the issues for, instead we can pass a fetch_from_last_read boolean
param to the channels controller, and just get the user's
last_read_message_id straight from the database to use for the
target_message_id. This gets rid of any sources of race conditions
or lack of updates from MessageBus.
#### FIX: Include missing memberships for thread tracking publish
When we publish the channel/message tracking state for a
user and that message was a thread reply the publisher
was erroring because we were not telling Chat::TrackingStateReportQuery
to return missing memberships (which have zeroed out unread counts)
as well, which is what we do for the channel tracking state here.
Also just make sure that the TrackingStateReport does not error
when passed an ID it doesn't have data for.
This commit follows up b6c5a2da08
by serializing the user's thread memberships in these cases:
1. When we do the initial channel fetch with messages, we get
all threads and all the user's thread memberships for those
messages.
2. When the thread list is fetched, we get all the user's memberships
in that list.
3. When the single thread is fetched, either from opening it from
the list, an OM indicator, or just from doing .find() on the
manager when a new MessageBus message comes in
This will let us track the lastReadMessageId on the client, and
will also let us fix an issue where the unread indicator in the
channel header was incrementing for every thread that got a
new message, regardless of whether the user was a member.
This commit adds the thread index and individual thread
in the index list unread indicators, and wires up the message
bus events to mark the threads as read/unread when:
1. People send a new message in the thread
2. The user marks a thread as read
There are several hacky parts and TODOs to cover before
this is more functional:
1. We need to flesh out the thread scrolling and message
visibility behaviour. Currently if you scroll to the end
of the thread it will just mark the whole thread read
unconditionally.
2. We need to send down the thread current user membership
along with the last read message ID to the client and
update that with read state.
3. We need to handle the sidebar unread dot for when threads
are unread in the channel and clear it based on when the
channel was last viewed.
4. We need to show some indicator of thread unreads on the
thread indicators on original messages.
5. UI improvements to make the experience nicer and more
like the actual design rather than just placeholders.
But, the basic premise around incrementing/decrementing the
thread overview count and showing which thread is unread
in the list is working as intended.
This commit regroups 3 changes
- serializes channel ID and json draft when calling the debouncer instead of inside the debounced function, it seems very unlikely to happen, but in a case where the debouncing wouldn't be canceled and the new message not set yet, we could save the draft on an invalid channel
- cancel persist draft handler when changing channel
- ensures we exit early when channel is not set
Very fast or specific mouse moves could allow to leave a message actions menu without reseting the active message. This commit should ensure we correctly catch this event.
No test as it's hard and not reliable to reproduce these in a test.
This should also make `message_notifications_with_sidebar_spec.rb` more resilient as we are now checking for `is-persisted` class instead of checking for the absence of `is-staged`.
This commit attempts to correctly change draft when the channel changes. It moves responsibility to the composer instead of the channel.
A new service `chatDraftsManager` is being introduced here to allow finer control and pave the way for future thread draft support.
These changes also now allow an editing message to be stored as a draft.
This PR adds status to mentions in chat and makes those mentions receive live updates.
There are known unfinished part in this implementation: when posting a message, status on mentions on that message appears immediately, but only if a user used autocomplete when typing the message. If user copy and paste a message with mentions into chat composer, those mentions won't have user status on them.
PRs with fixes for both problems are following soon.
Preparations for this PR that were made previously include:
- DEV: correct a relationship – a chat message may have several mentions 0dcfd7ddec
- DEV: extract the logic for extracting and expanding mentions from ChatNotifier 75b81b6854
- DEV: Always create chat mention records fa543cda06
- DEV: better split create_notification! and send_notifications logic e292c45924
- DEV: more tests for mentions when updating chat messages e7292e1682
- DEV: extract updating status on mentions into a lib function e49d338c21
- DEV: Create and update chat message mentions earlier 35a414bb38
- DEV: Create a chat_mention record when self mentioning 2703f2311a
- DEV: When deleting a chat message, do not delete mention records f4fde4e49b
Followup ae3231e140, when a
message is trashed we already update the lastReadMessageId of
all users in the channel to the latest non-deleted message on
the server side. However we didn't propagate this to the client,
so in some cases when we did the following:
1. Delete the last message in the channel
2. Switch to another channel
3. Switch back to the original
We would get a 404 error from the target message ID being looked
up still being the old lastReadMessageId (now deleted) for the
user's channel membership.
All we need to do is send the last not-deleted message ID for
the channel (or thread) to all the member users.
Before this commit the following actions would have shown the issue:
- visit a thread
- changes the width of the side panel
- open threads list
- the size has reverted to previous state
This was caused by the width change to not correctly be tracked.
It easier to check for presence in this case that to check for something not present, as depending on performance of the machine running the test this could take sometime to be changed and the test would fail.
This issue was especially visible in tests. the `@debounce(100)` was not cancelled when changing channel which was causing 404s as we were trying to load messages on a channel which was deleted as the channel has been destroyed at the end of the test.
This is still not a perfect solution, as we can only cancel the start of `fetchMessages`, but we can't cancel the actual `chatApi.channel` request which result can potentially happens after channel changed, which we try to mitigate with various checks on to ensure visible channel == loaded messages channel.
This commit also tries to make handler naming and cancelling more consistent.
<!-- 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 commit makes it easier to ensure each button have a similar behavior: hover, disabled, focus...
It also removes some dead code (__inline-button) and fixes a bug where the emoji button didnt have the right un-focused color (it was more visible than it should.
This commit makes some fundamental changes to how hashtag cooking and
icon generation works in the new experimental hashtag autocomplete mode.
Previously we cooked the appropriate SVG icon with the cooked hashtag,
though this has proved inflexible especially for theming purposes.
Instead, we now cook a data-ID attribute with the hashtag and add a new
span as an icon placeholder. This is replaced on the client side with an
icon (or a square span in the case of categories) on the client side via
the decorateCooked API for posts and chat messages.
This client side logic uses the generated hashtag, category, and channel
CSS classes added in a previous commit.
This is missing changes to the sidebar to use the new generated CSS
classes and also colors and the split square for categories in the
hashtag autocomplete menu -- I will tackle this in a separate PR so it
is clearer.
- few improved alignments
- displays emoji picker button inline on desktop
- keeps composer focused when focusing dropdown button
- align buttons to bottom when increasing height of textarea
- max-height of textarea is now linked to the height of the screen
Co-authored-by: chapoi <charlie@discourse.org>
This commit moves message lookup and querying to the
/chat/api/channel/:id endpoint and adds the ability
to query the tracking state overview for threads as well
as the threads and thread tracking state for any thread
original messages found.
This will allow us to get an initial overview of thread
tracking for a user when they first enter a channel, rather
than pre-emptively loading N threads and tracking state
for those across all channels on the current user serializer,
which would be expensive.
This initial overview will be used in subsequent PRs to
flesh out the thread unread indicators in the UI.
This also moves many chunks of code that were in services
to reusable Query classes, since use of services inside
services is discouraged.
Wrap scroll to bottom inside a next block to ensure the message has correctly been added to the array before actually attempt to scroll.
This commit also removes an unnecessary line which was essentially adding the message two times with no real consequences as we are uniq on ID.
- uses current user as user for fabricators, allows for correct avatar image and presence indicator
- uses a non existing channel ID to avoid setting a draft of an existing channel
- attempts to make color toggle more reliable
- Improves styleguide support
- Adds toggle color scheme to styleguide
- Adds properties mutators to styleguide
- Attempts to quit a session as soon as done with it in system specs, this should at least free resources faster
- Refactors fabricators to simplify them
- Adds more fabricators (uploads for example)
- Starts implementing components pattern in system specs
- Uses Chat::Message creator to create messages in system specs, this should help to have more real specs as the side effects should now happen
Followup from 9953a6edd9,
which broke an issue fixed in e8d6277062.
In the refactor we did not update the place where we resync
channel tracking based on onPresenceChange to use the new
tracking object we added to the serializer, and to directly
update channel.tracking
We had safe-area-inset-bottom still set at a wrong place which was causing issues but was also useful to some cases. This commit removes it and ensures the affected cases are corrected.
This is only the first steps of a redesign
- redesigns the buttons to have a larger hitzone
- generally bigger composer
- clicking near textarea focuses the input
- relies on the fact that safe-area-inset-bottom is set globally and doesn’t need to be set in sub components
This moves chat tracking state calculation for channels
and threads into a central Chat::TrackingStateManager service, that
serves a similar purpose to the TopicTrackingState model
in core.
This service calls down to these query classes:
* ThreadUnreadsQuery
* ChannelUnreadsQuery
To get the unread_count and mention_count for the appropriate
channels and threads.
As well as this, this commit refactors the client-side chat
tracking state.
Now, there is a central ChatTrackingStateManager Ember Service
so all tracking is accessible and can be counted from one place,
which can also initialize tracking from an initial payload.
The actual tracking counts are now maintained in a ChatTrackingState
class that is initialized on the `.tracking` property of both channel and
thread objects.
This removes the attributes on UserChatChannelMembership and decoration
of said membership from ChannelFetcher, preferring instead to have an additional
object for tracking in the JSON.
* DEV: add new thread icon
* FIX: Use new thread icon, fix typo in SVG
UX: move the thread list icon to the right of
the collapse button
---------
Co-authored-by: Martin Brennan <martin@discourse.org>
This commit also removes safe-area-inset-bottom when keyboard is displayed to avoid having a taller than needed space between composer and replying indicator.
Before this commit chat was applying a fixed height on everything under the `/chat` route. It's only really needed on the channel page with the composer at the bottom of the page.
This commits makes the following changes:
- moves height limitation from `#main-outlet-wrapper` to `.chat-channel`
- makes browse channel page and members list page full height and rely on main document scrollbar
- adds height computation for draft header and direct message creator block to ensure the height is correct when creating a draft channel
- makes chat index full height to rely on the browser scrollbar. As a result the <kbd> + </kbd> button used on mobile to create a direct message as been moved out of `<ChannelsList>` into the chat index template
- sidebar height was relying on chat setting a max height, as a result the height computation of sidebar has been changed to work correctly, especially with an opened keyboard on mobile or ipad
Hard to write a test for this behavior, this is a micro optimisation which doesn’t change the behavior but only makes it smoother by happening right before async request.
When using `navigator.virtualKeyboard.overlaysContent = false` we can rely on using only the resize event. Also attempts to no over trigger `setProperty` when value didn't change.
This issue was for example possibly causing the last visit indicator to be reset by `sent` messages events.
The following was happening:
- a user (bob) had a last message bus ID of 1 on a channel (id:1) subscription
- bob then go to another channel (id:2), unsubscribing from updates of channel (id:1)
- another user (laura) then send messages to channel (id:1)
- bob goes back to channel (id:1)
At this point we we doing in the same sequence:
- loading channel with messages, getting a new last message bus id
- subscribing to updates using the last known message bus id
Most of the times we were lucky enough for this to work (no events while away, or just got the new id in time...) but it was also very likely to do a double fetch of messages as MessageBus would think we were late.
This commit fixes the shift+click multi selection in threads. We were not correctly using the manager of the message and would attempt to find messages in the channel instead of the thread.
The `activeThread` was also not correctly set sometimes.
Also adds tests for message selection in threads.
We were combining both solutions which was apparently causing issues from chrome 113 on Android at least.
The commit will now use `geometrychange` (android) only when available and fallback to `visualViewport` otherwise (iOS).
This feature adds the replying indicator in threads, it uses the same `/chat-reply/CHANNEL_ID` prefix than the channel composer replying indicator as we don't have specific right on threads ATM (if you can access channel, you can access thread). Thread will however use a presence channel name of the following format: `/chat-reply/CHANNEL_ID/thread/THREAD_ID`
This commit also simplifies the computation of `users` to eventually avoid a race-condition leading to a leak of the indicator in another channel/thread.
<!-- 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. -->
Since we have channel message retention which deletes
messages, we can end up with cases where the thread
is still around but the message is deleted. We will
handle the cascade delete in a different commit --
for now we will ensure the thread list lookup handles
this case and doesn't error.
After this change, in order to join a chat channel, a user needs to be in a group with at least “Reply” permission for the category. If the user only has “See” permission, they are able to preview the channel, but not join it or send messages. The auto-join function also follows this new restriction.
---------
Co-authored-by: Martin Brennan <martin@discourse.org>
This commit adds an initial thread list UI. There are several limitations
with this that will be addressed in future PRs:
* There is no MessageBus reactivity, so e.g. if someone edits the original
message of the thread it will not be reflected in the list. However if
the thread title is updated the original message indicator will be updated.
* There is no unread functionality for threads in the list, if new messages
come into the thread there is no indicator in the UI.
* There is no unread indicator on the actual button to open the thread list.
* No pagination.
In saying that, this is the functionality so far:
* We show a list of the 50 threads that the user has most recently participated
in (i.e. sent a message) for the channel in descending order.
* Each thread we show a rich excerpt, the title, and the user who is the OM creator.
* The title is editable by staff and by the OM creator.
* Thread indicators show a title. We also replace emojis in the titles.
* Thread list works in the drawer/mobile.
- do not render sidepanel resizer as it's not usable on mobile
- removes 1px to the bottom spacing, this spacing won't be necessary once we implement chat-replying-indicator for thread
- correctly subscribe/unsubscribe channel
- instantly changes users list
- adds a test for testing channel change
- rewrites tests to be less verbose
- ensures users is always an array
This commit also adds a component test for it and fixes a bug in `chat-channel-archive-status` `#getTopicURL` property which was incorrectly called as a function.
- `ChatChannel`
- `UserChatChannelMembership`
Also creates a new `chat-direct-message` model used as the object for the`chatable` property of the `ChatChannel` when the `ChatChannel` is a direct message channel. When the chatable is a category a real `Category` object will now be returned.
Archive state of a `ChatChannel` is now hold in a `ChatChannelArchive` object.
After a long time with no activity or hidden browser (2.5 minutes), the app will re-sync the chat user-tracking-state to ensure unreads are synced.
We might also need to couple this later with more recovering logic.
Since our recent change of inverting thread scrolling direction it feels more responsive to scroll down in thread panel as soon as message is staged and not after it's actually persisted.
When hovering a thread indicator in a channel we will now append two `<link rel="preload" ...>` to the `<head>` of the document. Clicking on it should be significantly faster.
Co-authored-by: Martin Brennan <martin@discourse.org>
This commit implements all the necessary logic to create thread seamlessly. For this it relies on the same logic used for messages and generates a `staged-id`(using the format: `staged-thread-CHANNEL_ID-MESSAGE_ID` which is used to re-conciliate state client sides once the thread has been persisted on the backend.
Part of this change the client side is now always using real thread and channel objects instead of sometimes relying on a flat `threadId` or `channelId`.
This PR also brings three UX changes:
- thread starts from top
- number of buttons on message actions is dependent of the width of the enclosing container
- <kbd>shift + ArrowUp</kbd> will reply to the last message
* FIX: Link to thread for mentions inside thread
When mentioning a user in a thread, when we send the
notification and display it in the UI we want the URL
of the notification to point to the thread URL to open
the panel, rather than the main channel which is confusing.
For now, we don't have a way to highlight the linked-to message
in the thread, we can revisit this later.
* FIX: Mark mention notifications read when thread opens
Since we have no scrolling/message visibility/thread membership
for now, when a user opens the thread panel we just want to mark
all mention notifications relating to messages in the thread
for the user as read.
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 reverts commit 768851920e.
This was causing issues with the local date popup, cutting off
the top of it, there is no way to overrule an overflow:hidden
on the parent. Not z-index related.
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.
When a chat message is trashed and the message is used
for someone's UserChatChannelMembership#last_read_message_id,
the user would end up with some read state issues until
someone posted a new message in the channel, since we didn't
clear it like we did on bulk message delete.
This commit fixes the issue, and also takes the opportunity
to start a MessagesController in the API namespace, and move
the trash message functionality into the new service format.
- Raises the scroll distance to 250px instead of 100px to show the arrow down button
- Always have a margin on drawer when showing channel list, removes this margin when the scrollbar is apparent
- Makes all scrollbar used in chat look the same through the chat-scrollbar mixin
- Ensures hover state is not persistent on channel row in mobile
- Makes the channel row full width on mobile
This commit introduces a Chat::Publisher and MessageBus endpoint
that allows for updating a user's channel tracking state in bulk for
multiple channels, rather than having to do it for one channel
at a time.
This also required an improvement to ChannelUnreadsQuery -- now
multiple channel IDs can be passed to this to get the unread counts
and mention counts for those channels for a user, also increasing
efficiency rather than having to do a query for every individual
channel.
Followup to #20802
Instead of just marking the state read in JS for each channel
after the AJAX call, we can instead just rely on the MessageBus
user-tracking-state chat channel, and publish the state to all
the channels affected in MarkAllUserChannelsRead. This will make
it so the blue dots for the channels are cleared across all tabs.
This manual set was happening only after the request so was not much faster than just waiting on message bus update. It's not by itself changing any behavior or fixing any bug but it makes reasoning about the whole state easier as it happens in only one central place.
Followup to a0381157e9, we just
need to make sure we set currentUserMembership.last_read_message_id
to the last_read_message_id from the updated memberships after marking
all channels read, otherwise we do not scroll to the bottom and still
show the "last visit" separators in channels that have been
marked read.
This commit adds a keyboard shortcut (Shift+ESC) for chat which marks all
of the chat channels that the user is currently a following member of as read,
updating their `last_read_message_id`. This is done via a new service.
It also includes some refactors and controller changes:
* The old mark message read route from `ChatController` is now supplanted
by the `Chat::Api::ReadsController#update` route.
* The new controller can handle either marking a single or all messages read,
and uses the correct service based on the route and params.
* The `UpdateUserLastRead` service is now used (it wasn't before), and has been slightly
updated to just use the guardian user ID.
There are many situations that may cause users to lose permission to
send messages in a chat channel. Until now we have relied on security
checks in `Chat::ChatChannelFetcher` to remove channels which the
user may have a `UserChatChannelMembership` record for but which
they do not have access to.
This commit takes a more proactive approach. Now any of these following
`DiscourseEvent` triggers may cause `UserChatChannelMembership`
records to be deleted:
* `category_updated` - Permissions of the category changed
(i.e. CategoryGroup records changed)
* `user_removed_from_group` - Means the user may not be able to access the
channel based on `GroupUser` or also `chat_allowed_groups`
* `site_setting_changed` - The `chat_allowed_groups` was updated, some
users may no longer be in groups that can access chat.
* `group_destroyed` - Means the user may not be able to access the
channel based on `GroupUser` or also `chat_allowed_groups`
All of these are handled in a distinct service run in a background
job. Users removed are logged via `StaffActionLog` and then we
publish messages on a per-channel basis to users who had their
memberships deleted.
When the user has a channel they are kicked from open, we show
a dialog saying "You no longer have access to this channel".
When they click OK we redirect them either:
* To their first other public channel, if they have any followed
* The chat browse page if they don't
This is to save on tons of requests from kicked out users getting messages
from other channels.
When the user does not have the kicked channel open, we can just
silently yoink it out of their sidebar and turn off subscriptions.
Prior to this fix, we wouldn't display reactions done on different tabs. So, if user A was reacting on tab 1, tab 2 wouldn't display this reaction.
Since few weeks ago we now have the guarantee to have uniq reactions on a message which should prevent any duplicate.
This commit also removes various skipped tests related to reactions and makes `sign_in` explicit at the beginning of each test.
<!-- 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 commit takes advantage of the `ResizeObserver` to know when dates should be re-computed, it works like this:
```
scrollable-div
-- child-enclosing-div with resize observer
---- message 1
---- message 2
---- message x
```
It also switches to bottom/height for date separators sizing, instead of bottom/top, it prevents a bug where setting the top of the first item (at the top) would cause scrollbar to move to top.
<!-- 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 commit does a couple of things:
* Adds the ability to load messages in the chat thread panel when it is open. This just loads the most recent N messages, same as a channel, and does nothing more, no scrolling or anything like that.
* Displays the messages in an extremely simple unordered list with no additional features.
* Allows posting new messages to the thread, and echoes them into the main channel, but does not respond to any sort of MessageBus events.
I've moved messages/clearMessages/addMessages/findMessage code out of the `ChatChannel` model
and into a new `ChatMessagesManager` class, which is instantiated in both the `ChatChannel` model
and the `ChatThread` model. This allows both to manage messages in the same way via the
`TrackedArray` pattern.
This is all hidden behind experimental flags, there is no way to make this not completely broken
in a single commit. Much more work and refactoring needs to be done first.
Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
This behavior is hard to test as it's mostly fixing a race condition: User A sends a message at the same time than User B, which as a result doesn't cause a scroll for the second message and we don't update last read unless we do a small up and down scroll.
`updateLastRead` is debounced so it has no direct consequences to call it slightly more often than what should ideally be needed.
Prior to this fix, the upload was removed from DOM when collapsed and not decorated again on expand, which was causing lightbox to not get reapplied. The fix is reverting to previous state where content was not removed from DOM.
This commit main goal was to comply with Zeitwerk and properly rely on autoloading. To achieve this, most resources have been namespaced under the `Chat` module.
- Given all models are now namespaced with `Chat::` and would change the stored types in DB when using polymorphism or STI (single table inheritance), this commit uses various Rails methods to ensure proper class is loaded and the stored name in DB is unchanged, eg: `Chat::Message` model will be stored as `"ChatMessage"`, and `"ChatMessage"` will correctly load `Chat::Message` model.
- Jobs are now using constants only, eg: `Jobs::Chat::Foo` and should only be enqueued this way
Notes:
- This commit also used this opportunity to limit the number of registered css files in plugin.rb
- `discourse_dev` support has been removed within this commit and will be reintroduced later
<!-- 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. -->
Non-markdown tags weren't being escaped in chat excerpts. This could be
triggered by editing a chat message containing a tag (self XSS), or by
replying to a chat message with a tag (XSS).
Co-authored-by: Jan Cernik <jancernik12@gmail.com>
Usage:
```javascript
api.addChatDrawerStateCallback(({ isDrawerActive, isDrawerExpanded }) => {
// do something
});
```
Note this commit also uses this new API to add a css class (chat-drawer-active) on the body when the drawer is active.
This fix uses direct `scrollTop` manipulation instead of `scrollIntoView` when we are certain we actually want the bottom of the screen. This avoids a range of issues especially in safari but also chrome where the scroll position was not correct at the end of `scrollIntoView`, especially due to images.
- Will consider a message read only one the bottom of the message has been read
- Will allow to mark a message bigger than the view port as read
- Code should be more performant as the scroll is doing less (albeit more often)
- Gives us a very precise scroll state. Problem with throttling scroll is that you could end up never getting the even where scrollTop is at 0, opening a whole range of edge cases to handle
1. Restore the left margin on both (which reflects the right margin of the scroll bar space)
2. Fix the center alignment of scroll-to-bottom icon
3. Fix the spacing of the `-` character between a date label and "last visit" label
4. Fix the incorrect display the border on date label when at the bottom of viewport
When we introduced `existingUploads` as an arg to the
ChatComposerUploads component, we also introduced a bug where
if multiple uploads were being done at once, and the draft
was saved, then because of didReceiveAttrs we would cancel
the currently uploading files because the draft uploads became
the existingUploads.
To work around this, since we do want to keep this on didReceiveAttrs
for cases when the user opens a draft or edits another message,
the easiest thing to do is to just not save uploads into the chat
draft if there are still uploads in progress. That way only when
all uploads are complete do we make them a part of the draft.
There is a small risk that the user could do something to lose
their uploads in the draft, but it's a better gamble to have
that happen rather than in progress uploads to be cancelled
while the user is waiting for them to be done because of the
draft.
Also changes the uploads system spec back to the old way of
attaching multiple files since that is why it was failing.
- group writes when computing separators positions
- shows skeleton only on initial load
- forces date separator to be pinned when first message to prevent a pinned - not pinned - pinned sequence when loading more in past
- relies on `message.visible` property instead of checking `isElementInViewport`
- attempts to load next/prev messages earlier
- do not scroll to on fetch more
- hides `last visit` text while pinned
Note this test my prove to be flakey, so I might have to remove it or find a different solution. It's extremely complicated to test for something which shouldn't appear in a period of time and is not a present at T=0
This was possible due to specific events which are hard to represent in a test. The provided test is as close as possible to what was happening in production: a message bus event was played on a channel which has just loaded its state with the existing reaction.
On require login sites, the `site` is not setup and as a result `hashtag_configurations` was blank and causing an error when attempting to access `["chat-composer"]` on it.
