Merge pull request #683 from orangemug/fix/a11y-issue-320

Fix some accessibility issues

src/components/Button.jsx

@@ -12,10 +12,14 @@ class Button extends React.Component {
     children: PropTypes.node,
     disabled: PropTypes.bool,
     type: PropTypes.string,
+    id: PropTypes.string,
+    title: PropTypes.string,
   }
 
   render() {
     return <button
+      id={this.props.id}
+      title={this.props.title}
       type={this.props.type}
       onClick={this.props.onClick}
       disabled={this.props.disabled}

src/components/Toolbar.jsx

@@ -131,6 +131,16 @@ export default class Toolbar extends React.Component {
     this.props.onSetMapState(val);
   }
 
+  onSkip = (target) => {
+    if (target === "map") {
+      document.querySelector(".mapboxgl-canvas").focus();
+    }
+    else {
+      const el = document.querySelector("#skip-target-"+target);
+      el.focus();
+    }
+  }
+
   render() {
     const views = [
       {
@@ -144,22 +154,22 @@ export default class Toolbar extends React.Component {
       },
       {
         id: "filter-deuteranopia",
-        title: "Map (deuteranopia)",
+        title: "Deuteranopia color filter",
         disabled: !colorAccessibilityFiltersEnabled,
       },
       {
         id: "filter-protanopia",
-        title: "Map (protanopia)",
+        title: "Protanopia color filter",
         disabled: !colorAccessibilityFiltersEnabled,
       },
       {
         id: "filter-tritanopia",
-        title: "Map (tritanopia)",
+        title: "Tritanopia color filter",
         disabled: !colorAccessibilityFiltersEnabled,
       },
       {
         id: "filter-achromatopsia",
-        title: "Map (achromatopsia)",
+        title: "Achromatopsia color filter",
         disabled: !colorAccessibilityFiltersEnabled,
       },
     ];
@@ -173,23 +183,37 @@ export default class Toolbar extends React.Component {
         <div
           className="maputnik-toolbar-logo-container"
         >
-          <a className="maputnik-toolbar-skip" href="#skip-menu">
-            Skip navigation
-          </a>
-          <a
-            href="https://github.com/maputnik/editor"
-            rel="noopener noreferrer"
-            target="_blank"
+          {/* Keyboard accessible quick links */}
+          <button
+            className="maputnik-toolbar-skip"
+            onClick={e => this.onSkip("layer-list")}
+          >
+            Layers list
+          </button>
+          <button
+            className="maputnik-toolbar-skip"
+            onClick={e => this.onSkip("layer-editor")}
+          >
+            Layer editor
+          </button>
+          <button
+            className="maputnik-toolbar-skip"
+            onClick={e => this.onSkip("map")}
+          >
+            Map view
+          </button>
+          <div
             className="maputnik-toolbar-logo"
+            tabIndex="-1"
           >
             <span dangerouslySetInnerHTML={{__html: logoImage}} />
             <h1>
               <span className="maputnik-toolbar-name">{pkgJson.name}</span>
               <span className="maputnik-toolbar-version">v{pkgJson.version}</span>
             </h1>
-          </a>
           </div>
-        <div className="maputnik-toolbar__actions">
+        </div>
+        <div className="maputnik-toolbar__actions" role="navigation" aria-label="Toolbar">
           <ToolbarAction wdKey="nav:open" onClick={this.props.onToggleModal.bind(this, 'open')}>
             <MdOpenInBrowser />
             <IconText>Open</IconText>
@@ -209,7 +233,7 @@ export default class Toolbar extends React.Component {
 
           <ToolbarSelect wdKey="nav:inspect">
             <MdFindInPage />
-            <IconText>View </IconText>
+            <label>View
               <select
                 className="maputnik-select"
                 onChange={(e) => this.handleSelection(e.target.value)}
@@ -223,6 +247,7 @@ export default class Toolbar extends React.Component {
                   );
                 })}
               </select>
+            </label>
           </ToolbarSelect>
 
           <ToolbarLink href={"https://github.com/maputnik/editor/wiki"}>

src/components/fields/_DeleteStopButton.jsx

@@ -14,7 +14,7 @@ export default class DeleteStopButton extends React.Component {
     return <Button
       className="maputnik-delete-stop"
       onClick={this.props.onClick}
-      title={"Remove zoom level stop."}
+      title={"Remove zoom level from stop"}
     >
       <MdDelete />
     </Button>

src/components/fields/_ExpressionProperty.jsx

@@ -64,6 +64,7 @@ export default class ExpressionProperty extends React.Component {
             onClick={this.props.onUndo}
             disabled={undoDisabled}
             className="maputnik-delete-stop"
+            title="Revert from expression"
           >
             <MdUndo />
           </Button>
@@ -72,6 +73,7 @@ export default class ExpressionProperty extends React.Component {
           key="delete_action"
           onClick={this.props.onDelete}
           className="maputnik-delete-stop"
+          title="Delete expression"
         >
           <MdDelete />
         </Button>

src/components/fields/_FunctionButtons.jsx

@@ -40,6 +40,7 @@ export default class FunctionButtons extends React.Component {
         <Button
           className="maputnik-make-zoom-function"
           onClick={this.props.onExpressionClick}
+          title="Convert to expression"
         >
           <svg style={{width:"14px", height:"14px", verticalAlign: "middle"}} viewBox="0 0 24 24">
             <path fill="currentColor" d={mdiFunctionVariant} />
@@ -50,7 +51,7 @@ export default class FunctionButtons extends React.Component {
       makeZoomButton = <Button
         className="maputnik-make-zoom-function"
         onClick={this.props.onZoomClick}
-        title={"Turn property into a zoom function to enable a map feature to change with map's zoom level."}
+        title="Convert property into a zoom function"
       >
         <MdFunctions />
       </Button>
@@ -59,7 +60,7 @@ export default class FunctionButtons extends React.Component {
         makeDataButton = <Button
           className="maputnik-make-data-function"
           onClick={this.props.onDataClick}
-          title={"Turn property into a data function to enable a map feature to change according to data properties and the map's zoom level."}
+          title="Convert property to data function"
         >
           <MdInsertChart />
         </Button>

src/components/filter/FilterEditor.jsx

@@ -194,6 +194,7 @@ export default class CombiningFilterEditor extends React.Component {
         </p>
         <Button
           onClick={this.makeExpression}
+          title="Convert to expression"
         >
           <svg style={{marginRight: "0.2em", width:"14px", height:"14px", verticalAlign: "middle"}} viewBox="0 0 24 24">
             <path fill="currentColor" d={mdiFunctionVariant} />
@@ -211,6 +212,7 @@ export default class CombiningFilterEditor extends React.Component {
         <div>
           <Button
             onClick={this.makeExpression}
+            title="Convert to expression"
             className="maputnik-make-zoom-function"
           >
             <svg style={{width:"14px", height:"14px", verticalAlign: "middle"}} viewBox="0 0 24 24">

src/components/filter/FilterEditorBlock.jsx

@@ -15,6 +15,7 @@ class FilterEditorBlock extends React.Component {
         <Button
           className="maputnik-delete-filter"
           onClick={this.props.onDelete}
+          title="Delete filter block"
         >
           <MdDelete />
         </Button>

src/components/inputs/DynamicArrayInput.jsx

@@ -124,10 +124,11 @@ class DeleteValueButton extends React.Component {
     return <Button
       className="maputnik-delete-stop"
       onClick={this.props.onClick}
+      title="Remove array item"
     >
       <DocLabel
         label={<MdDelete />}
-        doc={"Remove array entry."}
+        doc={"Remove array item."}
       />
     </Button>
   }

src/components/inputs/EnumInput.jsx

@@ -19,15 +19,17 @@ class EnumInput extends React.Component {
     value: PropTypes.string,
     style: PropTypes.object,
     default: PropTypes.string,
+    name: PropTypes.string,
     onChange: PropTypes.func,
     options: PropTypes.array,
   }
 
   render() {
-    const {options, value, onChange} = this.props;
+    const {options, value, onChange, name} = this.props;
 
     if(options.length <= 3 && optionsLabelLength(options) <= 20) {
       return <MultiButtonInput
+        name={name}
         options={options}
         value={value || this.props.default}
         onChange={onChange}

src/components/inputs/MultiButtonInput.jsx

@@ -5,6 +5,7 @@ import Button from '../Button'
 
 class MultiButtonInput extends React.Component {
   static propTypes = {
+    name: PropTypes.string.isRequired,
     value: PropTypes.string.isRequired,
     options: PropTypes.array.isRequired,
     onChange: PropTypes.func.isRequired,
@@ -17,19 +18,24 @@ class MultiButtonInput extends React.Component {
     }
 
     const selectedValue = this.props.value || options[0][0]
-    const buttons = options.map(([val, label])=> {
-      return <Button
+    const radios = options.map(([val, label])=> {
+      return <label
         key={val}
-        onClick={e => this.props.onChange(val)}
-        className={classnames({"maputnik-button-selected": val === selectedValue})}
+        className={classnames("maputnik-radio-as-button", {"maputnik-button-selected": val === selectedValue})}
       >
+        <input type="radio"
+          name={this.props.name}
+          onChange={e => this.props.onChange(val)}
+          value={val}
+          checked={val === selectedValue}
+        />
         {label}
-      </Button>
+      </label>
     })
 
-    return <div className="maputnik-multibutton">
-      {buttons}
-    </div>
+    return <fieldset className="maputnik-multibutton">
+      {radios}
+    </fieldset>
   }
 }
 

src/components/layers/LayerEditor.jsx

@@ -289,6 +289,8 @@ export default class LayerEditor extends React.Component {
     }
 
     return <div className="maputnik-layer-editor"
+      role="main"
+      aria-label="Layer editor"
     >
       <header>
         <div className="layer-header">
@@ -301,7 +303,7 @@ export default class LayerEditor extends React.Component {
               onSelection={handleSelection}
               closeOnSelection={false}
             >
-              <Button className='more-menu__button'>
+              <Button id="skip-target-layer-editor" className='more-menu__button' title="Layer options">
                 <MdMoreVert className="more-menu__button__svg" />
               </Button>
               <Menu>

src/components/layers/LayerList.jsx

@@ -100,9 +100,18 @@ class LayerListContainer extends React.Component {
 
   groupedLayers() {
     const groups = []
+    const layerIdCount = new Map();
+
     for (let i = 0; i < this.props.layers.length; i++) {
+      const origLayer = this.props.layers[i];
       const previousLayer = this.props.layers[i-1]
-      const layer = this.props.layers[i]
+      layerIdCount.set(origLayer.id,
+        layerIdCount.has(origLayer.id) ? layerIdCount.get(origLayer.id) + 1 : 0
+      );
+      const layer = {
+        ...origLayer,
+        key: `layers-list-${origLayer.id}-${layerIdCount.get(origLayer.id)}`,
+      }
       if(previousLayer && layerPrefix(previousLayer.id) == layerPrefix(layer.id)) {
         const lastGroup = groups[groups.length - 1]
         lastGroup.push(layer)
@@ -200,14 +209,13 @@ class LayerListContainer extends React.Component {
 
     const listItems = []
     let idx = 0
-    const layerIdCount = new Map();
-
     const layersByGroup = this.groupedLayers();
     layersByGroup.forEach(layers => {
       const groupPrefix = layerPrefix(layers[0].id)
       if(layers.length > 1) {
         const grp = <LayerListGroup
           data-wd-key={[groupPrefix, idx].join('-')}
+          aria-controls={layers.map(l => l.key).join(" ")}
           key={`group-${groupPrefix}-${idx}`}
           title={groupPrefix}
           isActive={!this.isCollapsed(groupPrefix, idx) || idx === this.props.selectedLayerIndex}
@@ -232,10 +240,6 @@ class LayerListContainer extends React.Component {
           additionalProps.ref = this.selectedItemRef;
         }
 
-        layerIdCount.set(layer.id,
-          layerIdCount.has(layer.id) ? layerIdCount.get(layer.id) + 1 : 0
-        );
-        const key = `${layer.id}-${layerIdCount.get(layer.id)}`;
         const listItem = <LayerListItem
           className={classnames({
             'maputnik-layer-list-item-collapsed': layers.length > 1 && this.isCollapsed(groupPrefix, groupIdx) && idx !== this.props.selectedLayerIndex,
@@ -243,7 +247,7 @@ class LayerListContainer extends React.Component {
             'maputnik-layer-list-item--error': !!layerError
           })}
           index={idx}
-          key={key}
+          key={layer.key}
           layerId={layer.id}
           layerIndex={idx}
           layerType={layer.type}
@@ -260,7 +264,12 @@ class LayerListContainer extends React.Component {
       })
     })
 
-    return <div className="maputnik-layer-list" ref={this.scrollContainerRef}>
+    return <div
+      className="maputnik-layer-list"
+      role="complementary"
+      aria-label="Layers list"
+      ref={this.scrollContainerRef}
+    >
       <AddModal
           key={this.state.keys.add}
           layers={this.props.layers}
@@ -275,7 +284,7 @@ class LayerListContainer extends React.Component {
         <div className="maputnik-default-property">
           <div className="maputnik-multibutton">
             <button
-              id="skip-menu"
+              id="skip-target-layer-list"
               onClick={this.toggleLayers}
               className="maputnik-button">
               {this.state.areAllGroupsExpanded === true ? "Collapse" : "Expand"}
@@ -293,7 +302,10 @@ class LayerListContainer extends React.Component {
           </div>
         </div>
       </header>
-      <ul className="maputnik-layer-list-container">
+      <ul className="maputnik-layer-list-container"
+        role="navigation"
+        aria-label="Layers list"
+      >
         {listItems}
       </ul>
     </div>

src/components/layers/LayerListGroup.jsx

@@ -7,7 +7,8 @@ export default class LayerListGroup extends React.Component {
     title: PropTypes.string.isRequired,
     "data-wd-key": PropTypes.string,
     isActive: PropTypes.bool.isRequired,
-    onActiveToggle: PropTypes.func.isRequired
+    onActiveToggle: PropTypes.func.isRequired,
+    'aria-controls': PropTypes.string,
   }
 
   render() {
@@ -16,7 +17,13 @@ export default class LayerListGroup extends React.Component {
         data-wd-key={"layer-list-group:"+this.props["data-wd-key"]}
         onClick={e => this.props.onActiveToggle(!this.props.isActive)}
       >
-        <span className="maputnik-layer-list-group-title">{this.props.title}</span>
+        <button
+          className="maputnik-layer-list-group-title"
+          aria-controls={this.props['aria-controls']}
+          aria-expanded={this.props.isActive}
+        >
+          {this.props.title}
+        </button>
         <span className="maputnik-space" />
         <Collapser
           style={{ height: 14, width: 14 }}

src/components/layers/LayerListItem.jsx

@@ -14,7 +14,9 @@ const DraggableLabel = SortableHandle((props) => {
       className="layer-handle__icon"
       type={props.layerType}
     />
-    <span className="maputnik-layer-list-item-id">{props.layerId}</span>
+    <button className="maputnik-layer-list-item-id">
+      {props.layerId}
+    </button>
   </div>
 });
 
@@ -54,6 +56,7 @@ class IconAction extends React.Component {
       className={`maputnik-layer-list-icon-action ${classAdditions}`}
       data-wd-key={this.props.wdKey}
       onClick={this.props.onClick}
+      aria-hidden="true"
     >
       {this.renderIcon()}
     </button>

src/components/map/MapboxGlMap.jsx

@@ -221,6 +221,8 @@ export default class MapboxGlMap extends React.Component {
     if(IS_SUPPORTED) {
       return <div
         className="maputnik-map__map"
+        role="region"
+        aria-label="Map view"
         ref={x => this.container = x}
       ></div>
     }

src/components/map/OpenLayersMap.jsx

@@ -179,6 +179,8 @@ export default class OpenLayersMap extends React.Component {
       <div
         className="maputnik-ol"
         ref={x => this.container = x}
+        role="region"
+        aria-label="Map view"
         style={{
           ...this.props.style,
         }}>

src/components/modals/ExportModal.jsx

@@ -104,7 +104,10 @@ class ExportModal extends React.Component {
           </InputBlock>
         </div>
 
-        <Button onClick={this.downloadStyle.bind(this)}>
+        <Button
+          onClick={this.downloadStyle.bind(this)}
+          title="Download style"
+        >
           <MdFileDownload />
           Download
         </Button>

src/components/modals/Modal.jsx

@@ -54,6 +54,7 @@ class Modal extends React.Component {
             <h1 className="maputnik-modal-header-title">{this.props.title}</h1>
             <span className="maputnik-modal-header-space"></span>
             <button className="maputnik-modal-header-toggle"
+              title="Close modal"
               onClick={this.onClose}
               data-wd-key={this.props["data-wd-key"]+".close-modal"}
             >

src/components/modals/OpenModal.jsx

@@ -226,7 +226,7 @@ class OpenModal extends React.Component {
                   type="submit"
                   className="maputnik-big-button"
                   disabled={this.state.styleUrl.length < 1}
-                >Open URL</Button>
+                >Load from URL</Button>
               </div>
             </form>
           </section>

src/components/modals/SettingsModal.jsx

@@ -190,6 +190,7 @@ class SettingsModal extends React.Component {
       <InputBlock label={"Light anchor"} fieldSpec={latest.light.anchor}>
         <EnumInput
           {...inputProps}
+          name="light-anchor"
           value={light.anchor}
           options={Object.keys(latest.light.anchor.values)}
           default={latest.light.anchor.default}

src/components/modals/ShortcutsModal.jsx

@@ -13,40 +13,92 @@ class ShortcutsModal extends React.Component {
   render() {
     const help = [
       {
-        key: "?",
+        key: <kbd>?</kbd>,
         text: "Shortcuts menu"
       },
       {
-        key: "o",
+        key: <kbd>o</kbd>,
         text: "Open modal"
       },
       {
-        key: "e",
+        key: <kbd>e</kbd>,
         text: "Export modal"
       },
       {
-        key: "d",
+        key: <kbd>d</kbd>,
         text: "Data Sources modal"
       },
       {
-        key: "s",
+        key: <kbd>s</kbd>,
         text: "Style Settings modal"
       },
       {
-        key: "i",
+        key: <kbd>i</kbd>,
         text: "Toggle inspect"
       },
       {
-        key: "m",
+        key: <kbd>m</kbd>,
         text: "Focus map"
       },
       {
-        key: "!",
+        key: <kbd>!</kbd>,
         text: "Debug modal"
       },
     ]
 
 
+    const mapShortcuts = [
+      {
+        key: <kbd>+</kbd>,
+        text: "Increase the zoom level by 1.",
+      },
+      {
+        key: <><kbd>Shift</kbd> + <kbd>+</kbd></>,
+        text: "Increase the zoom level by 2.",
+      },
+      {
+        key: <kbd>-</kbd>,
+        text: "Decrease the zoom level by 1.",
+      },
+      {
+        key: <><kbd>Shift</kbd> + <kbd>-</kbd></>,
+        text: "Decrease the zoom level by 2.",
+      },
+      {
+        key: <kbd>Up</kbd>,
+        text: "Pan up by 100 pixels.",
+      },
+      {
+        key: <kbd>Down</kbd>,
+        text: "Pan down by 100 pixels.",
+      },
+      {
+        key: <kbd>Left</kbd>,
+        text: "Pan left by 100 pixels.",
+      },
+      {
+        key: <kbd>Right</kbd>,
+        text: "Pan right by 100 pixels.",
+      },
+      {
+        key: <><kbd>Shift</kbd> + <kbd>Right</kbd></>,
+        text: "Increase the rotation by 15 degrees.",
+      },
+      {
+        key: <><kbd>Shift</kbd> + <kbd>Left</kbd></>,
+        text: "Decrease the rotation by 15 degrees."
+      },
+      {
+        key: <><kbd>Shift</kbd> + <kbd>Up</kbd></>,
+        text: "Increase the pitch by 10 degrees."
+      },
+      {
+        key: <><kbd>Shift</kbd> + <kbd>Down</kbd></>,
+        text: "Decrease the pitch by 10 degrees."
+      },
+    ]
+
+
     return <Modal
       data-wd-key="shortcuts-modal"
       isOpen={this.props.isOpen}
@@ -57,10 +109,19 @@ class ShortcutsModal extends React.Component {
         <p>
           Press <code>ESC</code> to lose focus of any active elements, then press one of:
         </p>
+        <dl>
+          {help.map((item, idx) => {
+            return <div key={idx} className="maputnik-modal-shortcuts__shortcut">
+              <dt key={"dt"+idx}>{item.key}</dt>
+              <dd key={"dd"+idx}>{item.text}</dd>
+            </div>
+          })}
+        </dl>
+        <p>If the Map is in focused you can use the following shortcuts</p>
         <ul>
-          {help.map((item) => {
-            return <li key={item.key}>
-              <code>{item.key}</code> {item.text}
+          {mapShortcuts.map((item, idx) => {
+            return <li key={idx}>
+              <span>{item.key}</span> {item.text}
             </li>
           })}
         </ul>

src/styles/_filtereditor.scss

@@ -78,3 +78,19 @@
   width: 92.5%;
 }
 
+.maputnik-radio-as-button {
+  @extend .maputnik-button;
+
+  border: solid 1px transparent;
+
+  &:focus-within {
+    border: solid 1px $color-white;
+  }
+
+  input {
+    width: 0;
+    overflow: hidden;
+    opacity: 0;
+    margin: 0;
+  }
+}

src/styles/_layer.scss

@@ -36,6 +36,7 @@
   &-item-handle {
     flex: 1;
     display: flex;
+    cursor: grab;
 
     svg {
       margin-right: 4px;
@@ -43,12 +44,11 @@
   }
 
   &-item {
+    border: solid 1px transparent;
     font-weight: 400;
     color: $color-lowgray;
     font-size: $font-size-6;
-    border-width: 0 0 1px;
-    border-style: solid;
-    border-color: lighten($color-black, 0.1);
+    border-bottom-color: lighten($color-black, 0.1);
     user-select: none;
     list-style: none;
     z-index: 2000;
@@ -61,6 +61,10 @@
     -webkit-transition: opacity 600ms, visibility 600ms;
     transition: opacity 600ms, visibility 600ms;
 
+    &:focus-within {
+      border: solid 1px $color-lowgray;
+    }
+
     @media screen and (prefers-reduced-motion: reduce) {
       transition-duration: 0;
     }
@@ -131,22 +135,33 @@
   }
 
   &-item-id {
+    all: inherit;
     width: 115px;
     white-space: nowrap;
     overflow: hidden;
     text-overflow: ellipsis;
     color: inherit;
     text-decoration: none;
+    cursor: pointer;
   }
 
   &-group-header {
+    border: solid 1px transparent;
     font-size: $font-size-6;
     color: $color-lowgray;
     background-color: lighten($color-black, 2);
-    cursor: pointer;
     user-select: none;
     padding: $margin-2;
 
+    &:focus-within {
+      border: solid 1px $color-lowgray;
+    }
+
+    button {
+      all: unset;
+      cursor: pointer;
+    }
+
     @include flex-row;
 
     svg {

src/styles/_modal.scss

@@ -256,17 +256,33 @@
 }
 
 .maputnik-modal-shortcuts {
-  code {
+  position: relative;
+  overflow: hidden;
+  max-width: 30em;
+
+  kbd {
     color: white;
     background: #3c3c3c;
     padding: 2px 6px;
     display: inline-block;
     text-align: center;
     border-radius: 2px;
-    margin-right: 4px;
     font-family: monospace;
   }
 
+  &__shortcut {
+    margin-bottom: $margin-2;
+  }
+
+  dt {
+    display: inline;
+    margin-right: $margin-2;
+  }
+
+  dd {
+    display: inline;
+  }
+
   li {
     margin-bottom: 4px;
   }

src/styles/_toolbar.scss

@@ -132,6 +132,8 @@
 }
 
 .maputnik-toolbar-skip {
+  all: unset;
+  border: solid 1px transparent;
   position: absolute;
   overflow: hidden;
   width: 0px;
@@ -147,6 +149,7 @@
   &:active,
   &:focus {
     width: 100%;
+    border-color: $color-lowgray;
   }
 }