From 08f6067beb1ce0762391b9a4c4e5acc422ff9697 Mon Sep 17 00:00:00 2001
From: Nemanja Glumac <31325167+nemanjaglumac@users.noreply.github.com>
Date: Wed, 16 Sep 2020 15:08:39 +0200
Subject: [PATCH] Improve querying icons in tests (#13230)
* Add `role` and `aria-label` to icons
* Expand `Icon` unit tests
* Simplify `NON_STANDARD_VIEWBOX_ICON`
* Update snapshots related to a11y improvements to icons
* Pass `aria-label` directly as an icon attribute
---
frontend/src/metabase/components/Icon.jsx | 9 ++--
frontend/src/metabase/icon_paths.js | 4 +-
.../metabase/components/Icon.unit.spec.js | 42 ++++++++++++++++---
.../__snapshots__/Button.unit.spec.js.snap | 2 +
.../__snapshots__/Calendar.unit.spec.js.snap | 4 ++
.../components.unit.spec.js.snap | 40 ++++++++++++++++++
6 files changed, 90 insertions(+), 11 deletions(-)
diff --git a/frontend/src/metabase/components/Icon.jsx b/frontend/src/metabase/components/Icon.jsx
index a90a88ec8a1..d47c6d8d899 100644
--- a/frontend/src/metabase/components/Icon.jsx
+++ b/frontend/src/metabase/components/Icon.jsx
@@ -84,12 +84,11 @@ class BaseIcon extends Component {
delete props.size, props.scale;
if (icon.img) {
+ // avoid passing `role="img"` to an actual image file
+ // eslint-disable-next-line no-unused-vars
+ const { role, ...rest } = props;
return (
- <RetinaImage
- forceOriginalDimensions={false}
- src={icon.img}
- {...props}
- />
+ <RetinaImage forceOriginalDimensions={false} src={icon.img} {...rest} />
);
} else if (icon.svg) {
return <svg {...props} dangerouslySetInnerHTML={{ __html: icon.svg }} />;
diff --git a/frontend/src/metabase/icon_paths.js b/frontend/src/metabase/icon_paths.js
index c05887db5c4..286719ee4b7 100644
--- a/frontend/src/metabase/icon_paths.js
+++ b/frontend/src/metabase/icon_paths.js
@@ -414,7 +414,7 @@ ICON_PATHS["scalar"] = ICON_PATHS["number"];
export function parseViewBox(viewBox: string): Array<number> {
// a viewBox is a string that takes the form 'min-x, min-y, width, height'
// grab the values and return just width and height since we currently don't
- // tend to card about min-x or min-y
+ // tend to care about min-x or min-y
// we cast to numbers so we can do math-y stuff with the width and height
return viewBox
@@ -440,6 +440,8 @@ export function loadIcon(name: string) {
width: 16,
height: 16,
fill: "currentcolor",
+ role: "img",
+ "aria-label": name + " icon",
},
svg: undefined,
path: undefined,
diff --git a/frontend/test/metabase/components/Icon.unit.spec.js b/frontend/test/metabase/components/Icon.unit.spec.js
index 73efead971e..d4f2ccc1cb9 100644
--- a/frontend/test/metabase/components/Icon.unit.spec.js
+++ b/frontend/test/metabase/components/Icon.unit.spec.js
@@ -1,11 +1,14 @@
+import React from "react";
+import "@testing-library/jest-dom/extend-expect";
+import { render, screen } from "@testing-library/react";
+
import { ICON_PATHS, loadIcon, parseViewBox } from "metabase/icon_paths";
+import Icon from "metabase/components/Icon";
// find the first icon with a non standard viewBox
-const NON_STANDARD_VIEWBOX_ICON = Object.keys(ICON_PATHS).filter(key => {
- if (ICON_PATHS[key].attrs && ICON_PATHS[key].attrs.viewBox) {
- return ICON_PATHS[key];
- }
-})[0];
+const NON_STANDARD_VIEWBOX_ICON = Object.keys(ICON_PATHS).find(key => {
+ return ICON_PATHS[key].attrs && ICON_PATHS[key].attrs.viewBox;
+});
describe("Icon", () => {
describe("parseViewBox", () => {
@@ -27,4 +30,33 @@ describe("Icon", () => {
expect(height).toEqual(parsedHeight / 2);
});
});
+
+ describe("component", () => {
+ it("should render correct icon for the valid `name`", () => {
+ // used "google" to cover the use of `dangerouslySetInnerHTML`
+ render(<Icon name="google" />);
+ const icon = screen.getByRole("img", { name: /google icon/i });
+
+ expect(icon).toHaveClass("Icon-google");
+ });
+
+ it("should render `unknown` icon given non-existing `name`", () => {
+ render(<Icon name="404-icon-not-found" />);
+ const icon = screen.getByRole("img", { name: /unknown icon/i });
+
+ expect(icon).toHaveClass("Icon-unknown");
+ });
+
+ it("should render `unknown` icon when `name` is not provided", () => {
+ render(<Icon />);
+ const icon = screen.getByRole("img", { name: /unknown icon/i });
+
+ expect(icon).toHaveClass("Icon-unknown");
+ });
+
+ it("should render an image if `img` attribute is present in `ICON_PATHS`", () => {
+ render(<Icon name="slack" />);
+ expect(screen.getByRole("img")).toHaveAttribute("src");
+ });
+ });
});
diff --git a/frontend/test/metabase/components/__snapshots__/Button.unit.spec.js.snap b/frontend/test/metabase/components/__snapshots__/Button.unit.spec.js.snap
index 44644853788..dd827576271 100644
--- a/frontend/test/metabase/components/__snapshots__/Button.unit.spec.js.snap
+++ b/frontend/test/metabase/components/__snapshots__/Button.unit.spec.js.snap
@@ -26,9 +26,11 @@ exports[`Button should render correctly with an icon 1`] = `
style={null}
>
<svg
+ aria-label="star icon"
className="Icon Icon-star Icon-sc-1x67v3y-1 hFNRPZ"
fill="currentcolor"
height={14}
+ role="img"
viewBox="0 0 32 32"
width={14}
>
diff --git a/frontend/test/metabase/components/__snapshots__/Calendar.unit.spec.js.snap b/frontend/test/metabase/components/__snapshots__/Calendar.unit.spec.js.snap
index 103ec83247e..87948065209 100644
--- a/frontend/test/metabase/components/__snapshots__/Calendar.unit.spec.js.snap
+++ b/frontend/test/metabase/components/__snapshots__/Calendar.unit.spec.js.snap
@@ -12,9 +12,11 @@ exports[`Calendar should render correctly 1`] = `
onClick={[Function]}
>
<svg
+ aria-label="chevronleft icon"
className="Icon Icon-chevronleft Icon-sc-1x67v3y-1 hFNRPZ"
fill="currentcolor"
height={10}
+ role="img"
viewBox="0 0 32 32"
width={10}
>
@@ -37,9 +39,11 @@ exports[`Calendar should render correctly 1`] = `
onClick={[Function]}
>
<svg
+ aria-label="chevronright icon"
className="Icon Icon-chevronright Icon-sc-1x67v3y-1 hFNRPZ"
fill="currentcolor"
height={10}
+ role="img"
viewBox="0 0 32 32"
width={10}
>
diff --git a/frontend/test/metabase/internal/__snapshots__/components.unit.spec.js.snap b/frontend/test/metabase/internal/__snapshots__/components.unit.spec.js.snap
index 0fff2fd57b0..032cb8593c1 100644
--- a/frontend/test/metabase/internal/__snapshots__/components.unit.spec.js.snap
+++ b/frontend/test/metabase/internal/__snapshots__/components.unit.spec.js.snap
@@ -43,9 +43,11 @@ exports[`Button should render "with an icon" correctly 1`] = `
style={null}
>
<svg
+ aria-label="star icon"
className="Icon Icon-star Icon-sc-1x67v3y-1 hFNRPZ"
fill="currentcolor"
height={14}
+ role="img"
viewBox="0 0 32 32"
width={14}
>
@@ -259,9 +261,11 @@ exports[`CheckBox should render "Green" correctly 1`] = `
}
>
<svg
+ aria-label="check icon"
className="Icon Icon-check Icon-sc-1x67v3y-1 hFNRPZ"
fill="currentcolor"
height={12}
+ role="img"
style={
Object {
"color": "white",
@@ -291,9 +295,11 @@ exports[`CheckBox should render "On - Default blue" correctly 1`] = `
}
>
<svg
+ aria-label="check icon"
className="Icon Icon-check Icon-sc-1x67v3y-1 hFNRPZ"
fill="currentcolor"
height={12}
+ role="img"
style={
Object {
"color": "white",
@@ -323,9 +329,11 @@ exports[`CheckBox should render "Purple" correctly 1`] = `
}
>
<svg
+ aria-label="check icon"
className="Icon Icon-check Icon-sc-1x67v3y-1 hFNRPZ"
fill="currentcolor"
height={12}
+ role="img"
style={
Object {
"color": "white",
@@ -355,9 +363,11 @@ exports[`CheckBox should render "Red" correctly 1`] = `
}
>
<svg
+ aria-label="check icon"
className="Icon Icon-check Icon-sc-1x67v3y-1 hFNRPZ"
fill="currentcolor"
height={12}
+ role="img"
style={
Object {
"color": "white",
@@ -387,9 +397,11 @@ exports[`CheckBox should render "Yellow" correctly 1`] = `
}
>
<svg
+ aria-label="check icon"
className="Icon Icon-check Icon-sc-1x67v3y-1 hFNRPZ"
fill="currentcolor"
height={12}
+ role="img"
style={
Object {
"color": "white",
@@ -420,9 +432,11 @@ exports[`EntityMenu should render "Edit menu" correctly 1`] = `
onClick={[Function]}
>
<svg
+ aria-label="pencil icon"
className="Icon Icon-pencil Icon-sc-1x67v3y-1 bcaYRA"
fill="currentcolor"
height={18}
+ role="img"
viewBox="0 0 32 32"
width={18}
>
@@ -454,9 +468,11 @@ exports[`EntityMenu should render "More menu" correctly 1`] = `
onClick={[Function]}
>
<svg
+ aria-label="burger icon"
className="Icon Icon-burger Icon-sc-1x67v3y-1 bcaYRA"
fill="currentcolor"
height={18}
+ role="img"
viewBox="0 0 32 32"
width={18}
>
@@ -488,9 +504,11 @@ exports[`EntityMenu should render "Multiple menus" correctly 1`] = `
onClick={[Function]}
>
<svg
+ aria-label="pencil icon"
className="Icon Icon-pencil Icon-sc-1x67v3y-1 bcaYRA"
fill="currentcolor"
height={18}
+ role="img"
viewBox="0 0 32 32"
width={18}
>
@@ -511,10 +529,12 @@ exports[`EntityMenu should render "Multiple menus" correctly 1`] = `
onClick={[Function]}
>
<svg
+ aria-label="share icon"
className="Icon Icon-share Icon-sc-1x67v3y-1 bcaYRA"
fill="currentcolor"
fillRule="evenodd"
height={18}
+ role="img"
viewBox="0 0 32 41"
width={18}
>
@@ -535,9 +555,11 @@ exports[`EntityMenu should render "Multiple menus" correctly 1`] = `
onClick={[Function]}
>
<svg
+ aria-label="burger icon"
className="Icon Icon-burger Icon-sc-1x67v3y-1 bcaYRA"
fill="currentcolor"
height={18}
+ role="img"
viewBox="0 0 32 32"
width={18}
>
@@ -569,10 +591,12 @@ exports[`EntityMenu should render "Share menu" correctly 1`] = `
onClick={[Function]}
>
<svg
+ aria-label="share icon"
className="Icon Icon-share Icon-sc-1x67v3y-1 bcaYRA"
fill="currentcolor"
fillRule="evenodd"
height={18}
+ role="img"
viewBox="0 0 32 41"
width={18}
>
@@ -598,10 +622,12 @@ exports[`ModalContent should render "default" correctly 1`] = `
id={undefined}
>
<svg
+ aria-label="close icon"
className="Icon Icon-close text-light text-medium-hover cursor-pointer absolute z2 m2 p2 top right Icon-sc-1x67v3y-1 hFNRPZ"
fill="currentcolor"
height={16}
onClick={[Function]}
+ role="img"
viewBox="0 0 32 32"
width={16}
>
@@ -987,9 +1013,11 @@ exports[`Select should render "default" correctly 1`] = `
</span>
</span>
<svg
+ aria-label="chevrondown icon"
className="Icon Icon-chevrondown AdminSelect-chevron flex-align-right Icon-sc-1x67v3y-1 hFNRPZ"
fill="currentcolor"
height={12}
+ role="img"
viewBox="0 0 32 32"
width={12}
>
@@ -1028,9 +1056,11 @@ exports[`Select should render "kitchen_sink" correctly 1`] = `
</span>
</span>
<svg
+ aria-label="chevrondown icon"
className="Icon Icon-chevrondown AdminSelect-chevron flex-align-right Icon-sc-1x67v3y-1 hFNRPZ"
fill="currentcolor"
height={12}
+ role="img"
viewBox="0 0 32 32"
width={12}
>
@@ -1069,9 +1099,11 @@ exports[`Select should render "multiple" correctly 1`] = `
</span>
</span>
<svg
+ aria-label="chevrondown icon"
className="Icon Icon-chevrondown AdminSelect-chevron flex-align-right Icon-sc-1x67v3y-1 hFNRPZ"
fill="currentcolor"
height={12}
+ role="img"
viewBox="0 0 32 32"
width={12}
>
@@ -1106,9 +1138,11 @@ exports[`Select should render "search" correctly 1`] = `
</span>
</span>
<svg
+ aria-label="chevrondown icon"
className="Icon Icon-chevrondown AdminSelect-chevron flex-align-right Icon-sc-1x67v3y-1 hFNRPZ"
fill="currentcolor"
height={12}
+ role="img"
viewBox="0 0 32 32"
width={12}
>
@@ -1143,9 +1177,11 @@ exports[`Select should render "sections" correctly 1`] = `
</span>
</span>
<svg
+ aria-label="chevrondown icon"
className="Icon Icon-chevrondown AdminSelect-chevron flex-align-right Icon-sc-1x67v3y-1 hFNRPZ"
fill="currentcolor"
height={12}
+ role="img"
viewBox="0 0 32 32"
width={12}
>
@@ -1182,9 +1218,11 @@ exports[`StackedCheckBox should render "Checked with color" correctly 1`] = `
}
>
<svg
+ aria-label="check icon"
className="Icon Icon-check Icon-sc-1x67v3y-1 hFNRPZ"
fill="currentcolor"
height={12}
+ role="img"
style={
Object {
"color": "white",
@@ -1238,9 +1276,11 @@ exports[`StackedCheckBox should render "Checked" correctly 1`] = `
}
>
<svg
+ aria-label="check icon"
className="Icon Icon-check Icon-sc-1x67v3y-1 hFNRPZ"
fill="currentcolor"
height={12}
+ role="img"
style={
Object {
"color": "white",
--
GitLab