Enabling/Disabling threading has been possible through command line until now. This commit introduces two new UIs:
- When creating a channel, it will be available once the category has been selected
- On the settings page of a channel for admins
Followup to c6b43ce68b
We can just use the rich excerpt everywhere since we know
we don't need text_entities -- that introduced security issues
just to fix a spec.
Introduced in cec68b3e2c,
this is flaky because if you click the back button before
the route is fully transitioned to the loaded thread,
we end up going to the history _before_ the thread list,
which ends up being the channel.
We need to make sure that everything is loaded for the
thread first, meaning the skeleton is not there.
Also exclude some noise from the capybara logs (image load failures)
* FEATURE: Sort thread list by unread threads first
This commit changes the thread list to show the threads that
have unread messages at the top of the list sorted by the
last reply date + time, then all other threads sorted by
last reply date + time.
This also fixes some issues by removing the last_reply
relationship on the thread, which did not work for complex
querying scenarios because its order would be discarded.
* FIX: Various fixes for thread list loading
* Use the channel.threadsManager and find the channel first rather
than use activeChannel in the threads manager, otherwise we may
be looking at differenct channels.
* Look at threadsManager directly instead of storing result for threads
list otherwise it can get out of sync because of replace: true in
other places we are loading threads into the store.
* Fix sorting for thread.last_reply, needed a resort.
When clicking back from a thread, we want to either go back to the
channel if the thread was opened from an indicator, or to the thread
list if we opened it from there. Since ember doesn't give a nice way
to get the previous route, we need to store this ourselves. We only
do this on mobile, on desktop we just follow existing behaviour.
Also implements a chat router history.
---------
Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
Updates the interface for implementing summarization strategies and adds a cache layer to summarize topics once.
The cache stores the final summary and each chunk used to build it, which will be useful when we have to extend or rebuild it.
This commit adds an aria-label attribute to cooked hashtags using
the post/chat message decorateCooked functionality. I have just used
the inner content of the hashtag (the tag/category/channel name) for
the label -- we can reexamine at some point if we want something
different like "Link to dev category" or something, but from what I
can tell things like Twitter don't even have aria-labels for hashtags
so the text would be read out directly.
This commit also refactors any ruby specs checking the HTML of hashtags
to use rspec-html-matchers which is far clearer than having to maintain
the HTML structure in a HEREDOC for comparison, and gives better spec
failures.
c.f. https://meta.discourse.org/t/hashtags-are-getting-a-makeover/248866/23?u=martin
- Moves `<ChatMessageInfo />` to `<Chat::Message::Info />`
- Moves `<ChatMessageAvatar />` to `<Chat::Message::Avatar />`
- Moves `<ChatMessageLeftGutter />` to `<Chat::Message::LeftGutter />`, adds tests
- Creates `<Chat::Message::Error />`
- Creates `<Chat::Message::MentionWarning />`, adds tests and a styleguide
- Creates a model for ChatMessageMentionWarning, adds fabricator for it
- Keeps the enter/leave viewport logic inside the `<ChatMessage />` component instead of bubbling it to the channel and thread components
- Adds a scale animation when clicking a reaction
- Creates `chat/later-fn` modifier which accepts a function and a delay. It allows to call a function Xms after a component has been inserted, it's useful for animations.
- Moves css code out of chat-message into relevant files
- Deletes unused code
<!-- 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. -->
- FIX: improves reactions and thread indicator touch event on mobile
These "buttons" are located inside a scroll list which makes them very specific. The general idea is to ensure these events are passive and are not bubbling to the parent.
- DEV: moves state on top level message node
- FIX: ensures popover arrow has the correct border
- FIX: makes a message expanded by default
- FIX applies the same ios scroll fix on thread and channel
- UI: better active/hover state for thread indicator
- UI: attempts to follow more closely our BEM naming scheme
- FIX: reduces bottom padding on message with thread indicator and user info hidden
- UI: add padding for first message in thread
- FIX: prevents actions backdrop to open thread
- UI: makes thread indicator resizable
This commit adds a tracking dropdown to each individual thread, similar to topics,
that allows the user to change the notification level for a thread manually. Previously
the user had to reply to a thread to track it and see unread indicators.
Since the user can now manually track threads, the thread index has also been changed
to only show threads that the user is a member of, rather than threads that they had sent
messages in.
Unread indicators also respect the notification level -- Normal level thread tracking
will not show unread indicators in the UI when new messages are sent in the thread.
The events leading to this mistake are unclear but we decided few months ago to make direct messages NOT flaggable and even wrote a spec for this, when we actually support flagging of direct messages.
This commit ensures it will show for direct messages channels and inverses the existing spec.
This commit fixes the selection of message in threads and also applies various refactorings
- improves specs and especially page objects/components
- makes the channel/thread panes responsible of the state
- adds an animationend modifier
- continues to follow the logic of "state" should be displayed as data attributes on component by having a new `data-selected` attribute on chat messages
This commit adds the initial part of thread indicator improvements:
* Show the reply count, last reply date and excerpt,
and the participants of the thread's avatars and
count of additional participants
* Add a participants component for the thread that
can be reused for the list
* Add a query class to get the thread participants
* Live update the thread indicator more consistently
with the last reply and participant details
image image
In subsequent PRs we will cache the participants since
they do not change often, and improve the thread list
further with participants.
This commit also adds a showPresence boolean (default
true) to ChatUserAvatar, since we don't want to show the
online indicator for thread participants.
---------
Co-authored-by: chapoi <charlie@discourse.org>
* FEATURE: Content custom summarization strategies.
This PR establishes a pattern for plugins to register alternative ways of summarizing content by extending a class that defines an interface.
Core controls which strategy we'll use and who has access to it through the `summarization_strategy` and `custom_summarization_allowed_groups`. It also defines the UI for summarizing topics.
Other plugins can access this summarization mechanism and implement their features, removing cross-plugin customizations, as it currently happens between chat and the discourse-ai plugin.
* Group membership validation and rate limiting
* Work with objects instead of classes
* Port summarization feature from discourse-ai to chat
* Rename available summaries to 'Top Replies' and 'Summary'
* move the chat unread indicator to top to match the profile avatar indicator
* add white border to profile avatar indicator (badge notification) to match chat indicator and userstatus styling
* change `.urgent` to BEM
* congregate all styling into mixin
* update chat index to use mixin
* update thread indicator to use mixin
* update header indicator to use mixin
---------
Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
Co-authored-by: Martin Brennan <martin@discourse.org>
Currently navigating a long topic and then opening chat would cause the view to be scrolled to the bottom. Using `scrollTop` here ensures we correctly scroll to top.
This had been incorrectly moved into `deactivate` during another change.
* FIX: increases resize observer throttle delay
25ms is not necessary and was sometimes causing jankyness.
* FIX: removes ios momentum fix delay
Instead of a 50ms, simply use next+schedule("afterRender") to attempt to have the shortest delay possible.
* FIX: backdrop event propagation
Prevents backdrop touch to propagate to underlying channel/thread.
* UX: adds is-active class to container of active message
This change allows to keep the background on the active message while the actions menu is displayed.
* FIX: prevents skip-link to be selected on press
* UX: allows to close actions menu instantly
The backdrop should always receive events, we don't need to wait for the menu to be fully displayed.
* UI: adds spacing between last message and composer
* UI: makes backdrop less dark
* FIX: makes events passive on long-press modifier
This commit attempts to have a bullet proof solution to the following case:
- long press on message (finger is still pressed)
- menu appears
- a button is now at finger location
- user releases finger
- a click is triggered on the button
Classic event canceling solution won't work here for performance reasons as we need the event to be passive in a scroll list.
This commit contains multiple changes to improve the composer behavior especially in the context of a thread:
- Generally rename anything of the form `chatChannelThread...` to `chatThread...``
- Moves the textarea interactor instance inside the composer server
- Improves the focus state and closing of panel related to the use of the Escape shortcut
- Creates `Chat::ThreadList` as a component instead of having `Chat::Thread::ListItem` and others which could imply they were children of a the `Chat::Thread` component
One user can create a post or chat message with a hashtag they
have permission to use, but then when other users look at that
post they will see an empty space next to the hashtag because they
do not have the permission to load the colors in CSS classes for
the related category.
This fixes the issue by adding a default color with a special
CSS class if the user doesn't have permission to see the linked
channel/category on the hashtag.
What is the problem?
We were calling out to methods that calls `has_css?` or `has_selector?`
which returns a boolean. Since we are not using the return value, it
means the methods can be deemed unnecessary. However, we do want those
checks and this commit adds the necessarily assertions to make use of
the return values.
This reverts commit ddf4ecba04.
Causing a flaky test to appear:
```
main $ LOAD_PLUGINS=1 rspec plugins/chat/spec/system/chat/composer/shortcuts/channel_spec.rb
Randomized with seed 17765
.....F..
Failures:
1) Chat | composer | shortcuts | channel when using ArrowUp when last message is staged does not edit a message
Failure/Error: channel_page.send_message
expected `#<PageObjects::Components::Chat::Messages:0x00007fe823ac1710 @context=".chat-channel">.has_message?({:persisted=>true, :text=>"2"})` to be truthy, got false
[Screenshot Image]: /home/tgxworld/work/discourse/tmp/capybara/failures_r_spec_example_groups_chat_composer_shortcuts_channel_when_using_arrow_up_when_last_message_is_staged_does_not_edit_a_message_148.png
```
What is the problem?
We were calling out to methods that calls `has_css?` or `has_selector?`
which returns a boolean. Since we are not using the return value, it
means the methods can be deemed unnecessary. However, we do want those
checks and this commit adds the necessarily assertions to make use of
the return values.
If we're asserting that something is missing, we want to use
`has_no_css?` instead of `!has_css?` since `has_css?` will wait the full
capybara default wait time before return if the selector is not present.
- Made the emoji btn blue when composer is focused
- Moved everything chat-composer-button to its own file and BEM-ified it and making the choice to only work with our own is-disabled definition instead of with the attribute :disabled, for consistency
This should fix this failure:
```
Failures:
1) Thread list in side panel | full page when there are no threads that the user is participating in shows a message
Failure/Error: measurement = Benchmark.measure { example.run }
expected to find text "You are not participating in any threads in this channel." in "Community\nEverything\nMy Posts\nMore\nMessages\nInbox\nChannels\nRandom 25\nPersonal chat\nRandom 25\nShowing all messages\nOngoing discussions"
```
The screenshot failure was clearly showing the spinner still being present.
Rescuing them still makes timing-out tests fail but doesn't break `after` spec cleanup (which could trigger more errors) Using custom error class to avoid any other possible timeout-catching code.
Also:
* remove an unnecessary `.select { |x| x.size > 0 }`
* fix a typo in a test title
We were calling reset without the proper params which was causing errors in the console. This commit does the following changes:
- ensures `composer.cancel()` is the only way to cancel editing/reply
- adds a `draftSaved` property to chat message to allow for better tests
- writes a spec to ensure the flow is correct
- adds more page objects for better tests
- homogenize the default state of objects on chat message
Co-authored-by: Martin Brennan <martin@discourse.org>
Editing a message to an empty string and sending it, will delete it.
This commit also refactors a lot of channel/thread composer shortcuts specs.
---
This commit also includes various spec fixes which have been flakey while finishing this pull request.
This changes the thread header positioning of the
unread indicator to match the designs based on the route:
1. When the channel is open, show the indicator of # unread
threads with the icon
2. When the threads list is open, show no indicator since
you are on the list and can see which threads are unread
3. When a single thread is open, show the unread threads
indicator along with a left < back button, with a label
to show that this goes back to ongoing discussions
Drawer changes to come in another PR.
Why is this change required?
In the `PageObjects::Components::Chat::Messages#has_no_message?` method,
it ended up calling `has_selector` when trying to assert that the
selector is not present. This is an anti-pattern which results in us
waiting the full Capybara default wait time
What is this change required?
In the `chat/spec/system/transcript_spec.rb` test, there is a helper
method that uses `page.has_css?` in a conditional but it do not
specify a wait time and hence the default Capybara default max wait
time is used. However, there is no need for us to be waiting here so
we specify the `wait: 0` option.
This commit introduces a couple of changes:
1. When editing a chat channel's slug, we were using `this.model.set("title", title)` when the `set`
function does not exist. This was actually throwing the error in the
"can edit slug" system test where the modal was not closed after
saving and was flashing an error.
2. Introduce `PageObjects::Pages::ChatChannelAbout` and
`PageObjects::Modals::ChatChannelEdit` page object to encapsulate
logic better.
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
* 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
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 flakey has been very visible by the new headless chrome. The problem was that after moving a message we automatically redirect to the channel where the message has been moved to. However, we were not explicitly waiting for this transition and a result it could happen that we attempt to check the presence of the message on the channel page before the redirect actually happened.
The various naming changes are due to an early mistake we made in chat specs to use `chat` as the variable name for the page object which prevents to use the automatic path `chat.channel_path(...)`.
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
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 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`.
