diff --git a/devtools/client/accessibility/accessibility.css b/devtools/client/accessibility/accessibility.css index a27068c56431..7cfbf87e801b 100644 --- a/devtools/client/accessibility/accessibility.css +++ b/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 diff --git a/devtools/client/accessibility/components/AccessibilityRowValue.js b/devtools/client/accessibility/components/AccessibilityRowValue.js index b7ea18e7cd2e..f5f12a0db4c2 100644 --- a/devtools/client/accessibility/components/AccessibilityRowValue.js +++ b/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()), ); } } diff --git a/devtools/client/accessibility/components/AccessibilityTree.js b/devtools/client/accessibility/components/AccessibilityTree.js index 3587af4cc3b8..21ac0074bc0a 100644 --- a/devtools/client/accessibility/components/AccessibilityTree.js +++ b/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); } /** diff --git a/devtools/client/accessibility/components/Badge.js b/devtools/client/accessibility/components/Badge.js index ffd01da857ec..9fc2faec2dc4 100644 --- a/devtools/client/accessibility/components/Badge.js +++ b/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; diff --git a/devtools/client/accessibility/components/Badges.js b/devtools/client/accessibility/components/Badges.js index a2e60739b745..43c84fdbd3a8 100644 --- a/devtools/client/accessibility/components/Badges.js +++ b/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])); } } diff --git a/devtools/client/accessibility/components/ContrastBadge.js b/devtools/client/accessibility/components/ContrastBadge.js index ee695dcecb9b..70671a1ce609 100644 --- a/devtools/client/accessibility/components/ContrastBadge.js +++ b/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, }); } } diff --git a/devtools/client/accessibility/test/browser/browser_accessibility_tree_audit.js b/devtools/client/accessibility/test/browser/browser_accessibility_tree_audit.js index 6124f7aeeb8e..798680939f8e 100644 --- a/devtools/client/accessibility/test/browser/browser_accessibility_tree_audit.js +++ b/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 = ` @@ -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."); diff --git a/devtools/client/accessibility/test/browser/browser_accessibility_tree_audit_toolbar.js b/devtools/client/accessibility/test/browser/browser_accessibility_tree_audit_toolbar.js index 7e4b7961f430..dfc1093ccd36 100644 --- a/devtools/client/accessibility/test/browser/browser_accessibility_tree_audit_toolbar.js +++ b/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 = ` @@ -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: [{ diff --git a/devtools/client/accessibility/test/browser/head.js b/devtools/client/accessibility/test/browser/head.js index 74bceefbe9b1..1664f7b1db7e 100644 --- a/devtools/client/accessibility/test/browser/head.js +++ b/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. diff --git a/devtools/client/accessibility/test/jest/components/__snapshots__/badge.test.js.snap b/devtools/client/accessibility/test/jest/components/__snapshots__/badge.test.js.snap index fe463201d20f..324c72105bdc 100644 --- a/devtools/client/accessibility/test/jest/components/__snapshots__/badge.test.js.snap +++ b/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`] = `""`; - -exports[`Badge component: basic render inactive 1`] = `""`; - -exports[`Badge component: toggle filter 1`] = `""`; +exports[`Badge component: basic render 1`] = `"Contrast"`; diff --git a/devtools/client/accessibility/test/jest/components/__snapshots__/badges.test.js.snap b/devtools/client/accessibility/test/jest/components/__snapshots__/badges.test.js.snap index 6f6e175bfd37..1568889196f6 100644 --- a/devtools/client/accessibility/test/jest/components/__snapshots__/badges.test.js.snap +++ b/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`] = `""`; +exports[`Badges component: contrast ratio fail range render 1`] = `"accessibility.badge.contrast"`; -exports[`Badges component: contrast ratio fail render 1`] = `""`; +exports[`Badges component: contrast ratio fail render 1`] = `"accessibility.badge.contrast"`; exports[`Badges component: contrast ratio success render 1`] = `""`; diff --git a/devtools/client/accessibility/test/jest/components/__snapshots__/contrast-badge.test.js.snap b/devtools/client/accessibility/test/jest/components/__snapshots__/contrast-badge.test.js.snap index 7db6eb527383..20dfc6e7cf9d 100644 --- a/devtools/client/accessibility/test/jest/components/__snapshots__/contrast-badge.test.js.snap +++ b/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`] = `""`; +exports[`ContrastBadge component: fail render 1`] = `"accessibility.badge.contrast"`; exports[`ContrastBadge component: success large text render 1`] = `null`; diff --git a/devtools/client/accessibility/test/jest/components/badge.test.js b/devtools/client/accessibility/test/jest/components/badge.test.js index 1fc9babd4fc3..ab9a1df319a5 100644 --- a/devtools/client/accessibility/test/jest/components/badge.test.js +++ b/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); }); }); diff --git a/devtools/client/accessibility/test/jest/components/contrast-badge.test.js b/devtools/client/accessibility/test/jest/components/contrast-badge.test.js index 711f6eb47a0f..b65ab5723cc1 100644 --- a/devtools/client/accessibility/test/jest/components/contrast-badge.test.js +++ b/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", }); }); });