From ea1473e53260e8a458f8118b89e7b07b5cc50507 Mon Sep 17 00:00:00 2001 From: Keegan George Date: Tue, 22 Oct 2024 03:15:24 +0900 Subject: [PATCH] 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/components/composer-editor.js | 19 +--- .../javascripts/discourse/app/lib/uploads.js | 14 +++ .../discourse/app/lib/uppy/composer-upload.js | 70 +++++++++++++++ config/locales/server.en.yml | 1 + config/site_settings.yml | 3 + spec/system/composer_uploads_spec.rb | 87 +++++++++++++++++++ 6 files changed, 179 insertions(+), 15 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/composer-editor.js b/app/assets/javascripts/discourse/app/components/composer-editor.js index 5aaf5b55ac4..277dfbbefc4 100644 --- a/app/assets/javascripts/discourse/app/components/composer-editor.js +++ b/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({ diff --git a/app/assets/javascripts/discourse/app/lib/uploads.js b/app/assets/javascripts/discourse/app/lib/uploads.js index 6e7efed8f2a..7b4a9b4ee92 100644 --- a/app/assets/javascripts/discourse/app/lib/uploads.js +++ b/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) { diff --git a/app/assets/javascripts/discourse/app/lib/uppy/composer-upload.js b/app/assets/javascripts/discourse/app/lib/uppy/composer-upload.js index 3d984a66ff0..af2e78a4c33 100644 --- a/app/assets/javascripts/discourse/app/lib/uppy/composer-upload.js +++ b/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; + } } diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 2347a75bb1c..561be644488 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -2716,6 +2716,7 @@ en: experimental_form_templates: "EXPERIMENTAL: Enable the form templates feature. After enabled, manage the templates at Customize / Templates." 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" diff --git a/config/site_settings.yml b/config/site_settings.yml index 9bf7445419a..6b22d80a46c 100644 --- a/config/site_settings.yml +++ b/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: diff --git a/spec/system/composer_uploads_spec.rb b/spec/system/composer_uploads_spec.rb index 0a5b4e64f1a..c3d19d7a099 100644 --- a/spec/system/composer_uploads_spec.rb +++ b/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