Recently we added a new feature for automatically gridding images in the composer (https://github.com/discourse/discourse/pull/29260). After testing this feature under a setting for a short period of time, the feature is no longer experimental anymore.
This PR removes the site setting `experimental_auto_grid_images`.
This PR is a follow-up to ea1473e532. When we initially added the experimental feature for automatically adding `[grid]` to images, we add the [grid] surrounding images after all the uploads have been completed.
This can lead to confusion when `[grid]` is delayed to be added in the composer, as users may try to add grid manually leading to breakage. This also leads to issues with Discourse AI's automatic image caption feature.
**In this PR**: we simply move the logic to be added when the images are uploaded and processing. This way, `[grid]` surrounding images is added immediately. We also apply a fix for an edge-case to prevent images from being wrapped in `[grid]` when they are already inside `[grid]` tags.
This PR adds the feature where three or more image uploads in the composer will result in the images being surrounded by `[grid]` tags. This helps take advantage of the grid feature (https://github.com/discourse/discourse/pull/21513) and display images in a more appealing way immediately after upload.
If there is a codec issue or something trying to process a video file
for thumbnail generation, uploads could hang indefinitely. This fix
ensures that we continue the upload process even if we encounter an
error trying to generate a thumbnail for it.
If we don't get a `videoWidth` back for a video don't try and generate a
thumbnail for it.
Also as part of this change I switched getImageData, the function
throwing the error, to use video.videoWidth instead of canvas.width
because it's very likely we were setting canvas.width too early before
the width could be read. Now that we are reading the value inside of the
setTimeout hopefully we will actually have a width. Just incase we don't
detect a width we will now exit early instead of throwing an error.
We only need to check for `0` and not null because the value is an
integer and will always return a 0 if it can't be read. https://developer.mozilla.org/en-US/docs/Web/API/HTMLVideoElement/videoWidth
See https://meta.discourse.org/t/322363
I am changing many of these to notes or resolving them as is,
most of these I have not actively worked on in years so someone
else can work on them when we get to these areas again.
When uploading a video, the composer will now show a thumbnail image in
the composer preview instead of just the video placeholder image.
If `enable_diffhtml_preview` is enabled the video will be rendered in
the composer preview and is playable.
Using min_trust_to_create_topic and create_topic_allowed_groups together was part of #24740
Now, when plugins specs are fixed, we can safely remove that part of logic.
Tries to fix the composer upload spec by making the upload
slow enough to allow clicking the Cancel button, and improves
generally the API for CDP network changes.
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.
Keegan George
Blake Erickson
Martin Brennan
Jarek Radosz
Krzysztof Kotlarek