Merge pull request #1806 from hypothesis/fix-button-pressed-state

Do not set "aria-pressed" attribute for non-toggle buttons

src/sidebar/components/annotation-action-bar.js

@@ -96,7 +96,7 @@ function AnnotationActionBar({
       )}
       {showFlagAction && annotation.flagged && (
         <Button
-          isActive={true}
+          isPressed={true}
           icon="flag--active"
           title="Annotation has been reported to the moderators"
         />

src/sidebar/components/button.js

@@ -21,7 +21,7 @@ export default function Button({
   className = '',
   disabled = false,
   icon = '',
-  isActive = false,
+  isPressed,
   onClick = () => null,
   style = {},
   title,
@@ -35,6 +35,12 @@ export default function Button({
 
   const baseClassName = buttonText ? 'button--labeled' : 'button--icon-only';
 
+  const extraProps = {};
+  if (typeof isPressed === 'boolean') {
+    // Indicate that this is a toggle button.
+    extraProps['aria-pressed'] = isPressed;
+  }
+
   return (
     <button
       className={classnames(
@@ -44,15 +50,15 @@ export default function Button({
           'button--compact': useCompactStyle,
           'button--input': useInputStyle,
           'button--primary': usePrimaryStyle,
-          'is-active': isActive,
+          'is-active': isPressed,
         },
         className
       )}
       onClick={onClick}
-      aria-pressed={isActive}
       title={title}
       style={style}
       disabled={disabled}
+      {...extraProps}
     >
       {icon && <SvgIcon name={icon} className="button__icon" />}
       {buttonText}
@@ -104,8 +110,13 @@ Button.propTypes = {
    */
   icon: requiredStringIfButtonTextMissing,
 
-  /** Is this button currently in an "active"/"on" state? */
-  isActive: propTypes.bool,
+  /**
+   * Indicate that this is a toggle button (if `isPressed` is a boolean) and
+   * whether it is pressed.
+   *
+   * If omitted, the button is a non-toggle button.
+   */
+  isPressed: propTypes.bool,
 
   /** callback for button clicks */
   onClick: propTypes.func,

src/sidebar/components/test/button-test.js

@@ -14,7 +14,6 @@ describe('Button', () => {
     return mount(
       <Button
         icon="fakeIcon"
-        isActive={false}
         title="My Action"
         onClick={fakeOnClick}
         {...props}
@@ -31,8 +30,8 @@ describe('Button', () => {
     $imports.$restore();
   });
 
-  it('adds active className if `isActive` is `true`', () => {
-    const wrapper = createComponent({ isActive: true });
+  it('adds active className if `isPressed` is `true`', () => {
+    const wrapper = createComponent({ isPressed: true });
 
     assert.isTrue(wrapper.find('button').hasClass('is-active'));
   });
@@ -42,9 +41,16 @@ describe('Button', () => {
     assert.equal(wrapper.find('SvgIcon').prop('name'), 'fakeIcon');
   });
 
-  it('sets ARIA `aria-pressed` attribute if `isActive`', () => {
-    const wrapper = createComponent({ isActive: true });
-    assert.isTrue(wrapper.find('button').prop('aria-pressed'));
+  [true, false].forEach(isPressed => {
+    it('sets `aria-pressed` attribute if `isPressed` is a boolean', () => {
+      const wrapper = createComponent({ isPressed });
+      assert.equal(wrapper.find('button').prop('aria-pressed'), isPressed);
+    });
+  });
+
+  it('does not set `aria-pressed` attribute if `isPressed` is omitted', () => {
+    const wrapper = createComponent();
+    assert.notProperty(wrapper.find('button').props(), 'aria-pressed');
   });
 
   it('sets `title` to provided `title` prop', () => {

src/sidebar/components/test/top-bar-test.js

@@ -119,7 +119,7 @@ describe('TopBar', () => {
 
         wrapper.update();
 
-        assert.isTrue(helpButton.props().isActive);
+        assert.isTrue(helpButton.props().isPressed);
       });
 
       context('help service handler configured in services', () => {
@@ -228,7 +228,7 @@ describe('TopBar', () => {
     const wrapper = createTopBar();
     const shareButton = getButton(wrapper, 'share');
 
-    assert.isTrue(shareButton.prop('isActive'));
+    assert.isTrue(shareButton.prop('isPressed'));
   });
 
   it('displays search input in the sidebar', () => {

src/sidebar/components/top-bar.js

@@ -105,7 +105,7 @@ function TopBar({
           <Button
             className="top-bar__icon-button"
             icon="help"
-            isActive={currentActivePanel === uiConstants.PANEL_HELP}
+            isPressed={currentActivePanel === uiConstants.PANEL_HELP}
             onClick={requestHelp}
             title="Help"
             useCompactStyle
@@ -135,7 +135,7 @@ function TopBar({
             <Button
               className="top-bar__icon-button"
               icon="share"
-              isActive={
+              isPressed={
                 currentActivePanel === uiConstants.PANEL_SHARE_ANNOTATIONS
               }
               onClick={toggleSharePanel}
@@ -146,7 +146,7 @@ function TopBar({
           <Button
             className="top-bar__icon-button"
             icon="help"
-            isActive={currentActivePanel === uiConstants.PANEL_HELP}
+            isPressed={currentActivePanel === uiConstants.PANEL_HELP}
             onClick={requestHelp}
             title="Help"
             useCompactStyle