Previously we were using the `didInsertElement` hook and querying the DOM to check whether the other button was visible. This is problematic from a performance point of view because it forces the browser to render the layout prematurely. It can also lead to subtle bugs based on the current scroll position.
In addition, having this logic on a `didInsertElement` hook makes it totally incompatible with the new 'loading slider' feature (because the component is not re-rendered between different topic lists).
This commit updates the logic to be based simply on the count of topics in the list. If there are fewer than 5 topics, the top button is hidden.
Under certain conditions, this `afterRender` hook can be triggered after the topic-list-item has been removed from the DOM. This is more likely when the 'loading slider' strategy is used on a site.
Since we create threads in the background regardless of whether
threading is enabled for a channel, we get the unexpected behaviour
of everyone having a lot of unread threads when threading is enabled
for the channel.
To counteract this, when the admin enables threads for a channel
we can just run a high priority background job to mark all threads
as read in the channel for all users, so they are essentially
starting from a clean slate.
1) Edit Category when editing a category with form templates set should have form templates enabled and showing the selected templates
Failure/Error: expect(category_page).to have_selected_template(selected_templates)
expected `#<PageObjects::Pages::Category:0x00007fdb278fbd30>.has_selected_template?("template_0,template_1")` to be truthy, got false
Wait for CSS rather than trying to compare attr directly
and also make sure the ids are always in order.
Followup to 802fb3b194
We should not hide the replies count if there is only 1 participant
for a thread, because this makes it look like the last reply is the
only reply.
This introduces a PLATFORM_KEY_MODIFIER const that
can be used both client and server side, to determine
whether we should be using the Meta or Ctrl key based
on whether the user is on Windows/Linux or Mac.
Why this change?
Before this commit, there is a chance that we will transition the user
to a different route if the chat thread component has been destroyed
prior to the request for fetching messasges in a chat thread returning.
This commit makes it such that we simply ignore the request if the chat
thread component has been destroyed.
We believe this is the cause of the flaky system tests in plugins/chat/spec/system/navigation_spec.rb
which we've been seeing on CI.
Why this change?
`Faker::Lorem.paragraph` generates a differrent length of string
every time. When a string happens to be long, it can change the UI
across system test runs making it harder to reason about our system
tests across multiple runs since the state is never really consistent.
We will just generate a paragraph with a fixed length going forward so
that the UI remains consistent. This should make certain tests which
relies on the UI being in a certain state to become less flaky.
Why this change?
This is abit of a trial and error but we're starting to see selenium
session not created errors on CI. One of the reason for this is that the
system has run out of resources to create a new tab.
This commit reduces the number of parallel test processors in an attempt
to increase the amount of resources available to each test process and
hopefully lead to more stable CI system tests.
Why this change?
A new component based API for modals was introduced in
b3a23bd9d6. This commit moves the edit
sidebar section modal to the new API.
Reviewer notes
No functionality or visual change is introduced in this PR.
In previous PR https://github.com/discourse/discourse/pull/22340 bug was introduced. Notifications were blocked when, even if topic was watched directly. New query is taking TopicUser into consideration.
In addition, in user interface, when `watched_precedence_over_muted` is not set, then value from SiteSetting should be displayed.
Why this change?
This change ensures that we scroll to the top of the message when
hovering over a message to ensure that the message actions container
that appears on hover is not hidden in the chat drawer when the content
of the chat message is long.
This reverts commit 865f7a9852.
The flakiness that we have been seeing and fixing on CI were not related
to system resource problems. Therefore, we can bump this up back to 5.
This commit includes several fixes and improvements to thread
original message handling:
1. When a thread's original message is deleted, the thread no longer
counts as unread for a user
2. When a thread original message is deleted and the user is looking
at the thread list, it will be removed from the list
3. When a thread original message is restored and the user is looking
at the thread list, it will be added back to the list if it was
previously loaded
In specific conditions (generally a small drawer, with a long message) it is possible to have the message’s actions menu to be displayed hover the drawer's header.
This is particularly hard to fix correctly using popper due to our positioning which is slightly at the limit of the container.
The proposed fix targets mostly the specs by ensuring the messages actions will be hidden before attempting to click any header's button.
Why this change?
In CI, we know we're clicking a link to a chat channel's threads list.
However, the threads list is not loaded and we want to add more
assertions here to try and figure out why. By asserting for the current
URL, we will at least know that the transition to the URL is successful.
- Presence needs to be explicitly set on the component now
- We were not checking and testing correctly the presence of the unread indicator in the menu
This commit replaces two existing screens:
- draft
- channel selection modal
Main features compared to existing solutions
- features are now combined, meaning you can for example create multi users DM
- it will show users with chat disabled
- it shows unread state
- hopefully a better look/feel
- lots of small details and fixes...
Other noticeable fixes
- starting a DM with a user, even from the user card and clicking <kbd>Chat</kbd> will not show a green dot for the target user (or even the channel) until a message is actually sent
- it should almost never do a full page reload anymore
---------
Co-authored-by: Martin Brennan <mjrbrennan@gmail.com>
Co-authored-by: Jordan Vidrine <30537603+jordanvidrine@users.noreply.github.com>
Co-authored-by: chapoi <101828855+chapoi@users.noreply.github.com>
Co-authored-by: Mark VanLandingham <markvanlan@gmail.com>
This brings the functionality from https://github.com/discourse/discourse-loading-slider into Discourse core. Default behaviour remains the same - the new slider mode can be enabled using the new 'page_loading_indicator' site setting.
A follow-up to 585a2e4e. A couple of tests with the new rich tooltip were flaky.
We suppose the reason is some problem related to widgets lifecycle. This PR
doesn't fix the issue, but isolates testing of the tooltip related logic related
inside its own test, which should make it not flaky.
This is a temporal solution, we're going to move all these code to using
glimmer components.
Without this fix, the following error is raised:
```
ActiveRecord::StatementInvalid:
PG::SyntaxError: ERROR: syntax error at or near ")"
LINE 4: WHERE thread_id IN ()
```
This is not a valid route and is causing routing errors to be raised in
the test env adding noise to the logs. We'll just "handle" the route in
the test env.
Why this change?
The process's pid is useful when we're trying to link output from
different processes together. In this case, we want to be able to link
the Rails server logs to the right rspec process.
Before:
[2] Viewing sidebar mobile collapses the sidebar when clicking outside of it
After:
[2] (#176342) Viewing sidebar mobile collapses the sidebar when clicking outside of it
Why this change?
Chat system tests that opens the message actions on mobile have been
flaky on our CI. Those system test usually fails when the message
actions do not show up as expected causing subsequent actions to fail.
In the case of the `Reply to message - channel - mobile when the message has an existing thread replies to the existing thread`
system test, failure screenshot shows that we ended up navigating to the
thread instead of opening the message actions button. To understand why
this happens, we first need to understand that by default Capybara clicks
on the centre of an element. Also, we need to note that the HTML structure of
a chat message is like so:
```
<div class="chat-message-container">
<div class="chat-message">
<div class="chat-message-avatar" />
<div class="chat-message-content" />
<div class="chat-message-thread-indicator" />
</div>
</div>
```
Since `PageObjects::Pages::ChatChannel#expand_message_actions_mobile`
attempts to click on the `.chat-message-contaier`, there is a
possibility that the center of that element is the
`.chat-message-thread-indicator` element which would explain why we
navigated to the thread list instead of opening up the message actions.
This is possible because the content of the original chat message as
well as the message excerpt in the thread is randomly generated where the
length of the message and how the text wraps on mobile can affect the
height of the `.chat-message-content` element as thus its position in
the `.chat-message-container` element. In most cases, the middle of the
`.chat-message-container` happens to be the `.chat-message-content`
which is why this test "flakes" sometimes.
What is the solution?
Instead of clicking on the `.chat-message-container`, we be more
specific and click on the `.chat-message-content` element instead.
Why is this change required?
We've been seeing flaky tests due to server errors on CI but are unable
to debug it because we do not log any of the errors. This change gives
us a fighting chance the next time we encounter a server error during
system test runs.
See
https://github.com/discourse/discourse/actions/runs/5459248864/jobs/9935049920?pr=22424
for an example of server errors encountered during system tests.
Previously , the test was flaky and failing with a selenium stale
element error because we were retrieving the tag nodes with `all` and
then calling `.map(&:text)` on it. However, there is a chance that a
re-render happens and those nodes will end up being stale resulting in
the selenium error.
David Taylor
Joffrey JAFFEUX
Mark VanLandingham
Loïc Guitaut
Martin Brennan
Alan Guo Xiang Tan
Krzysztof Kotlarek
dependabot[bot]
Sérgio Saquetim
Kris
Jarek Radosz
Isaac Janzen
Andrei Prigorshnev
Keegan George