FEATURE: Allow users to edit alt text from the image preview in the editor (#14480)

2024-11-27 02:03:44 +08:00 · 2021-11-09 14:34:09 +08:00 · 2021-11-09 14:34:09 +08:00 · 0b495e9ad4
commit 0b495e9ad4
parent 46fed1303f
5 changed files with 315 additions and 46 deletions
--- 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);
--- 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");
--- a/app/assets/javascripts/pretty-text/engines/discourse-markdown/resize-controls.js
+++ b/app/assets/javascripts/pretty-text/engines/discourse-markdown/resize-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 `
+<span class="alt-text-readonly-container">
+  <span class="alt-text" aria-label="${I18n.t(
+    "composer.image_alt_text.aria_label"
+  )}">${altText}</span>
+  <span class="alt-text-edit-btn"><svg aria-hidden="true" class="fa d-icon d-icon-pencil svg-icon svg-string"><use xlink:href="#pencil-alt"></use></svg></span>
+  <input class="alt-text-input" hidden="true" type="text" value="${altText}" />
+</span>
+`;
+}
+
 // 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 = `<span class="image-wrapper">`;
+
+      result += oldRule(tokens, idx, options, env, slf);
+
+      result += `<span class="button-wrapper" data-image-index="${index}">`;
+
+      result += `<span class="scale-btn-container">`;
+      result += SCALES.map((scale) =>
+        buildScaleButton(selectedScale, scale)
+      ).join("");
+      result += `</span>`;
+
+      result += buildImageAltTextButton(token.attrs[token.attrIndex("alt")][1]);
+
+      result += "</span></span>";
+
+      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 = "<span class='image-wrapper'>";
-          result += oldRule(tokens, idx, options, env, slf);
-
-          result +=
-            "<span class='button-wrapper' data-image-index='" + index + "'>";
-
-          result += SCALES.map((scale) =>
-            buildScaleButton(selectedScale, scale)
-          ).join("");
-
-          result += "</span></span>";
-
-          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);
    });
--- 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;
+        }
+      }
+    }
  }
 }

--- 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: