Bug 1547068 - making badges in tree non-interactive as per ui-review. r=mtigley

Depends on D29342

Differential Revision: https://phabricator.services.mozilla.com/D29363

--HG--
extra : moz-landing-system : lando

devtools/client/accessibility/accessibility.css

@@ -22,7 +22,6 @@
   --badge-active-background-color: var(--blue-50);
   --badge-active-border-color: #FFFFFFB3;
   --badge-interactive-background-color: var(--grey-20);
-  --badge-interactive-color: var(--grey-90);
   --accessible-label-background-color: white;
   --accessible-label-border-color: #CACAD1;
   --accessible-label-color: var(--grey-60);
@@ -38,7 +37,6 @@
   --badge-active-background-color: var(--blue-60);
   --badge-active-border-color: #FFF6;
   --badge-interactive-background-color: var(--grey-70);
-  --badge-interactive-color: var(--grey-30);
   --accessible-label-background-color: var(--grey-80);
   --accessible-label-border-color: var(--grey-50);
   --accessible-label-color: var(--grey-40);
@@ -334,18 +332,20 @@ body {
   color: var(--theme-toolbar-color);
 }
 
-.badge.toggle-button,
-.mainFrame .treeTable .treeRow .badges .badge {
-  background-color: var(--badge-interactive-background-color);
-  color: var(--badge-interactive-color);
+.badge {
+  font: message-box;
   border-radius: 3px;
   padding: 0px 2px;
   margin-inline-start: 5px;
+  color: var(--accessible-label-color);
+  background-color: var(--accessible-label-background-color);
+  border: 1px solid var(--accessible-label-border-color);
 }
 
 .badge.toggle-button {
-  border: 1px solid transparent;
   color: var(--theme-body-color);
+  background-color: var(--badge-interactive-background-color);
+  border-color: transparent;
 }
 
 .devtools-toolbar .badge.toggle-button:focus {
@@ -355,25 +355,19 @@ body {
   -moz-outline-radius: 2px;
 }
 
-.mainFrame .treeTable .treeRow .badges .badge {
-  border: 1px solid var(--accessible-label-border-color);
-}
-
-.mainFrame .treeTable:focus .treeRow.selected .badges .badge {
-  background-color: var(--badge-interactive-background-color);
-  border: 1px solid var(--accessible-label-border-color);
-  color: var(--badge-interactive-color);
-}
-
-.badge.toggle-button.checked,
-.mainFrame .treeTable:focus .treeRow.selected .badges .badge.checked,
-.mainFrame .treeTable .treeRow .badges .badge.checked {
+.treeTable:focus .treeRow.selected .badges .badge {
   background-color: var(--badge-active-background-color);
+  border-color: var(--accessible-active-border-color);
   color: var(--theme-selection-color);
 }
 
-.mainFrame .treeTable .treeRow .badges .badge.checked {
-  border: 1px solid var(--badge-active-border-color);
+.treeTable:not(:focus):not(:focus-within) .treeRow.selected .badges .badge {
+  color: var(--accessible-label-color);
+}
+
+.badge.toggle-button.checked {
+  background-color: var(--badge-active-background-color);
+  color: var(--theme-selection-color);
 }
 
 /* Avoid having a default dotted border on keyboard focus since we provide focus

devtools/client/accessibility/components/AccessibilityRowValue.js

@@ -24,12 +24,11 @@ class AccessibilityRowValue extends Component {
         object: PropTypes.object,
       }).isRequired,
       supports: PropTypes.object.isRequired,
-      walker: PropTypes.object.isRequired,
     };
   }
 
   render() {
-    const { member, supports: { audit }, walker } = this.props;
+    const { member, supports: { audit } } = this.props;
 
     return span({
       role: "presentation",
@@ -42,7 +41,7 @@ class AccessibilityRowValue extends Component {
       audit && AuditController({
         accessible: member.object,
       },
-        Badges({ walker })),
+        Badges()),
     );
   }
 }

devtools/client/accessibility/components/AccessibilityTree.js

@@ -134,12 +134,7 @@ class AccessibilityTree extends Component {
   }
 
   renderValue(props) {
-    const { walker } = this.props;
-
-    return AccessibilityRowValue({
-      ...props,
-      walker,
-    });
+    return AccessibilityRowValue(props);
   }
 
   /**

devtools/client/accessibility/components/Badge.js

@@ -4,95 +4,28 @@
 "use strict";
 
 // React
-const { Component, createFactory } = require("devtools/client/shared/vendor/react");
+const { Component } = require("devtools/client/shared/vendor/react");
+const { span } = require("devtools/client/shared/vendor/react-dom-factories");
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
-const { connect } = require("devtools/client/shared/vendor/react-redux");
-
-const ToggleButton = createFactory(require("./Button").ToggleButton);
-
-const { audit, auditing, filterToggle } = require("../actions/audit");
-const { preventDefaultAndStopPropagation } = require("devtools/client/shared/events");
 
 class Badge extends Component {
   static get propTypes() {
     return {
-      active: PropTypes.bool.isRequired,
-      filterKey: PropTypes.string.isRequired,
-      dispatch: PropTypes.func.isRequired,
       label: PropTypes.string.isRequired,
       tooltip: PropTypes.string,
-      walker: PropTypes.object.isRequired,
     };
   }
 
-  constructor(props) {
-    super(props);
-
-    this.toggleFilter = this.toggleFilter.bind(this);
-    this.onClick = this.onClick.bind(this);
-    this.onKeyDown = this.onKeyDown.bind(this);
-  }
-
-  shouldComponentUpdate(nextProps) {
-    return nextProps.active !== this.props.active;
-  }
-
-  async toggleFilter() {
-    const { dispatch, filterKey, walker, active } = this.props;
-    if (!active) {
-      dispatch(auditing(filterKey));
-      await dispatch(audit(walker, filterKey));
-    }
-
-    // We wait to dispatch filter toggle until the tree is ready to be filtered
-    // right after the audit. This is to make sure that we render an empty tree
-    // (filtered) while the audit is running.
-    dispatch(filterToggle(filterKey));
-  }
-
-  onClick(e) {
-    preventDefaultAndStopPropagation(e);
-    const { mozInputSource, MOZ_SOURCE_KEYBOARD } = e.nativeEvent;
-    if (e.isTrusted && mozInputSource === MOZ_SOURCE_KEYBOARD) {
-      // Already handled by key down handler on user input.
-      return;
-    }
-
-    this.toggleFilter();
-  }
-
-  onKeyDown(e) {
-    // We explicitely handle "click" and "keydown" events this way here because
-    // there seem to be a difference in the sequence of keyboard/click events
-    // fired when Space/Enter is pressed. When Space is pressed the sequence of
-    // events is keydown->keyup->click but when Enter is pressed the sequence is
-    // keydown->click->keyup. This results in an unwanted badge click (when
-    // pressing Space) within the accessibility tree row when activating it
-    // because it gets focused before the click event is dispatched.
-    if (![" ", "Enter"].includes(e.key)) {
-      return;
-    }
-
-    preventDefaultAndStopPropagation(e);
-    this.toggleFilter();
-  }
-
   render() {
-    const { active, label, tooltip } = this.props;
+    const { label, tooltip } = this.props;
 
-    return ToggleButton({
+    return span({
       className: "audit-badge badge",
-      label,
-      active,
-      tooltip,
-      onClick: this.onClick,
-      onKeyDown: this.onKeyDown,
-    });
+      title: tooltip,
+      "aria-label": label,
+    },
+      label);
   }
 }
 
-const mapStateToProps = ({ audit: { filters } }, { filterKey }) => ({
-  active: filters[filterKey],
-});
-
-module.exports = connect(mapStateToProps)(Badge);
+module.exports = Badge;

devtools/client/accessibility/components/Badges.js

@@ -27,12 +27,11 @@ class Badges extends Component {
   static get propTypes() {
     return {
       checks: PropTypes.object,
-      walker: PropTypes.object.isRequired,
     };
   }
 
   render() {
-    const { checks, walker } = this.props;
+    const { checks } = this.props;
     if (!checks) {
       return null;
     }
@@ -41,10 +40,7 @@ class Badges extends Component {
     for (const type in checks) {
       const component = getComponentForAuditType(type);
       if (checks[type] && component) {
-        items.push(component({
-          ...checks[type],
-          walker,
-        }));
+        items.push(component(checks[type]));
       }
     }
 

devtools/client/accessibility/components/ContrastBadge.js

@@ -13,25 +13,21 @@ const { accessibility: { SCORES } } = require("devtools/shared/constants");
 
 loader.lazyGetter(this, "Badge", () => createFactory(require("./Badge")));
 
-const { FILTERS } = require("../constants");
-
 /**
 * Component for rendering a badge for contrast accessibliity check
 * failures association with a given accessibility object in the accessibility
 * tree.
 */
-
 class ContrastBadge extends Component {
   static get propTypes() {
     return {
       error: PropTypes.string,
       score: PropTypes.string,
-      walker: PropTypes.object.isRequired,
     };
   }
 
   render() {
-    const { error, score, walker } = this.props;
+    const { error, score } = this.props;
     if (error) {
       return null;
     }
@@ -43,8 +39,6 @@ class ContrastBadge extends Component {
     return Badge({
       label: L10N.getStr("accessibility.badge.contrast"),
       tooltip: L10N.getStr("accessibility.badge.contrast.tooltip"),
-      filterKey: FILTERS.CONTRAST,
-      walker,
     });
   }
 }

devtools/client/accessibility/test/browser/browser_accessibility_tree_audit.js

@@ -3,7 +3,7 @@
 
 "use strict";
 
-/* global toggleRow, toggleBadge */
+/* global toggleRow, toggleFilter */
 
 const TEST_URI = `<html>
   <head>
@@ -52,9 +52,9 @@ const tests = [{
     }],
   },
 }, {
-  desc: "Click on the badge.",
+  desc: "Click on the contrast filter.",
   setup: async ({ doc }) => {
-    await toggleBadge(doc, 2, 0);
+    await toggleFilter(doc, 0);
   },
   expected: {
     tree: [{
@@ -69,9 +69,9 @@ const tests = [{
     }],
   },
 }, {
-  desc: "Click on the badge again.",
+  desc: "Click on the contrast filter again.",
   setup: async ({ doc }) => {
-    await toggleBadge(doc, 0, 0);
+    await toggleFilter(doc, 0);
   },
   expected: {
     tree: [{
@@ -98,7 +98,8 @@ const tests = [{
 
 /**
  * Simple test that checks content of the Accessibility panel tree when one of
- * the tree rows has a "contrast" badge and auditing is activated via badge.
+ * the tree rows has a "contrast" badge and auditing is activated via toolbar
+ * filter.
  */
 addA11yPanelTestsTask(tests, TEST_URI,
-  "Test Accessibility panel tree with contrast badge audit activation.");
+  "Test Accessibility panel tree with contrast badge present.");

devtools/client/accessibility/test/browser/browser_accessibility_tree_audit_toolbar.js

@@ -3,7 +3,7 @@
 
 "use strict";
 
-/* global toggleRow, toggleBadge, toggleFilter */
+/* global toggleRow, toggleFilter */
 
 const TEST_URI = `<html>
   <head>
@@ -58,7 +58,7 @@ const tests = [{
 }, {
   desc: "Click on the filter again.",
   setup: async ({ doc }) => {
-    await toggleBadge(doc, 0, 0);
+    await toggleFilter(doc, 0);
   },
   expected: {
     tree: [{

devtools/client/accessibility/test/browser/head.js

@@ -6,7 +6,7 @@
 
 /* global waitUntilState, gBrowser */
 /* exported addTestTab, checkTreeState, checkSidebarState, checkAuditState, selectRow,
-            toggleRow, toggleBadge, toggleFilter, addA11yPanelTestsTask, reload,
+            toggleRow, toggleFilter, addA11yPanelTestsTask, reload,
             navigate */
 
 "use strict";
@@ -439,22 +439,6 @@ async function toggleRow(doc, rowNumber) {
     expected === twisty.classList.contains("open"), "Twisty updated.");
 }
 
-/**
- * Toggle an accessibility audit badge based on its index in the badges group.
- * @param  {document} doc        panel documnent.
- * @param  {Number}   badgeIndex index of the badge to be toggled.
- */
-async function toggleBadge(doc, rowNumber, badgeIndex) {
-  const win = doc.defaultView;
-  const row = doc.querySelectorAll(".treeRow")[rowNumber];
-  const badge = row.querySelectorAll(".audit-badge.badge")[badgeIndex];
-  const expected = !badge.classList.contains("checked");
-
-  EventUtils.synthesizeMouseAtCenter(badge, {}, win);
-  await BrowserTestUtils.waitForCondition(() =>
-    expected === badge.classList.contains("checked"), "Badge updated.");
-}
-
 /**
  * Toggle an accessibility audit filter based on its index in the toolbar.
  * @param  {document} doc         panel documnent.

devtools/client/accessibility/test/jest/components/__snapshots__/badge.test.js.snap

@@ -1,7 +1,3 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`Badge component: basic render active 1`] = `"<button aria-pressed=\\"true\\" class=\\"audit-badge badge toggle-button checked\\">Contrast</button>"`;
-
-exports[`Badge component: basic render inactive 1`] = `"<button aria-pressed=\\"false\\" class=\\"audit-badge badge toggle-button\\">Contrast</button>"`;
-
-exports[`Badge component: toggle filter 1`] = `"<button aria-pressed=\\"false\\" class=\\"audit-badge badge toggle-button\\">Contrast</button>"`;
+exports[`Badge component: basic render 1`] = `"<span class=\\"audit-badge badge\\" title=\\"Does not meet WCAG standards for accessible text.\\" aria-label=\\"Contrast\\">Contrast</span>"`;

devtools/client/accessibility/test/jest/components/__snapshots__/badges.test.js.snap

@@ -1,8 +1,8 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`Badges component: contrast ratio fail range render 1`] = `"<span class=\\"badges\\" role=\\"group\\" aria-label=\\"accessibility.badges\\"><button aria-pressed=\\"false\\" class=\\"audit-badge badge toggle-button\\" title=\\"accessibility.badge.contrast.tooltip\\">accessibility.badge.contrast</button></span>"`;
+exports[`Badges component: contrast ratio fail range render 1`] = `"<span class=\\"badges\\" role=\\"group\\" aria-label=\\"accessibility.badges\\"><span class=\\"audit-badge badge\\" title=\\"accessibility.badge.contrast.tooltip\\" aria-label=\\"accessibility.badge.contrast\\">accessibility.badge.contrast</span></span>"`;
 
-exports[`Badges component: contrast ratio fail render 1`] = `"<span class=\\"badges\\" role=\\"group\\" aria-label=\\"accessibility.badges\\"><button aria-pressed=\\"false\\" class=\\"audit-badge badge toggle-button\\" title=\\"accessibility.badge.contrast.tooltip\\">accessibility.badge.contrast</button></span>"`;
+exports[`Badges component: contrast ratio fail render 1`] = `"<span class=\\"badges\\" role=\\"group\\" aria-label=\\"accessibility.badges\\"><span class=\\"audit-badge badge\\" title=\\"accessibility.badge.contrast.tooltip\\" aria-label=\\"accessibility.badge.contrast\\">accessibility.badge.contrast</span></span>"`;
 
 exports[`Badges component: contrast ratio success render 1`] = `"<span class=\\"badges\\" role=\\"group\\" aria-label=\\"accessibility.badges\\"></span>"`;
 

devtools/client/accessibility/test/jest/components/__snapshots__/contrast-badge.test.js.snap

@@ -2,7 +2,7 @@
 
 exports[`ContrastBadge component: error render 1`] = `null`;
 
-exports[`ContrastBadge component: fail render 1`] = `"<button aria-pressed=\\"false\\" class=\\"audit-badge badge toggle-button\\" title=\\"accessibility.badge.contrast.tooltip\\">accessibility.badge.contrast</button>"`;
+exports[`ContrastBadge component: fail render 1`] = `"<span class=\\"audit-badge badge\\" title=\\"accessibility.badge.contrast.tooltip\\" aria-label=\\"accessibility.badge.contrast\\">accessibility.badge.contrast</span>"`;
 
 exports[`ContrastBadge component: success large text render 1`] = `null`;
 

devtools/client/accessibility/test/jest/components/badge.test.js

@@ -9,68 +9,26 @@ const { createFactory } = require("devtools/client/shared/vendor/react");
 const Provider = createFactory(require("devtools/client/shared/vendor/react-redux").Provider);
 const { setupStore } = require("devtools/client/accessibility/test/jest/helpers");
 
-const { ToggleButton } = require("devtools/client/accessibility/components/Button");
-const ConnectedBadgeClass = require("devtools/client/accessibility/components/Badge");
-const BadgeClass = ConnectedBadgeClass.WrappedComponent;
-const Badge = createFactory(ConnectedBadgeClass);
-
-const { FILTERS } = require("devtools/client/accessibility/constants");
+const BadgeClass = require("devtools/client/accessibility/components/Badge");
+const Badge = createFactory(BadgeClass);
 
 describe("Badge component:", () => {
-  const props = {
-    label: "Contrast",
-    filterKey: FILTERS.CONTRAST,
-  };
+  const label = "Contrast";
+  const tooltip = "Does not meet WCAG standards for accessible text.";
+  const props = { label, tooltip };
 
-  it("basic render inactive", () => {
+  it("basic render", () => {
     const store = setupStore();
     const wrapper = mount(Provider({ store }, Badge(props)));
     expect(wrapper.html()).toMatchSnapshot();
 
     const badge = wrapper.find(BadgeClass);
-    expect(badge.prop("active")).toBe(false);
     expect(badge.children().length).toBe(1);
+    expect(badge.find(`span[aria-label="${label}"][title="${tooltip}"]`)).toHaveLength(1);
 
-    const toggleButton = badge.childAt(0);
-    expect(toggleButton.type()).toBe(ToggleButton);
-    expect(toggleButton.children().length).toBe(1);
-
-    const button = toggleButton.childAt(0);
-    expect(button.is("button")).toBe(true);
-    expect(button.hasClass("audit-badge")).toBe(true);
-    expect(button.hasClass("badge")).toBe(true);
-    expect(button.hasClass("toggle-button")).toBe(true);
-    expect(button.prop("aria-pressed")).toBe(false);
-    expect(button.text()).toBe("Contrast");
-  });
-
-  it("basic render active", () => {
-    const store = setupStore({
-      preloadedState: { audit: { filters: { [FILTERS.CONTRAST]: true }}},
-    });
-    const wrapper = mount(Provider({ store }, Badge(props)));
-    expect(wrapper.html()).toMatchSnapshot();
-    const badge = wrapper.find(BadgeClass);
-    expect(badge.prop("active")).toBe(true);
-
-    const button = wrapper.find("button");
-    expect(button.prop("aria-pressed")).toBe(true);
-  });
-
-  it("toggle filter", () => {
-    const store = setupStore();
-    const wrapper = mount(Provider({ store }, Badge(props)));
-    expect(wrapper.html()).toMatchSnapshot();
-
-    const badgeInstance = wrapper.find(BadgeClass).instance();
-    badgeInstance.toggleFilter = jest.fn();
-    wrapper.find("button.audit-badge.badge").simulate("keydown", { key: " " });
-    expect(badgeInstance.toggleFilter.mock.calls.length).toBe(1);
-
-    wrapper.find("button.audit-badge.badge").simulate("keydown", { key: "Enter" });
-    expect(badgeInstance.toggleFilter.mock.calls.length).toBe(2);
-
-    wrapper.find("button.audit-badge.badge").simulate("click");
-    expect(badgeInstance.toggleFilter.mock.calls.length).toBe(3);
+    const badgeText = badge.childAt(0);
+    expect(badgeText.hasClass("audit-badge")).toBe(true);
+    expect(badgeText.hasClass("badge")).toBe(true);
+    expect(badgeText.text()).toBe(label);
   });
 });

devtools/client/accessibility/test/jest/components/contrast-badge.test.js

@@ -14,7 +14,6 @@ const { setupStore } = require("devtools/client/accessibility/test/jest/helpers"
 const Badge = require("devtools/client/accessibility/components/Badge");
 const ContrastBadgeClass = require("devtools/client/accessibility/components/ContrastBadge");
 const ContrastBadge = createFactory(ContrastBadgeClass);
-const { FILTERS } = require("devtools/client/accessibility/constants");
 
 describe("ContrastBadge component:", () => {
   const store = setupStore();
@@ -70,7 +69,7 @@ describe("ContrastBadge component:", () => {
     expect(badge.type()).toBe(Badge);
     expect(badge.props()).toMatchObject({
       label: "accessibility.badge.contrast",
-      filterKey: FILTERS.CONTRAST,
+      tooltip: "accessibility.badge.contrast.tooltip",
     });
   });
 });