From 0b495e9ad421317b8d6ce5b49d6df497f012b05f Mon Sep 17 00:00:00 2001 From: Natalie Tay Date: Tue, 9 Nov 2021 14:34:09 +0800 Subject: [PATCH] FEATURE: Allow users to edit alt text from the image preview in the editor (#14480) --- .../app/components/composer-editor.js | 88 +++++++++-- .../tests/acceptance/composer-test.js | 137 +++++++++++++++++- .../{resize-controls.js => image-controls.js} | 84 +++++++---- app/assets/stylesheets/common/d-editor.scss | 49 ++++++- config/locales/client.en.yml | 3 + 5 files changed, 315 insertions(+), 46 deletions(-) rename app/assets/javascripts/pretty-text/engines/discourse-markdown/{resize-controls.js => image-controls.js} (54%) diff --git a/app/assets/javascripts/discourse/app/components/composer-editor.js b/app/assets/javascripts/discourse/app/components/composer-editor.js index 2aed00dae34..a0164cd118d 100644 --- a/app/assets/javascripts/discourse/app/components/composer-editor.js +++ b/app/assets/javascripts/discourse/app/components/composer-editor.js @@ -38,6 +38,18 @@ import { loadOneboxes } from "discourse/lib/load-oneboxes"; import putCursorAtEnd from "discourse/lib/put-cursor-at-end"; import userSearch from "discourse/lib/user-search"; +// 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; const REBUILD_SCROLL_MAP_EVENTS = ["composer:resized", "composer:typed-reply"]; let uploadHandlers = []; @@ -589,24 +601,15 @@ export default Component.extend(ComposerUpload, { }, _registerImageScaleButtonClick($preview) { - // 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 imageScaleRegex = /!\[(.*?)\|(\d{1,4}x\d{1,4})(,\s*\d{1,3}%)?(.*?)\]\((upload:\/\/.*?)\)(?!(.*`))/g; $preview.off("click", ".scale-btn").on("click", ".scale-btn", (e) => { - const index = parseInt($(e.target).parent().attr("data-image-index"), 10); + const index = parseInt( + $(e.target).closest(".button-wrapper").attr("data-image-index"), + 10 + ); const scale = e.target.attributes["data-scale"].value; const matchingPlaceholder = this.get("composer.reply").match( - imageScaleRegex + IMAGE_MARKDOWN_REGEX ); if (matchingPlaceholder) { @@ -614,7 +617,7 @@ export default Component.extend(ComposerUpload, { if (match) { const replacement = match.replace( - imageScaleRegex, + IMAGE_MARKDOWN_REGEX, `![$1|$2, ${scale}%$4]($5)` ); @@ -622,7 +625,7 @@ export default Component.extend(ComposerUpload, { "composer:replace-text", matchingPlaceholder[index], replacement, - { regex: imageScaleRegex, index } + { regex: IMAGE_MARKDOWN_REGEX, index } ); } } @@ -632,6 +635,58 @@ export default Component.extend(ComposerUpload, { }); }, + _registerImageAltTextButtonClick($preview) { + $preview + .off("click", ".alt-text-edit-btn") + .on("click", ".alt-text-edit-btn", (e) => { + const parentContainer = $(e.target).closest( + ".alt-text-readonly-container" + ); + const altText = parentContainer.find(".alt-text"); + const correspondingInput = parentContainer.find(".alt-text-input"); + + $(e.target).hide(); + altText.hide(); + correspondingInput.val(altText.text()); + correspondingInput.show(); + e.preventDefault(); + }); + + $preview + .off("keypress", ".alt-text-input") + .on("keypress", ".alt-text-input", (e) => { + if (e.key === "[" || e.key === "]") { + e.preventDefault(); + } + + if (e.key === "Enter") { + const index = parseInt( + $(e.target).closest(".button-wrapper").attr("data-image-index"), + 10 + ); + const matchingPlaceholder = this.get("composer.reply").match( + IMAGE_MARKDOWN_REGEX + ); + const match = matchingPlaceholder[index]; + const replacement = match.replace( + IMAGE_MARKDOWN_REGEX, + `![${$(e.target).val()}|$2$3$4]($5)` + ); + + this.appEvents.trigger("composer:replace-text", match, replacement); + + const parentContainer = $(e.target).closest( + ".alt-text-readonly-container" + ); + const altText = parentContainer.find(".alt-text"); + const altTextButton = parentContainer.find(".alt-text-edit-btn"); + altText.show(); + altTextButton.show(); + $(e.target).hide(); + } + }); + }, + @on("willDestroyElement") _composerClosed() { this._unbindMobileUploadButton(); @@ -807,6 +862,7 @@ export default Component.extend(ComposerUpload, { } this._registerImageScaleButtonClick($preview); + this._registerImageAltTextButtonClick($preview); this.trigger("previewRefreshed", $preview[0]); this.afterRefresh($preview); diff --git a/app/assets/javascripts/discourse/tests/acceptance/composer-test.js b/app/assets/javascripts/discourse/tests/acceptance/composer-test.js index 28831241ff9..b011c2887fe 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/composer-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/composer-test.js @@ -8,7 +8,13 @@ import { updateCurrentUser, visible, } from "discourse/tests/helpers/qunit-helpers"; -import { click, currentURL, fillIn, visit } from "@ember/test-helpers"; +import { + click, + currentURL, + fillIn, + triggerKeyEvent, + visit, +} from "@ember/test-helpers"; import { skip, test } from "qunit"; import Draft from "discourse/models/draft"; import I18n from "I18n"; @@ -968,6 +974,135 @@ acceptance("Composer", function (needs) { ); }); + test("Editing alt text for single image in preview edits alt text in composer", async function (assert) { + const altText = ".image-wrapper .button-wrapper .alt-text"; + const editAltTextButton = + ".image-wrapper .button-wrapper .alt-text-edit-btn"; + const altTextInput = ".image-wrapper .button-wrapper .alt-text-input"; + + await visit("/"); + + await click("#create-topic"); + await fillIn(".d-editor-input", `![zorro|200x200](upload://zorro.png)`); + + // placement of elements + + assert.ok( + exists(altText), + "shows alt text in the image wrapper's button wrapper" + ); + + assert.ok( + exists(editAltTextButton + " .d-icon-pencil"), + "alt text edit button with icon is in the image wrapper's button wrapper" + ); + + assert.ok( + exists(altTextInput), + "alt text input is in the image wrapper's button wrapper" + ); + + // logical + + assert.equal(query(altText).innerText, "zorro", "correct alt text"); + assert.ok(visible(altText), "alt text is visible"); + assert.ok(visible(editAltTextButton), "alt text edit button is visible"); + assert.ok(invisible(altTextInput), "alt text input is not visible"); + + await click(editAltTextButton); + + assert.ok(invisible(altText), "readonly alt text is not visible"); + assert.ok( + invisible(editAltTextButton), + "alt text edit button is not visible" + ); + assert.ok(visible(altTextInput), "alt text input is visible"); + assert.equal( + queryAll(altTextInput).val(), + "zorro", + "correct alt text in input" + ); + + await triggerKeyEvent(altTextInput, "keypress", "[".charCodeAt(0)); + await triggerKeyEvent(altTextInput, "keypress", "]".charCodeAt(0)); + assert.equal( + queryAll(altTextInput).val(), + "zorro", + "does not input [ ] keys" + ); + + await fillIn(altTextInput, "steak"); + await triggerKeyEvent(altTextInput, "keypress", 13); + + assert.equal( + queryAll(".d-editor-input").val(), + "![steak|200x200](upload://zorro.png)", + "alt text updated" + ); + assert.equal(query(altText).innerText, "steak", "shows the alt text"); + assert.ok(visible(editAltTextButton), "alt text edit button is visible"); + assert.ok(invisible(altTextInput), "alt text input is not visible"); + }); + + test("Editing alt text for one of two images in preview updates correct alt text in composer", async function (assert) { + const editAltTextButton = + ".image-wrapper .button-wrapper .alt-text-edit-btn"; + const altTextInput = ".image-wrapper .button-wrapper .alt-text-input"; + + await visit("/"); + await click("#create-topic"); + + await fillIn( + ".d-editor-input", + `![zorro|200x200](upload://zorro.png) ![not-zorro|200x200](upload://not-zorro.png)` + ); + await click(editAltTextButton); + + await fillIn(altTextInput, "tomtom"); + await triggerKeyEvent(altTextInput, "keypress", 13); + + assert.equal( + queryAll(".d-editor-input").val(), + `![tomtom|200x200](upload://zorro.png) ![not-zorro|200x200](upload://not-zorro.png)`, + "the correct image's alt text updated" + ); + }); + + test("Deleting alt text for image empties alt text in composer and allows further modification", async function (assert) { + const altText = ".image-wrapper .button-wrapper .alt-text"; + const editAltTextButton = + ".image-wrapper .button-wrapper .alt-text-edit-btn"; + const altTextInput = ".image-wrapper .button-wrapper .alt-text-input"; + + await visit("/"); + + await click("#create-topic"); + await fillIn(".d-editor-input", `![zorro|200x200](upload://zorro.png)`); + + await click(editAltTextButton); + + await fillIn(altTextInput, ""); + await triggerKeyEvent(altTextInput, "keypress", 13); + + assert.equal( + queryAll(".d-editor-input").val(), + "![|200x200](upload://zorro.png)", + "alt text updated" + ); + assert.equal(query(altText).innerText, "", "shows the alt text"); + + await click(editAltTextButton); + + await fillIn(altTextInput, "tomtom"); + await triggerKeyEvent(altTextInput, "keypress", 13); + + assert.equal( + queryAll(".d-editor-input").val(), + "![tomtom|200x200](upload://zorro.png)", + "alt text updated" + ); + }); + skip("Shows duplicate_link notice", async function (assert) { await visit("/t/internationalization-localization/280"); await click("#topic-footer-buttons .create"); diff --git a/app/assets/javascripts/pretty-text/engines/discourse-markdown/resize-controls.js b/app/assets/javascripts/pretty-text/engines/discourse-markdown/image-controls.js similarity index 54% rename from app/assets/javascripts/pretty-text/engines/discourse-markdown/resize-controls.js rename to app/assets/javascripts/pretty-text/engines/discourse-markdown/image-controls.js index 8b342446112..ec438d57da4 100644 --- a/app/assets/javascripts/pretty-text/engines/discourse-markdown/resize-controls.js +++ b/app/assets/javascripts/pretty-text/engines/discourse-markdown/image-controls.js @@ -1,3 +1,5 @@ +import I18n from "I18n"; + const SCALES = ["100", "75", "50"]; function isUpload(token) { @@ -65,51 +67,81 @@ function buildScaleButton(selectedScale, scale) { ); } +function buildImageAltTextButton(altText) { + return ` + + ${altText} + + + +`; +} + // We need this to load after `upload-protocol` which is priority 0 export const priority = 1; +function ruleWithImageControls(oldRule) { + return function (tokens, idx, options, env, slf) { + const token = tokens[idx]; + const scaleIndex = token.attrIndex("scale"); + const imageIndex = token.attrIndex("index-image"); + + if (scaleIndex !== -1) { + let selectedScale = token.attrs[scaleIndex][1]; + let index = token.attrs[imageIndex][1]; + + let result = ``; + + result += oldRule(tokens, idx, options, env, slf); + + result += ``; + + result += ``; + result += SCALES.map((scale) => + buildScaleButton(selectedScale, scale) + ).join(""); + result += ``; + + result += buildImageAltTextButton(token.attrs[token.attrIndex("alt")][1]); + + result += ""; + + return result; + } else { + return oldRule(tokens, idx, options, env, slf); + } + }; +} + export function setup(helper) { const opts = helper.getOptions(); if (opts.previewing) { helper.allowList([ "span.image-wrapper", "span.button-wrapper", + "span[class=scale-btn-container]", "span[class=scale-btn]", "span[class=scale-btn active]", "span.separator", "span.scale-btn[data-scale]", "span.button-wrapper[data-image-index]", + "span[aria-label]", + "span.alt-text-readonly-container", + "span.alt-text-readonly-container.alt-text", + "span.alt-text-readonly-container.alt-text-edit-btn", + "svg[class=fa d-icon d-icon-pencil svg-icon svg-string]", + "use[xlink:href=#pencil-alt]", + "input[type=text]", + "input[hidden=true]", + "input[class=alt-text-input]", ]); helper.registerPlugin((md) => { const oldRule = md.renderer.rules.image; - md.renderer.rules.image = function (tokens, idx, options, env, slf) { - const token = tokens[idx]; - const scaleIndex = token.attrIndex("scale"); - const imageIndex = token.attrIndex("index-image"); - - if (scaleIndex !== -1) { - let selectedScale = token.attrs[scaleIndex][1]; - let index = token.attrs[imageIndex][1]; - - let result = ""; - result += oldRule(tokens, idx, options, env, slf); - - result += - ""; - - result += SCALES.map((scale) => - buildScaleButton(selectedScale, scale) - ).join(""); - - result += ""; - - return result; - } else { - return oldRule(tokens, idx, options, env, slf); - } - }; + md.renderer.rules.image = ruleWithImageControls(oldRule); md.core.ruler.after("upload-protocol", "resize-controls", rule); }); diff --git a/app/assets/stylesheets/common/d-editor.scss b/app/assets/stylesheets/common/d-editor.scss index a93e7ae3b3e..48446ae7ae7 100644 --- a/app/assets/stylesheets/common/d-editor.scss +++ b/app/assets/stylesheets/common/d-editor.scss @@ -177,23 +177,34 @@ opacity: 1; } } + .button-wrapper { + display: flex; + flex-wrap: wrap; + gap: 0 0.5em; + position: absolute; height: var(--resizer-height); bottom: 0; left: 0; opacity: 0; - display: flex; - align-items: center; transition: all 0.25s; z-index: 1; // needs to be higher than image width: 100%; background: var(--secondary); // for when images are wider than controls + .scale-btn-container, + .alt-text-readonly-container { + background: var(--secondary); + display: flex; + height: var(--resizer-height); + align-items: center; + } + .scale-btn { - background: var(--secondary); // for when controls are wider than image color: var(--tertiary); padding: 0 1em; + &:first-child, &:last-child { padding: 0; @@ -209,6 +220,38 @@ cursor: pointer; } } + + .alt-text-readonly-container { + max-width: 100%; + flex: 1 1; + + .alt-text { + margin-right: 0.5em; + overflow: hidden; + white-space: nowrap; + text-overflow: ellipsis; + max-width: 100%; + } + + .alt-text-edit-btn svg { + padding-right: 0.5em; + pointer-events: none; + } + + .alt-text-input { + height: var(--resizer-height); + width: 100%; + overflow: hidden; + white-space: nowrap; + text-overflow: ellipsis; + max-width: 100%; + margin: 0; + + &[hidden="true"] { + display: none; + } + } + } } } diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 6fc088a64a0..e88a15ced35 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -2234,6 +2234,9 @@ en: reload: "Reload" ignore: "Ignore" + image_alt_text: + aria_label: Alt text for image + notifications: tooltip: regular: