FEATURE: Automatically grid images when 3 or more image uploads (#29260)

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.

app/assets/javascripts/discourse/app/components/composer-editor.js

@@ -19,7 +19,10 @@ import {
 } from "discourse/lib/link-mentions";
 import { loadOneboxes } from "discourse/lib/load-oneboxes";
 import putCursorAtEnd from "discourse/lib/put-cursor-at-end";
-import { authorizesOneOrMoreImageExtensions } from "discourse/lib/uploads";
+import {
+  authorizesOneOrMoreImageExtensions,
+  IMAGE_MARKDOWN_REGEX,
+} from "discourse/lib/uploads";
 import UppyComposerUpload from "discourse/lib/uppy/composer-upload";
 import userSearch from "discourse/lib/user-search";
 import {
@@ -44,20 +47,6 @@ import discourseComputed, {
 } from "discourse-common/utils/decorators";
 import I18n from "discourse-i18n";
 
-// original string `![image|foo=bar|690x220, 50%|bar=baz](upload://1TjaobgKObzpU7xRMw2HuUc87vO.png "image title")`
-// group 1 `image|foo=bar`
-// group 2 `690x220`
-// group 3 `, 50%`
-// group 4 '|bar=baz'
-// group 5 'upload://1TjaobgKObzpU7xRMw2HuUc87vO.png "image title"'
-
-// Notes:
-// Group 3 is optional. group 4 can match images with or without a markdown title.
-// All matches are whitespace tolerant as long it's still valid markdown.
-// If the image is inside a code block, we'll ignore it `(?!(.*`))`.
-const IMAGE_MARKDOWN_REGEX =
-  /!\[(.*?)\|(\d{1,4}x\d{1,4})(,\s*\d{1,3}%)?(.*?)\]\((upload:\/\/.*?)\)(?!(.*`))/g;
-
 let uploadHandlers = [];
 export function addComposerUploadHandler(extensions, method) {
   uploadHandlers.push({

app/assets/javascripts/discourse/app/lib/uploads.js

@@ -10,6 +10,20 @@ function isGUID(value) {
   );
 }
 
+// original string `![image|foo=bar|690x220, 50%|bar=baz](upload://1TjaobgKObzpU7xRMw2HuUc87vO.png "image title")`
+// group 1 `image|foo=bar`
+// group 2 `690x220`
+// group 3 `, 50%`
+// group 4 '|bar=baz'
+// group 5 'upload://1TjaobgKObzpU7xRMw2HuUc87vO.png "image title"'
+
+// Notes:
+// Group 3 is optional. group 4 can match images with or without a markdown title.
+// All matches are whitespace tolerant as long it's still valid markdown.
+// If the image is inside a code block, we'll ignore it `(?!(.*`))`.
+export const IMAGE_MARKDOWN_REGEX =
+  /!\[(.*?)\|(\d{1,4}x\d{1,4})(,\s*\d{1,3}%)?(.*?)\]\((upload:\/\/.*?)\)(?!(.*`))/g;
+
 // This wrapper simplifies unit testing the dialog service
 export const dialog = {
   alert(msg) {

app/assets/javascripts/discourse/app/lib/uppy/composer-upload.js

@@ -13,6 +13,8 @@ import {
   displayErrorForBulkUpload,
   displayErrorForUpload,
   getUploadMarkdown,
+  IMAGE_MARKDOWN_REGEX,
+  isImage,
   validateUploadedFile,
 } from "discourse/lib/uploads";
 import UppyS3Multipart from "discourse/lib/uppy/s3-multipart";
@@ -55,6 +57,7 @@ export default class UppyComposerUpload {
   #inProgressUploads = [];
   #bufferedUploadErrors = [];
   #placeholders = {};
+  #consecutiveImages = [];
 
   #useUploadPlaceholders = true;
   #uploadTargetBound = false;
@@ -331,6 +334,7 @@ export default class UppyComposerUpload {
               extension: file.extension,
             })
           );
+
           const placeholder = this.#uploadPlaceholder(file);
           this.#placeholders[file.id] = {
             uploadPlaceholder: placeholder,
@@ -363,6 +367,11 @@ export default class UppyComposerUpload {
           getUploadMarkdown(upload)
         );
 
+        // Track consecutive images for surrounding with [grid] later:
+        if (isImage(upload.url)) {
+          this.#consecutiveImages.push(markdown);
+        }
+
         cacheShortUploadUrl(upload.short_url, upload);
 
         new ComposerVideoThumbnailUppy(getOwner(this)).generateVideoThumbnail(
@@ -376,6 +385,7 @@ export default class UppyComposerUpload {
                 markdown
               );
             }
+
             this.#resetUpload(file, { removePlaceholder: false });
             this.appEvents.trigger(
               `${this.composerEventPrefix}:upload-success`,
@@ -387,6 +397,14 @@ export default class UppyComposerUpload {
               this.appEvents.trigger(
                 `${this.composerEventPrefix}:all-uploads-complete`
               );
+
+              const MIN_IMAGES_TO_AUTO_GRID = 3;
+              if (
+                this.siteSettings.experimental_auto_grid_images &&
+                this.#consecutiveImages?.length >= MIN_IMAGES_TO_AUTO_GRID
+              ) {
+                this.#autoGridImages();
+              }
               this.#displayBufferedErrors();
               this.#reset();
             }
@@ -605,6 +623,7 @@ export default class UppyComposerUpload {
     });
     this.#inProgressUploads = [];
     this.#bufferedUploadErrors = [];
+    this.#consecutiveImages = [];
     this.uppyWrapper.resetPreProcessors();
     this.#fileInputEl.value = "";
   }
@@ -712,4 +731,55 @@ export default class UppyComposerUpload {
       selectionStart === 0 || textArea.value.charAt(selectionStart - 1) === "\n"
     );
   }
+
+  #autoGridImages() {
+    const reply = this.composerModel.get("reply");
+    const imagesToWrapGrid = new Set(this.#consecutiveImages);
+    const matches = reply.match(IMAGE_MARKDOWN_REGEX) || [];
+
+    const foundImages = [];
+    matches.forEach((fullImageMarkdown) => {
+      fullImageMarkdown = fullImageMarkdown.trim();
+
+      // Check if the matched image markdown is in the imagesToWrapGrid
+      if (imagesToWrapGrid.has(fullImageMarkdown)) {
+        foundImages.push(fullImageMarkdown);
+        imagesToWrapGrid.delete(fullImageMarkdown);
+
+        // Check if we've found all the images
+        if (imagesToWrapGrid.size === 0) {
+          return;
+        }
+      }
+    });
+
+    // Check if all consecutive images have been found
+    if (foundImages.length === this.#consecutiveImages.length) {
+      const firstImageMarkdown = foundImages[0];
+      const lastImageMarkdown = foundImages[foundImages.length - 1];
+
+      const startIndex = reply.indexOf(firstImageMarkdown);
+      const endIndex =
+        reply.indexOf(lastImageMarkdown) + lastImageMarkdown.length;
+
+      if (startIndex !== -1 && endIndex !== -1) {
+        const textArea = this.#editorEl.querySelector(this.editorInputClass);
+        if (textArea) {
+          textArea.focus();
+          textArea.selectionStart = startIndex;
+          textArea.selectionEnd = endIndex;
+          this.appEvents.trigger(
+            `${this.composerEventPrefix}:apply-surround`,
+            "[grid]",
+            "[/grid]",
+            "grid_surround",
+            { useBlockMode: true }
+          );
+        }
+      }
+    }
+    // Clear found images for the next consecutive images:
+    this.#consecutiveImages.length = 0;
+    foundImages.length = 0;
+  }
 }

config/locales/server.en.yml

@@ -2716,6 +2716,7 @@ en:
     experimental_form_templates: "EXPERIMENTAL: Enable the form templates feature. <b>After enabled,</b> manage the templates at <a href='%{base_path}/admin/customize/form-templates'>Customize / Templates</a>."
     admin_sidebar_enabled_groups: "Enable sidebar navigation for the admin UI for the specified groups, which replaces the top-level admin navigation buttons."
     lazy_load_categories_groups: "EXPERIMENTAL: Lazy load category information only for users of these groups. This improves performance on sites with many categories."
+    experimental_auto_grid_images: "EXPERIMENTAL: Automatically wraps images in [grid] tags when 3 or more images are uploaded in the composer."
 
     page_loading_indicator: "Configure the loading indicator which appears during page navigations within Discourse. 'Spinner' is a full page indicator. 'Slider' shows a narrow bar at the top of the screen."
     show_user_menu_avatars: "Show user avatars in the user menu"

config/site_settings.yml

@@ -2471,6 +2471,9 @@ developer:
     default: 50
     hidden: true
     client: true
+  experimental_auto_grid_images:
+    default: false
+    client: true
 
 navigation:
   navigation_menu:

spec/system/composer_uploads_spec.rb

@@ -158,4 +158,91 @@ describe "Uploading files in the composer", type: :system do
       )
     end
   end
+
+  context "when multiple images are uploaded" do
+    before { SiteSetting.experimental_auto_grid_images = true }
+
+    it "automatically wraps images in [grid] tags on 3 or more images" do
+      visit "/new-topic"
+      expect(composer).to be_opened
+
+      file_path_1 = file_from_fixtures("logo.png", "images").path
+      file_path_2 = file_from_fixtures("logo.jpg", "images").path
+      file_path_3 = file_from_fixtures("downsized.png", "images").path
+      attach_file([file_path_1, file_path_2, file_path_3]) do
+        composer.click_toolbar_button("upload")
+      end
+
+      expect(composer).to have_no_in_progress_uploads
+      expect(composer.composer_input.value).to match(
+        %r{\[grid\].*!\[.*?\]\(upload://.*?\).*!\[.*?\]\(upload://.*?\).*!\[.*?\]\(upload://.*?\).*?\[/grid\]}m,
+      )
+    end
+
+    it "does not wrap [grid] tags on less than 3 images" do
+      visit "/new-topic"
+      expect(composer).to be_opened
+
+      file_path_1 = file_from_fixtures("logo.png", "images").path
+      file_path_2 = file_from_fixtures("logo.jpg", "images").path
+      attach_file([file_path_1, file_path_2]) { composer.click_toolbar_button("upload") }
+
+      expect(composer).to have_no_in_progress_uploads
+      expect(composer.composer_input.value).to match(
+        %r{!\[.*?\]\(upload://.*?\).*?!\[.*?\]\(upload://.*?\)}m,
+      )
+    end
+
+    it "automatically wraps images in [grid] tags even after clearing previous uploads" do
+      visit "/new-topic"
+      expect(composer).to be_opened
+
+      file_path_1 = file_from_fixtures("logo.png", "images").path
+      file_path_2 = file_from_fixtures("logo.jpg", "images").path
+      file_path_3 = file_from_fixtures("downsized.png", "images").path
+      file_path_4 = file_from_fixtures("logo-dev.png", "images").path
+      file_path_5 = file_from_fixtures("large_icon_correct.png", "images").path
+      file_path_6 = file_from_fixtures("large_icon_incorrect.png", "images").path
+
+      attach_file([file_path_1, file_path_2, file_path_3]) do
+        composer.click_toolbar_button("upload")
+      end
+
+      expect(composer).to have_no_in_progress_uploads
+
+      expect(composer.composer_input.value).to match(
+        %r{\[grid\].*!\[.*?\]\(upload://.*?\).*!\[.*?\]\(upload://.*?\).*!\[.*?\]\(upload://.*?\).*?\[/grid\]}m,
+      )
+
+      composer.clear_content
+
+      attach_file([file_path_4, file_path_5, file_path_6]) do
+        composer.click_toolbar_button("upload")
+      end
+
+      expect(composer).to have_no_in_progress_uploads
+      expect(composer.composer_input.value).to match(
+        %r{\[grid\].*!\[.*?\]\(upload://.*?\).*!\[.*?\]\(upload://.*?\).*!\[.*?\]\(upload://.*?\).*?\[/grid\]}m,
+      )
+    end
+
+    it "does not automatically wrap images in [grid] tags when setting is disabled" do
+      SiteSetting.experimental_auto_grid_images = false
+
+      visit "/new-topic"
+      expect(composer).to be_opened
+
+      file_path_1 = file_from_fixtures("logo.png", "images").path
+      file_path_2 = file_from_fixtures("logo.jpg", "images").path
+      file_path_3 = file_from_fixtures("downsized.png", "images").path
+      attach_file([file_path_1, file_path_2, file_path_3]) do
+        composer.click_toolbar_button("upload")
+      end
+
+      expect(composer).to have_no_in_progress_uploads
+      expect(composer.composer_input.value).to match(
+        %r{!\[.*?\]\(upload://.*?\).*!\[.*?\]\(upload://.*?\).*!\[.*?\]\(upload://.*?\)}m,
+      )
+    end
+  end
 end