* UX: update styling for related/suggested
This PR addresses state issues for icons of the Related & Suggested buttons, as well as well as fixes alignment issues for folding phones / tablets, wider mobile devices by moving styling to the desktop scss file; also replaces border with box-shadow.
This adds support for oneboxing WEBP and AVIF images in posts and fixing
oneboxing fixes download remote images for those formats too.
Reported in https://meta.discourse.org/t/-/276433?u=falco
It is hard to catch and debug potential bugs related to live updates of
user status (though, we haven't seen many such bugs so far). We have
this `console.warn` statement that should help us to catch one class of
such bugs:
70f1cc5552/app/assets/javascripts/discourse/app/models/user.js (L1384-L1389)
But in tests, we quite often operate with stubs of users that don't have ID,
and this warning create unnecessary noise. This PR disable the warning in
the testing environment.
It will now replies count and participants list. Also the title will be OM excerpt or user defined title, no more default "Thread" title. Lastly, the author of the last reply is also shown as prefix of it.
This could happen after you had already change the separation mode and would cause unexpected bugs.
This PR also adds more tests around using switch buttons with chat.
Reverts e2705df and re-lands #23187 and #23219.
The issue was incorrect order of execution of Rails' `assets:precompile` task in our own precompilation stack.
Co-authored-by: David Taylor <david@taylorhq.com>
* UX: update styling for related/suggested
This PR fixes styling for previous related/suggested changes' positioning being off for topics and updates the active icon color by removing a line that changed its active color.
Short answer -- the problem is the video thumbnail generator & uploader
code added a couple of months back in f144c64e13.
It was implemented as another Mixin which overrides `this._uppyInstance`
when uploading the video thumbnail after the initial upload is complete,
which means the composer's `this._uppyInstance` value is overridden,
and it loses all of its preprocessors & upload code.
This is generally a problem with the Mixin based architecture that I
used for the Uppy code, which we need to remove at some point and
refacotr.
The most ideal thing to do here would be to convert this video thumbnail
code into an Uppy
[postprocessor](https://uppy.io/docs/uppy/#addpostprocessorfn) plugin,
which runs on each upload after they are complete. I started looking
into this, and the main hurdle here is adding support to tracking the
progress of postprocessors to
[ExtendableUploader](cf42466dea/app/assets/javascripts/discourse/app/mixins/extendable-uploader.js)
so that is out of scope at this time.
The fix here makes it so the ComposerVideoThumbnailUppy code is no
longer a Mixin, but acts more like a normal class, a pattern which
we have used in chat. I also clean up a lot of the thumbnail uploader
code and remove some unnecessary things.
Attempted to add a system spec, but video streaming does not work
in Chrome for Testing at this time, and it is needed for the
onloadedmetadata event.
This PR updates the styling for the related & suggested topics to distract less from the Reply buttons and clearly indicate its usage as tabs, also referred to as pills.
`Window`'s `resize` event was unreliable. You could shrink down the browser window so that the timeline would disappear but the progress element would not render to replace it.
This commit makes it rely on a media query listener instead so it 1. matches the css 2. fires only when that query result changes (perf win)
By default, the workbox-expiration plugin will not expire cache entries which include a `Vary` header in the response. This means that cached entries can build up until the browser storage quota is hit.
This commit introduces the `ignoreVary: true` option, so that deletion is performed correctly. This will only apply going forward, so this commit also bumps the cache version and deletes the old caches.
Ref https://github.com/GoogleChrome/workbox/issues/2206
This fixes the problem reported in
https://meta.discourse.org/t/custom-status-message-in-front-of-by-header-on-scroll/273320.
This problem can be reproduced with any tooltip created using the DTooltip
component or the createDTooltip function.
The problem happens because the trigger for tooltip on mobile is click, and for tooltip
to disappear the user has to click outside the tooltip. This is the default behavior
of tippy.js – the library we use under the hood.
Note that this PR fixes the problem in topics, but not in chat. I'm going to investigate and
address it in chat in a following PR.
To fix it for tooltips created with the createDTooltip function, I had to make a refactoring.
We had a somewhat not ideal solution there, we were leaking an implementation detail
by passing tippy instances to calling sides, so they could then destroy them. With this fix,
I would have to make it more complex, because now we need to also remove onScrool
handlers, and I would need to leak this implementation detail too. So, I firstly refactored
the current solution in 5a4af05 and then added onScroll handlers in fb4aabe.
When refactoring this, I was running locally some temporarily skipped flaky tests. Turned out
they got a bit outdated, so I fixed them. Note that I'm not unskipping them in this commit,
we'll address them separately later.
Prior to this fix we would test by visiting the tab which could create a false positive, as the tab could not be present but we could still access the tab, the implementation and tests have been changed to correctly ensure this.
This commit adds some system specs to test uploads with
direct to S3 single and multipart uploads via uppy. This
is done with minio as a local S3 replacement. We are doing
this to catch regressions when uppy dependencies need to
be upgraded or we change uppy upload code, since before
this there was no way to know outside manual testing whether
these changes would cause regressions.
Minio's server lifecycle and the installed binaries are managed
by the https://github.com/discourse/minio_runner gem, though the
binaries are already installed on the discourse_test image we run
GitHub CI from.
These tests will only run in CI unless you specifically use the
CI=1 or RUN_S3_SYSTEM_SPECS=1 env vars.
For a history of experimentation here see https://github.com/discourse/discourse/pull/22381
Related PRs:
* https://github.com/discourse/minio_runner/pull/1
* https://github.com/discourse/minio_runner/pull/2
* https://github.com/discourse/minio_runner/pull/3
This is also fixes the issue of chat composer warnings persisting across channels. Currently if you try to mention more groups than is allowed for example, a mention warning pops up. When you change channels the mention warning will not disappear even if there is no text in the composer.
This adds a reset function to the chat-composer-warnings-tracker.js, which is called when the channel is changed and the message is empty. In the event that the message is not empty we call captureMentions to check the loaded drafts' mentions.
This PR would be nicer if the post-send notice used the new chat notices API to publish the mention warnings but we would have to change the existing ones and I thought that would be too much change for this PR. It'd be a good followup though.
This PR changes how we track which lists are available for a topic and how we decide which is the active one. The new approach centralizes everything in the service, and exposes functions for adding/removing a list, which each calls via `did-insert/will-destroy` modifiers.
It makes it much easier to track and update state when navigated to another topic or PM, ensuring things get updated correctly.
This partially reverts 2ecc829.
The problem is that if we don't transform mentions right away,
there is a noticeable lag before a mention gets fully rendered,
while with the transformation, everything is super smooth.
I'm reverting that change only for mentions. Another part was about
category hashtags, but unfortunately they lag both with and without
the transformation. We need to address them separately.
71ff3417 removed the mobile-specific template for discovery/topics. It was noted that this would introduce an additional `<div class='show-more'>` wrapper around the new topic indicator on mobile, but I didn't realise that it would cause positioning problems on mobile.
This commit moves the desktop-specific CSS into the desktop SCSS file so that it does not apply to mobile.
Separate mobile templates are a pattern we're moving away from. They are not supported by Ember, and make things more difficult to develop/test. The differences between the mobile and desktop templates for `discovery/topics` are very minimal, so they can be easily integrated.
The only feature missing from the main template was the new 'list header controls' UI. This commit introduces that to the main template inside an `mobileView` conditional.
Key changes in behavior, many of which could be considered bug fixes, are:
- Mobile will now include 'redirected reason'
- Mobile will now include shared drafts
- Mobile will now include before-topic-list and after-topic-list Plugin Outlets
- Mobile will now have a `<div class="show-more">` wrapper around the 'new or updated' UI, to match desktop. This does not seem to cause any visual change.
Mobile-specific template overrides of `discovery/topics` will continue to function as before - this should not be a breaking change for any themes/plugins.
Mobile-specific templates for the topic list and topic-list-item remain in place.
In the past we were only intercepting 429 and 404; it's probably better to surface any error.
There are already tests for the 404 and 429, I consider them enough for now.