Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@uppy/image-editor: fix tooltips #5156

Conversation

avneetmalhotra
Copy link
Contributor

@avneetmalhotra avneetmalhotra commented May 10, 2024

Due to removal of "aria-label='<tooltip-text>'" from control button's label the tooltip has stopped working. PR ref - #5111

Based on package being used for tooltips https://www.npmjs.com/package/microtip#usage, there should be following attributes for the tooltip to work:-

  1. aria-label
  2. data-microtip-position
  3. role="tooltip"

So, in this PR I have removed the label element and moved all 3 attributes to button element now

Copy link
Contributor

Diff output files
diff --git a/packages/@uppy/image-editor/lib/Editor.js b/packages/@uppy/image-editor/lib/Editor.js
index 4a372db..6ddf3cf 100644
--- a/packages/@uppy/image-editor/lib/Editor.js
+++ b/packages/@uppy/image-editor/lib/Editor.js
@@ -118,43 +118,38 @@ export default class Editor extends Component {
       opts,
     } = this.props;
     return h(
-      "label",
+      "button",
       {
-        role: "tooltip",
+        role: "button tooltip",
         "data-microtip-position": "top",
+        type: "button",
+        className: "uppy-u-reset uppy-c-btn",
+        "aria-label": i18n("revert"),
+        onClick: () => {
+          this.cropper.reset();
+          this.cropper.setAspectRatio(opts.cropperOptions.initialAspectRatio);
+          this.setState({
+            angle90Deg: 0,
+            angleGranular: 0,
+          });
+        },
       },
       h(
-        "button",
+        "svg",
         {
-          type: "button",
-          className: "uppy-u-reset uppy-c-btn",
-          "aria-label": i18n("revert"),
-          onClick: () => {
-            this.cropper.reset();
-            this.cropper.setAspectRatio(opts.cropperOptions.initialAspectRatio);
-            this.setState({
-              angle90Deg: 0,
-              angleGranular: 0,
-            });
-          },
+          "aria-hidden": "true",
+          className: "uppy-c-icon",
+          width: "24",
+          height: "24",
+          viewBox: "0 0 24 24",
         },
-        h(
-          "svg",
-          {
-            "aria-hidden": "true",
-            className: "uppy-c-icon",
-            width: "24",
-            height: "24",
-            viewBox: "0 0 24 24",
-          },
-          h("path", {
-            d: "M0 0h24v24H0z",
-            fill: "none",
-          }),
-          h("path", {
-            d: "M13 3c-4.97 0-9 4.03-9 9H1l3.89 3.89.07.14L9 12H6c0-3.87 3.13-7 7-7s7 3.13 7 7-3.13 7-7 7c-1.93 0-3.68-.79-4.94-2.06l-1.42 1.42C8.27 19.99 10.51 21 13 21c4.97 0 9-4.03 9-9s-4.03-9-9-9zm-1 5v5l4.28 2.54.72-1.21-3.5-2.08V8H12z",
-          }),
-        ),
+        h("path", {
+          d: "M0 0h24v24H0z",
+          fill: "none",
+        }),
+        h("path", {
+          d: "M13 3c-4.97 0-9 4.03-9 9H1l3.89 3.89.07.14L9 12H6c0-3.87 3.13-7 7-7s7 3.13 7 7-3.13 7-7 7c-1.93 0-3.68-.79-4.94-2.06l-1.42 1.42C8.27 19.99 10.51 21 13 21c4.97 0 9-4.03 9-9s-4.03-9-9-9zm-1 5v5l4.28 2.54.72-1.21-3.5-2.08V8H12z",
+        }),
       ),
     );
   }
@@ -163,36 +158,31 @@ export default class Editor extends Component {
       i18n,
     } = this.props;
     return h(
-      "label",
+      "button",
       {
-        role: "tooltip",
+        role: "button tooltip",
         "data-microtip-position": "top",
+        type: "button",
+        className: "uppy-u-reset uppy-c-btn",
+        "aria-label": i18n("rotate"),
+        onClick: this.onRotate90Deg,
       },
       h(
-        "button",
+        "svg",
         {
-          type: "button",
-          className: "uppy-u-reset uppy-c-btn",
-          "aria-label": i18n("rotate"),
-          onClick: this.onRotate90Deg,
+          "aria-hidden": "true",
+          className: "uppy-c-icon",
+          width: "24",
+          height: "24",
+          viewBox: "0 0 24 24",
         },
-        h(
-          "svg",
-          {
-            "aria-hidden": "true",
-            className: "uppy-c-icon",
-            width: "24",
-            height: "24",
-            viewBox: "0 0 24 24",
-          },
-          h("path", {
-            d: "M0 0h24v24H0V0zm0 0h24v24H0V0z",
-            fill: "none",
-          }),
-          h("path", {
-            d: "M14 10a2 2 0 012 2v7a2 2 0 01-2 2H6a2 2 0 01-2-2v-7a2 2 0 012-2h8zm0 1.75H6a.25.25 0 00-.243.193L5.75 12v7a.25.25 0 00.193.243L6 19.25h8a.25.25 0 00.243-.193L14.25 19v-7a.25.25 0 00-.193-.243L14 11.75zM12 .76V4c2.3 0 4.61.88 6.36 2.64a8.95 8.95 0 012.634 6.025L21 13a1 1 0 01-1.993.117L19 13h-.003a6.979 6.979 0 00-2.047-4.95 6.97 6.97 0 00-4.652-2.044L12 6v3.24L7.76 5 12 .76z",
-          }),
-        ),
+        h("path", {
+          d: "M0 0h24v24H0V0zm0 0h24v24H0V0z",
+          fill: "none",
+        }),
+        h("path", {
+          d: "M14 10a2 2 0 012 2v7a2 2 0 01-2 2H6a2 2 0 01-2-2v-7a2 2 0 012-2h8zm0 1.75H6a.25.25 0 00-.243.193L5.75 12v7a.25.25 0 00.193.243L6 19.25h8a.25.25 0 00.243-.193L14.25 19v-7a.25.25 0 00-.193-.243L14 11.75zM12 .76V4c2.3 0 4.61.88 6.36 2.64a8.95 8.95 0 012.634 6.025L21 13a1 1 0 01-1.993.117L19 13h-.003a6.979 6.979 0 00-2.047-4.95 6.97 6.97 0 00-4.652-2.044L12 6v3.24L7.76 5 12 .76z",
+        }),
       ),
     );
   }
@@ -201,36 +191,31 @@ export default class Editor extends Component {
       i18n,
     } = this.props;
     return h(
-      "label",
+      "button",
       {
-        role: "tooltip",
+        role: "button tooltip",
         "data-microtip-position": "top",
+        type: "button",
+        className: "uppy-u-reset uppy-c-btn",
+        "aria-label": i18n("flipHorizontal"),
+        onClick: () => this.cropper.scaleX(-this.cropper.getData().scaleX || -1),
       },
       h(
-        "button",
+        "svg",
         {
-          type: "button",
-          className: "uppy-u-reset uppy-c-btn",
-          "aria-label": i18n("flipHorizontal"),
-          onClick: () => this.cropper.scaleX(-this.cropper.getData().scaleX || -1),
+          "aria-hidden": "true",
+          className: "uppy-c-icon",
+          width: "24",
+          height: "24",
+          viewBox: "0 0 24 24",
         },
-        h(
-          "svg",
-          {
-            "aria-hidden": "true",
-            className: "uppy-c-icon",
-            width: "24",
-            height: "24",
-            viewBox: "0 0 24 24",
-          },
-          h("path", {
-            d: "M0 0h24v24H0z",
-            fill: "none",
-          }),
-          h("path", {
-            d: "M15 21h2v-2h-2v2zm4-12h2V7h-2v2zM3 5v14c0 1.1.9 2 2 2h4v-2H5V5h4V3H5c-1.1 0-2 .9-2 2zm16-2v2h2c0-1.1-.9-2-2-2zm-8 20h2V1h-2v22zm8-6h2v-2h-2v2zM15 5h2V3h-2v2zm4 8h2v-2h-2v2zm0 8c1.1 0 2-.9 2-2h-2v2z",
-          }),
-        ),
+        h("path", {
+          d: "M0 0h24v24H0z",
+          fill: "none",
+        }),
+        h("path", {
+          d: "M15 21h2v-2h-2v2zm4-12h2V7h-2v2zM3 5v14c0 1.1.9 2 2 2h4v-2H5V5h4V3H5c-1.1 0-2 .9-2 2zm16-2v2h2c0-1.1-.9-2-2-2zm-8 20h2V1h-2v22zm8-6h2v-2h-2v2zM15 5h2V3h-2v2zm4 8h2v-2h-2v2zm0 8c1.1 0 2-.9 2-2h-2v2z",
+        }),
       ),
     );
   }
@@ -239,39 +224,34 @@ export default class Editor extends Component {
       i18n,
     } = this.props;
     return h(
-      "label",
+      "button",
       {
-        role: "tooltip",
+        role: "button tooltip",
         "data-microtip-position": "top",
+        type: "button",
+        className: "uppy-u-reset uppy-c-btn",
+        "aria-label": i18n("zoomIn"),
+        onClick: () => this.cropper.zoom(0.1),
       },
       h(
-        "button",
+        "svg",
         {
-          type: "button",
-          className: "uppy-u-reset uppy-c-btn",
-          "aria-label": i18n("zoomIn"),
-          onClick: () => this.cropper.zoom(0.1),
+          "aria-hidden": "true",
+          className: "uppy-c-icon",
+          height: "24",
+          viewBox: "0 0 24 24",
+          width: "24",
         },
-        h(
-          "svg",
-          {
-            "aria-hidden": "true",
-            className: "uppy-c-icon",
-            height: "24",
-            viewBox: "0 0 24 24",
-            width: "24",
-          },
-          h("path", {
-            d: "M0 0h24v24H0V0z",
-            fill: "none",
-          }),
-          h("path", {
-            d: "M15.5 14h-.79l-.28-.27C15.41 12.59 16 11.11 16 9.5 16 5.91 13.09 3 9.5 3S3 5.91 3 9.5 5.91 16 9.5 16c1.61 0 3.09-.59 4.23-1.57l.27.28v.79l5 4.99L20.49 19l-4.99-5zm-6 0C7.01 14 5 11.99 5 9.5S7.01 5 9.5 5 14 7.01 14 9.5 11.99 14 9.5 14z",
-          }),
-          h("path", {
-            d: "M12 10h-2v2H9v-2H7V9h2V7h1v2h2v1z",
-          }),
-        ),
+        h("path", {
+          d: "M0 0h24v24H0V0z",
+          fill: "none",
+        }),
+        h("path", {
+          d: "M15.5 14h-.79l-.28-.27C15.41 12.59 16 11.11 16 9.5 16 5.91 13.09 3 9.5 3S3 5.91 3 9.5 5.91 16 9.5 16c1.61 0 3.09-.59 4.23-1.57l.27.28v.79l5 4.99L20.49 19l-4.99-5zm-6 0C7.01 14 5 11.99 5 9.5S7.01 5 9.5 5 14 7.01 14 9.5 11.99 14 9.5 14z",
+        }),
+        h("path", {
+          d: "M12 10h-2v2H9v-2H7V9h2V7h1v2h2v1z",
+        }),
       ),
     );
   }
@@ -280,36 +260,31 @@ export default class Editor extends Component {
       i18n,
     } = this.props;
     return h(
-      "label",
+      "button",
       {
-        role: "tooltip",
+        role: "button tooltip",
         "data-microtip-position": "top",
+        type: "button",
+        className: "uppy-u-reset uppy-c-btn",
+        "aria-label": i18n("zoomOut"),
+        onClick: () => this.cropper.zoom(-0.1),
       },
       h(
-        "button",
+        "svg",
         {
-          type: "button",
-          className: "uppy-u-reset uppy-c-btn",
-          "aria-label": i18n("zoomOut"),
-          onClick: () => this.cropper.zoom(-0.1),
+          "aria-hidden": "true",
+          className: "uppy-c-icon",
+          width: "24",
+          height: "24",
+          viewBox: "0 0 24 24",
         },
-        h(
-          "svg",
-          {
-            "aria-hidden": "true",
-            className: "uppy-c-icon",
-            width: "24",
-            height: "24",
-            viewBox: "0 0 24 24",
-          },
-          h("path", {
-            d: "M0 0h24v24H0V0z",
-            fill: "none",
-          }),
-          h("path", {
-            d: "M15.5 14h-.79l-.28-.27C15.41 12.59 16 11.11 16 9.5 16 5.91 13.09 3 9.5 3S3 5.91 3 9.5 5.91 16 9.5 16c1.61 0 3.09-.59 4.23-1.57l.27.28v.79l5 4.99L20.49 19l-4.99-5zm-6 0C7.01 14 5 11.99 5 9.5S7.01 5 9.5 5 14 7.01 14 9.5 11.99 14 9.5 14zM7 9h5v1H7z",
-          }),
-        ),
+        h("path", {
+          d: "M0 0h24v24H0V0z",
+          fill: "none",
+        }),
+        h("path", {
+          d: "M15.5 14h-.79l-.28-.27C15.41 12.59 16 11.11 16 9.5 16 5.91 13.09 3 9.5 3S3 5.91 3 9.5 5.91 16 9.5 16c1.61 0 3.09-.59 4.23-1.57l.27.28v.79l5 4.99L20.49 19l-4.99-5zm-6 0C7.01 14 5 11.99 5 9.5S7.01 5 9.5 5 14 7.01 14 9.5 11.99 14 9.5 14zM7 9h5v1H7z",
+        }),
       ),
     );
   }
@@ -318,36 +293,31 @@ export default class Editor extends Component {
       i18n,
     } = this.props;
     return h(
-      "label",
+      "button",
       {
-        role: "tooltip",
+        role: "button tooltip",
         "data-microtip-position": "top",
+        type: "button",
+        className: "uppy-u-reset uppy-c-btn",
+        "aria-label": i18n("aspectRatioSquare"),
+        onClick: () => this.cropper.setAspectRatio(1),
       },
       h(
-        "button",
+        "svg",
         {
-          type: "button",
-          className: "uppy-u-reset uppy-c-btn",
-          "aria-label": i18n("aspectRatioSquare"),
-          onClick: () => this.cropper.setAspectRatio(1),
+          "aria-hidden": "true",
+          className: "uppy-c-icon",
+          width: "24",
+          height: "24",
+          viewBox: "0 0 24 24",
         },
-        h(
-          "svg",
-          {
-            "aria-hidden": "true",
-            className: "uppy-c-icon",
-            width: "24",
-            height: "24",
-            viewBox: "0 0 24 24",
-          },
-          h("path", {
-            d: "M0 0h24v24H0z",
-            fill: "none",
-          }),
-          h("path", {
-            d: "M19 5v14H5V5h14m0-2H5c-1.1 0-2 .9-2 2v14c0 1.1.9 2 2 2h14c1.1 0 2-.9 2-2V5c0-1.1-.9-2-2-2z",
-          }),
-        ),
+        h("path", {
+          d: "M0 0h24v24H0z",
+          fill: "none",
+        }),
+        h("path", {
+          d: "M19 5v14H5V5h14m0-2H5c-1.1 0-2 .9-2 2v14c0 1.1.9 2 2 2h14c1.1 0 2-.9 2-2V5c0-1.1-.9-2-2-2z",
+        }),
       ),
     );
   }
@@ -356,36 +326,31 @@ export default class Editor extends Component {
       i18n,
     } = this.props;
     return h(
-      "label",
+      "button",
       {
-        role: "tooltip",
+        role: "button tooltip",
         "data-microtip-position": "top",
+        type: "button",
+        className: "uppy-u-reset uppy-c-btn",
+        "aria-label": i18n("aspectRatioLandscape"),
+        onClick: () => this.cropper.setAspectRatio(16 / 9),
       },
       h(
-        "button",
+        "svg",
         {
-          type: "button",
-          className: "uppy-u-reset uppy-c-btn",
-          "aria-label": i18n("aspectRatioLandscape"),
-          onClick: () => this.cropper.setAspectRatio(16 / 9),
+          "aria-hidden": "true",
+          className: "uppy-c-icon",
+          width: "24",
+          height: "24",
+          viewBox: "0 0 24 24",
         },
-        h(
-          "svg",
-          {
-            "aria-hidden": "true",
-            className: "uppy-c-icon",
-            width: "24",
-            height: "24",
-            viewBox: "0 0 24 24",
-          },
-          h("path", {
-            d: "M 19,4.9999992 V 17.000001 H 4.9999998 V 6.9999992 H 19 m 0,-2 H 4.9999998 c -1.0999999,0 -1.9999999,0.9000001 -1.9999999,2 V 17.000001 c 0,1.1 0.9,2 1.9999999,2 H 19 c 1.1,0 2,-0.9 2,-2 V 6.9999992 c 0,-1.0999999 -0.9,-2 -2,-2 z",
-          }),
-          h("path", {
-            fill: "none",
-            d: "M0 0h24v24H0z",
-          }),
-        ),
+        h("path", {
+          d: "M 19,4.9999992 V 17.000001 H 4.9999998 V 6.9999992 H 19 m 0,-2 H 4.9999998 c -1.0999999,0 -1.9999999,0.9000001 -1.9999999,2 V 17.000001 c 0,1.1 0.9,2 1.9999999,2 H 19 c 1.1,0 2,-0.9 2,-2 V 6.9999992 c 0,-1.0999999 -0.9,-2 -2,-2 z",
+        }),
+        h("path", {
+          fill: "none",
+          d: "M0 0h24v24H0z",
+        }),
       ),
     );
   }
@@ -394,36 +359,31 @@ export default class Editor extends Component {
       i18n,
     } = this.props;
     return h(
-      "label",
+      "button",
       {
-        role: "tooltip",
+        role: "button tooltip",
         "data-microtip-position": "top",
+        type: "button",
+        "aria-label": i18n("aspectRatioPortrait"),
+        className: "uppy-u-reset uppy-c-btn",
+        onClick: () => this.cropper.setAspectRatio(9 / 16),
       },
       h(
-        "button",
+        "svg",
         {
-          type: "button",
-          "aria-label": i18n("aspectRatioPortrait"),
-          className: "uppy-u-reset uppy-c-btn",
-          onClick: () => this.cropper.setAspectRatio(9 / 16),
+          "aria-hidden": "true",
+          className: "uppy-c-icon",
+          width: "24",
+          height: "24",
+          viewBox: "0 0 24 24",
         },
-        h(
-          "svg",
-          {
-            "aria-hidden": "true",
-            className: "uppy-c-icon",
-            width: "24",
-            height: "24",
-            viewBox: "0 0 24 24",
-          },
-          h("path", {
-            d: "M 19.000001,19 H 6.999999 V 5 h 10.000002 v 14 m 2,0 V 5 c 0,-1.0999999 -0.9,-1.9999999 -2,-1.9999999 H 6.999999 c -1.1,0 -2,0.9 -2,1.9999999 v 14 c 0,1.1 0.9,2 2,2 h 10.000002 c 1.1,0 2,-0.9 2,-2 z",
-          }),
-          h("path", {
-            d: "M0 0h24v24H0z",
-            fill: "none",
-          }),
-        ),
+        h("path", {
+          d: "M 19.000001,19 H 6.999999 V 5 h 10.000002 v 14 m 2,0 V 5 c 0,-1.0999999 -0.9,-1.9999999 -2,-1.9999999 H 6.999999 c -1.1,0 -2,0.9 -2,1.9999999 v 14 c 0,1.1 0.9,2 2,2 h 10.000002 c 1.1,0 2,-0.9 2,-2 z",
+        }),
+        h("path", {
+          d: "M0 0h24v24H0z",
+          fill: "none",
+        }),
       ),
     );
   }

Copy link
Member

@Murderlon Murderlon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for noticing and making a PR

@Murderlon Murderlon changed the title image-editor-controls-tooltip-not-showing @uppy/image-editor: fix tooltips May 13, 2024
@Murderlon Murderlon merged commit 4acd793 into transloadit:main May 13, 2024
17 checks passed
@avneetmalhotra avneetmalhotra deleted the image-editor-controls-tooltip-not-showing branch May 13, 2024 08:39
@avneetmalhotra
Copy link
Contributor Author

@Murderlon when do you think this fix will be released?

@Murderlon
Copy link
Member

I can do a release today

@avneetmalhotra
Copy link
Contributor Author

Thanks

@github-actions github-actions bot mentioned this pull request May 14, 2024
github-actions bot added a commit that referenced this pull request May 14, 2024
| Package            | Version | Package            | Version |
| ------------------ | ------- | ------------------ | ------- |
| @uppy/core         |  3.11.3 | uppy               |  3.25.3 |
| @uppy/image-editor |   2.4.6 |                    |         |

- @uppy/image-editor: fix tooltips (Avneet Singh Malhotra / #5156)
- meta: Remove redundant `plugins` prop from examples (Merlijn Vos / #5145)
- @uppy/image-editor: Remove `target` option from examples and document consistently (Merlijn Vos / #5146)
- @uppy/core: make getObjectOfFilesPerState more efficient (Merlijn Vos / #5155)
Murderlon added a commit that referenced this pull request May 14, 2024
* commit '86e2e6750192055bfba24abf62e227976be5e085':
  Release: uppy@3.25.3 (#5161)
  @uppy/image-editor: fix tooltips (#5156)
  Remove redundant `plugins` prop from examples (#5145)
  Remove `target` option from examples and document consistently (#5146)
  @uppy/core: make getObjectOfFilesPerState more efficient (#5155)
  Release: uppy@3.25.2 (#5151)
  Upgrade @transloadit/prettier-bytes (#5150)
github-actions bot added a commit that referenced this pull request May 14, 2024
| Package                |      Version | Package                |      Version |
| ---------------------- | ------------ | ---------------------- | ------------ |
| @uppy/companion        | 5.0.0-beta.6 | @uppy/status-bar       | 4.0.0-beta.7 |
| @uppy/companion-client | 4.0.0-beta.6 | @uppy/unsplash         | 4.0.0-beta.6 |
| @uppy/compressor       | 2.0.0-beta.7 | @uppy/url              | 4.0.0-beta.6 |
| @uppy/core             | 4.0.0-beta.7 | @uppy/utils            | 6.0.0-beta.6 |
| @uppy/dashboard        | 4.0.0-beta.7 | @uppy/webcam           | 4.0.0-beta.6 |
| @uppy/dropbox          | 4.0.0-beta.6 | @uppy/xhr-upload       | 4.0.0-beta.4 |
| @uppy/image-editor     | 3.0.0-beta.4 | uppy                   | 4.0.0-beta.7 |
| @uppy/screen-capture   | 4.0.0-beta.5 |                        |              |

- @uppy/companion: switch from `node-redis` to `ioredis` (Dominik Schmidt / #4623)
- meta: Fix headings in xhr.mdx (Merlijn Vos)
- @uppy/xhr-upload: introduce hooks similar to tus (Merlijn Vos / #5094)
- @uppy/core: close->destroy, clearUploadedFiles->clear (Merlijn Vos / #5154)
- @uppy/companion-client,@uppy/dropbox,@uppy/screen-capture,@uppy/unsplash,@uppy/url,@uppy/webcam: Use `title` consistently from locales (Merlijn Vos / #5134)




| Package            | Version | Package            | Version |
| ------------------ | ------- | ------------------ | ------- |
| @uppy/core         |  3.11.3 | uppy               |  3.25.3 |
| @uppy/image-editor |   2.4.6 |                    |         |

- @uppy/image-editor: fix tooltips (Avneet Singh Malhotra / #5156)
- meta: Remove redundant `plugins` prop from examples (Merlijn Vos / #5145)
- @uppy/image-editor: Remove `target` option from examples and document consistently (Merlijn Vos / #5146)
- @uppy/core: make getObjectOfFilesPerState more efficient (Merlijn Vos / #5155)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants