From 6a6a62f13fd7846ba032543635580980b74ea14c Mon Sep 17 00:00:00 2001
From: Sorin Davidoi <sorin.davidoi@gmail.com>
Date: Wed, 26 Jul 2017 13:46:53 +0200
Subject: [PATCH] Improve accessibility (part 2) (#4377)

* fix(column_header): Invalid ARIA role

* fix(column): Remove hidden nodes from the DOM

* refactor(column_link): Remove unused property hideOnMobile

* fix(column_header): Use aria-pressed

* fix(column_header): Make collapsed content not focusable, add focusable property

* fix(column_loading): Make header non-focusable

* fix(column_settings): Use role to group the toggles
---
 .../mastodon/components/column_header.js      | 13 +++--
 .../components/column_settings.js             | 58 +++++++++++--------
 .../mastodon/features/ui/components/column.js | 12 ++--
 .../features/ui/components/column_header.js   |  5 +-
 .../features/ui/components/column_link.js     |  6 +-
 .../features/ui/components/column_loading.js  |  2 +-
 app/javascript/styles/components.scss         | 12 ----
 7 files changed, 54 insertions(+), 54 deletions(-)

diff --git a/app/javascript/mastodon/components/column_header.js b/app/javascript/mastodon/components/column_header.js
index 89e2f9189..d3256fbec 100644
--- a/app/javascript/mastodon/components/column_header.js
+++ b/app/javascript/mastodon/components/column_header.js
@@ -21,6 +21,7 @@ export default class ColumnHeader extends React.PureComponent {
     icon: PropTypes.string.isRequired,
     active: PropTypes.bool,
     multiColumn: PropTypes.bool,
+    focusable: PropTypes.bool,
     showBackButton: PropTypes.bool,
     children: PropTypes.node,
     pinned: PropTypes.bool,
@@ -29,6 +30,10 @@ export default class ColumnHeader extends React.PureComponent {
     onClick: PropTypes.func,
   };
 
+  static defaultProps = {
+    focusable: true,
+  }
+
   state = {
     collapsed: true,
     animating: false,
@@ -61,7 +66,7 @@ export default class ColumnHeader extends React.PureComponent {
   }
 
   render () {
-    const { title, icon, active, children, pinned, onPin, multiColumn, showBackButton, intl: { formatMessage } } = this.props;
+    const { title, icon, active, children, pinned, onPin, multiColumn, focusable, showBackButton, intl: { formatMessage } } = this.props;
     const { collapsed, animating } = this.state;
 
     const wrapperClassName = classNames('column-header__wrapper', {
@@ -123,12 +128,12 @@ export default class ColumnHeader extends React.PureComponent {
     }
 
     if (children || multiColumn) {
-      collapseButton = <button className={collapsibleButtonClassName} aria-label={formatMessage(collapsed ? messages.show : messages.hide)} onClick={this.handleToggleClick}><i className='fa fa-sliders' /></button>;
+      collapseButton = <button className={collapsibleButtonClassName} aria-label={formatMessage(collapsed ? messages.show : messages.hide)} aria-pressed={collapsed ? 'false' : 'true'} onClick={this.handleToggleClick}><i className='fa fa-sliders' /></button>;
     }
 
     return (
       <div className={wrapperClassName}>
-        <div role='button heading' tabIndex='0' className={buttonClassName} onClick={this.handleTitleClick}>
+        <div role='heading' tabIndex={focusable && '0'} className={buttonClassName} aria-label={title} onClick={this.handleTitleClick}>
           <i className={`fa fa-fw fa-${icon} column-header__icon`} />
           {title}
 
@@ -138,7 +143,7 @@ export default class ColumnHeader extends React.PureComponent {
           </div>
         </div>
 
-        <div className={collapsibleClassName} onTransitionEnd={this.handleTransitionEnd}>
+        <div className={collapsibleClassName} tabIndex={collapsed && -1} onTransitionEnd={this.handleTransitionEnd}>
           <div className='column-header__collapsible-inner'>
             {(!collapsed || animating) && collapsedContent}
           </div>
diff --git a/app/javascript/mastodon/features/notifications/components/column_settings.js b/app/javascript/mastodon/features/notifications/components/column_settings.js
index 31cac5bc7..88a29d4d3 100644
--- a/app/javascript/mastodon/features/notifications/components/column_settings.js
+++ b/app/javascript/mastodon/features/notifications/components/column_settings.js
@@ -36,40 +36,48 @@ export default class ColumnSettings extends React.PureComponent {
           <ClearColumnButton onClick={onClear} />
         </div>
 
-        <span className='column-settings__section'><FormattedMessage id='notifications.column_settings.follow' defaultMessage='New followers:' /></span>
-
-        <div className='column-settings__row'>
-          <SettingToggle prefix='notifications_desktop' settings={settings} settingKey={['alerts', 'follow']} onChange={onChange} label={alertStr} />
-          {showPushSettings && <SettingToggle prefix='notifications_push' settings={pushSettings} settingKey={['alerts', 'follow']} meta={pushMeta} onChange={this.onPushChange} label={pushStr} />}
-          <SettingToggle prefix='notifications' settings={settings} settingKey={['shows', 'follow']} onChange={onChange} label={showStr} />
-          <SettingToggle prefix='notifications' settings={settings} settingKey={['sounds', 'follow']} onChange={onChange} label={soundStr} />
+        <div role='group' aria-labelledby='notifications-follow'>
+          <span id='notifications-follow' className='column-settings__section'><FormattedMessage id='notifications.column_settings.follow' defaultMessage='New followers:' /></span>
+
+          <div className='column-settings__row'>
+            <SettingToggle prefix='notifications_desktop' settings={settings} settingKey={['alerts', 'follow']} onChange={onChange} label={alertStr} />
+            {showPushSettings && <SettingToggle prefix='notifications_push' settings={pushSettings} settingKey={['alerts', 'follow']} meta={pushMeta} onChange={this.onPushChange} label={pushStr} />}
+            <SettingToggle prefix='notifications' settings={settings} settingKey={['shows', 'follow']} onChange={onChange} label={showStr} />
+            <SettingToggle prefix='notifications' settings={settings} settingKey={['sounds', 'follow']} onChange={onChange} label={soundStr} />
+          </div>
         </div>
 
-        <span className='column-settings__section'><FormattedMessage id='notifications.column_settings.favourite' defaultMessage='Favourites:' /></span>
+        <div role='group' aria-labelledby='notifications-favourite'>
+          <span id='notifications-favourite' className='column-settings__section'><FormattedMessage id='notifications.column_settings.favourite' defaultMessage='Favourites:' /></span>
 
-        <div className='column-settings__row'>
-          <SettingToggle prefix='notifications_desktop' settings={settings} settingKey={['alerts', 'favourite']} onChange={onChange} label={alertStr} />
-          {showPushSettings && <SettingToggle prefix='notifications_push' settings={pushSettings} settingKey={['alerts', 'favourite']} meta={pushMeta} onChange={this.onPushChange} label={pushStr} />}
-          <SettingToggle prefix='notifications' settings={settings} settingKey={['shows', 'favourite']} onChange={onChange} label={showStr} />
-          <SettingToggle prefix='notifications' settings={settings} settingKey={['sounds', 'favourite']} onChange={onChange} label={soundStr} />
+          <div className='column-settings__row'>
+            <SettingToggle prefix='notifications_desktop' settings={settings} settingKey={['alerts', 'favourite']} onChange={onChange} label={alertStr} />
+            {showPushSettings && <SettingToggle prefix='notifications_push' settings={pushSettings} settingKey={['alerts', 'favourite']} meta={pushMeta} onChange={this.onPushChange} label={pushStr} />}
+            <SettingToggle prefix='notifications' settings={settings} settingKey={['shows', 'favourite']} onChange={onChange} label={showStr} />
+            <SettingToggle prefix='notifications' settings={settings} settingKey={['sounds', 'favourite']} onChange={onChange} label={soundStr} />
+          </div>
         </div>
 
-        <span className='column-settings__section'><FormattedMessage id='notifications.column_settings.mention' defaultMessage='Mentions:' /></span>
+        <div role='group' aria-labelledby='notifications-mention'>
+          <span id='notifications-mention' className='column-settings__section'><FormattedMessage id='notifications.column_settings.mention' defaultMessage='Mentions:' /></span>
 
-        <div className='column-settings__row'>
-          <SettingToggle prefix='notifications_desktop' settings={settings} settingKey={['alerts', 'mention']} onChange={onChange} label={alertStr} />
-          {showPushSettings && <SettingToggle prefix='notifications_push' settings={pushSettings} settingKey={['alerts', 'mention']} meta={pushMeta} onChange={this.onPushChange} label={pushStr} />}
-          <SettingToggle prefix='notifications' settings={settings} settingKey={['shows', 'mention']} onChange={onChange} label={showStr} />
-          <SettingToggle prefix='notifications' settings={settings} settingKey={['sounds', 'mention']} onChange={onChange} label={soundStr} />
+          <div className='column-settings__row'>
+            <SettingToggle prefix='notifications_desktop' settings={settings} settingKey={['alerts', 'mention']} onChange={onChange} label={alertStr} />
+            {showPushSettings && <SettingToggle prefix='notifications_push' settings={pushSettings} settingKey={['alerts', 'mention']} meta={pushMeta} onChange={this.onPushChange} label={pushStr} />}
+            <SettingToggle prefix='notifications' settings={settings} settingKey={['shows', 'mention']} onChange={onChange} label={showStr} />
+            <SettingToggle prefix='notifications' settings={settings} settingKey={['sounds', 'mention']} onChange={onChange} label={soundStr} />
+          </div>
         </div>
 
-        <span className='column-settings__section'><FormattedMessage id='notifications.column_settings.reblog' defaultMessage='Boosts:' /></span>
+        <div role='group' aria-labelledby='notifications-reblog'>
+          <span id='notifications-reblog' className='column-settings__section'><FormattedMessage id='notifications.column_settings.reblog' defaultMessage='Boosts:' /></span>
 
-        <div className='column-settings__row'>
-          <SettingToggle prefix='notifications_desktop' settings={settings} settingKey={['alerts', 'reblog']} onChange={onChange} label={alertStr} />
-          {showPushSettings && <SettingToggle prefix='notifications_push' settings={pushSettings} settingKey={['alerts', 'reblog']} meta={pushMeta} onChange={this.onPushChange} label={pushStr} />}
-          <SettingToggle prefix='notifications' settings={settings} settingKey={['shows', 'reblog']} onChange={onChange} label={showStr} />
-          <SettingToggle prefix='notifications' settings={settings} settingKey={['sounds', 'reblog']} onChange={onChange} label={soundStr} />
+          <div className='column-settings__row'>
+            <SettingToggle prefix='notifications_desktop' settings={settings} settingKey={['alerts', 'reblog']} onChange={onChange} label={alertStr} />
+            {showPushSettings && <SettingToggle prefix='notifications_push' settings={pushSettings} settingKey={['alerts', 'reblog']} meta={pushMeta} onChange={this.onPushChange} label={pushStr} />}
+            <SettingToggle prefix='notifications' settings={settings} settingKey={['shows', 'reblog']} onChange={onChange} label={showStr} />
+            <SettingToggle prefix='notifications' settings={settings} settingKey={['sounds', 'reblog']} onChange={onChange} label={soundStr} />
+          </div>
         </div>
       </div>
     );
diff --git a/app/javascript/mastodon/features/ui/components/column.js b/app/javascript/mastodon/features/ui/components/column.js
index ce1dca171..2d46264a2 100644
--- a/app/javascript/mastodon/features/ui/components/column.js
+++ b/app/javascript/mastodon/features/ui/components/column.js
@@ -3,6 +3,7 @@ import ColumnHeader from './column_header';
 import PropTypes from 'prop-types';
 import { debounce } from 'lodash';
 import scrollTop from '../../../scroll';
+import { isMobile } from '../../../is_mobile';
 
 export default class Column extends React.PureComponent {
 
@@ -37,13 +38,12 @@ export default class Column extends React.PureComponent {
   render () {
     const { heading, icon, children, active, hideHeadingOnMobile } = this.props;
 
-    let columnHeaderId = null;
-    let header = '';
+    const showHeading = !hideHeadingOnMobile || (hideHeadingOnMobile && !isMobile(window.innerWidth));
 
-    if (heading) {
-      columnHeaderId = heading.replace(/ /g, '-');
-      header = <ColumnHeader icon={icon} active={active} type={heading} onClick={this.handleHeaderClick} hideOnMobile={hideHeadingOnMobile} columnHeaderId={columnHeaderId} />;
-    }
+    const columnHeaderId = showHeading && heading.replace(/ /g, '-');
+    const header = showHeading && (
+      <ColumnHeader icon={icon} active={active} type={heading} onClick={this.handleHeaderClick} columnHeaderId={columnHeaderId} />
+    );
     return (
       <div
         ref={this.setRef}
diff --git a/app/javascript/mastodon/features/ui/components/column_header.js b/app/javascript/mastodon/features/ui/components/column_header.js
index dc601d6e1..af195ea9c 100644
--- a/app/javascript/mastodon/features/ui/components/column_header.js
+++ b/app/javascript/mastodon/features/ui/components/column_header.js
@@ -8,7 +8,6 @@ export default class ColumnHeader extends React.PureComponent {
     type: PropTypes.string,
     active: PropTypes.bool,
     onClick: PropTypes.func,
-    hideOnMobile: PropTypes.bool,
     columnHeaderId: PropTypes.string,
   };
 
@@ -17,7 +16,7 @@ export default class ColumnHeader extends React.PureComponent {
   }
 
   render () {
-    const { type, active, hideOnMobile, columnHeaderId } = this.props;
+    const { type, active, columnHeaderId } = this.props;
 
     let icon = '';
 
@@ -26,7 +25,7 @@ export default class ColumnHeader extends React.PureComponent {
     }
 
     return (
-      <div role='button heading' tabIndex='0' className={`column-header ${active ? 'active' : ''} ${hideOnMobile ? 'hidden-on-mobile' : ''}`} onClick={this.handleClick} id={columnHeaderId || null}>
+      <div role='heading' tabIndex='0' className={`column-header ${active ? 'active' : ''}`} onClick={this.handleClick} id={columnHeaderId || null}>
         {icon}
         {type}
       </div>
diff --git a/app/javascript/mastodon/features/ui/components/column_link.js b/app/javascript/mastodon/features/ui/components/column_link.js
index cbdb6534f..ad7ec9318 100644
--- a/app/javascript/mastodon/features/ui/components/column_link.js
+++ b/app/javascript/mastodon/features/ui/components/column_link.js
@@ -2,17 +2,17 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import Link from 'react-router-dom/Link';
 
-const ColumnLink = ({ icon, text, to, href, method, hideOnMobile }) => {
+const ColumnLink = ({ icon, text, to, href, method }) => {
   if (href) {
     return (
-      <a href={href} className={`column-link ${hideOnMobile ? 'hidden-on-mobile' : ''}`} data-method={method}>
+      <a href={href} className='column-link' data-method={method}>
         <i className={`fa fa-fw fa-${icon} column-link__icon`} />
         {text}
       </a>
     );
   } else {
     return (
-      <Link to={to} className={`column-link ${hideOnMobile ? 'hidden-on-mobile' : ''}`}>
+      <Link to={to} className='column-link'>
         <i className={`fa fa-fw fa-${icon} column-link__icon`} />
         {text}
       </Link>
diff --git a/app/javascript/mastodon/features/ui/components/column_loading.js b/app/javascript/mastodon/features/ui/components/column_loading.js
index 7ecfaf77a..1c4058926 100644
--- a/app/javascript/mastodon/features/ui/components/column_loading.js
+++ b/app/javascript/mastodon/features/ui/components/column_loading.js
@@ -6,7 +6,7 @@ import ColumnHeader from '../../../components/column_header';
 
 const ColumnLoading = ({ title = '', icon = ' ' }) => (
   <Column>
-    <ColumnHeader icon={icon} title={title} multiColumn={false} />
+    <ColumnHeader icon={icon} title={title} multiColumn={false} focusable={false} />
     <div className='scrollable' />
   </Column>
 );
diff --git a/app/javascript/styles/components.scss b/app/javascript/styles/components.scss
index 8de456754..a51cd962e 100644
--- a/app/javascript/styles/components.scss
+++ b/app/javascript/styles/components.scss
@@ -1743,12 +1743,6 @@
   &:hover {
     background: lighten($ui-base-color, 11%);
   }
-
-  &.hidden-on-mobile {
-    @media screen and (max-width: 1024px) {
-      display: none;
-    }
-  }
 }
 
 .column-link__icon {
@@ -2132,12 +2126,6 @@ button.icon-button.active i.fa-retweet {
     }
   }
 
-  &.hidden-on-mobile {
-    @media screen and (max-width: 1024px) {
-      display: none;
-    }
-  }
-
   &:focus,
   &:active {
     outline: 0;