Merge branch 'improve-location-picker-accessibility' into master

7 files changed, 220 insertions, 362 deletions

diff --git a/gui/src/renderer/components/Cell.tsx b/gui/src/renderer/components/Cell.tsx
index 1848426e91..7c021b060d 100644
--- a/gui/src/renderer/components/Cell.tsx
+++ b/gui/src/renderer/components/Cell.tsx
@@ -30,13 +30,17 @@ interface IContainerProps extends React.HTMLAttributes<HTMLDivElement> {
   disabled?: boolean;
 }
 
-export function Container({ disabled, ...otherProps }: IContainerProps) {
+export const Container = React.forwardRef(function ContainerT(
+  props: IContainerProps,
+  ref: React.Ref<HTMLDivElement>,
+) {
+  const { disabled, ...otherProps } = props;
   return (
     <CellDisabledContext.Provider value={disabled ?? false}>
-      <StyledContainer {...otherProps} />
+      <StyledContainer ref={ref} {...otherProps} />
     </CellDisabledContext.Provider>
   );
-}
+});
 
 interface ICellButtonProps extends React.ButtonHTMLAttributes<HTMLButtonElement> {
   selected?: boolean;
diff --git a/gui/src/renderer/components/ChevronButton.tsx b/gui/src/renderer/components/ChevronButton.tsx
index 0c08408dff..63452b521e 100644
--- a/gui/src/renderer/components/ChevronButton.tsx
+++ b/gui/src/renderer/components/ChevronButton.tsx
@@ -3,12 +3,15 @@ import styled from 'styled-components';
 import { colors } from '../../config.json';
 import * as Cell from './Cell';
 
-interface IProps {
+interface IProps extends React.HTMLAttributes<HTMLButtonElement> {
   up: boolean;
-  onClick?: (event: React.MouseEvent) => void;
-  className?: string;
 }
 
+const Button = styled.button({
+  border: 'none',
+  background: 'none',
+});
+
 const Icon = styled(Cell.Icon)({
   flex: 0,
   alignSelf: 'stretch',
@@ -16,15 +19,17 @@ const Icon = styled(Cell.Icon)({
 });
 
 export default function ChevronButton(props: IProps) {
+  const { up, ...otherProps } = props;
+
   return (
-    <Icon
-      tintColor={colors.white80}
-      tintHoverColor={colors.white}
-      onClick={props.onClick}
-      source={props.up ? 'icon-chevron-up' : 'icon-chevron-down'}
-      height={24}
-      width={24}
-      className={props.className}
-    />
+    <Button {...otherProps}>
+      <Icon
+        tintColor={colors.white80}
+        tintHoverColor={colors.white}
+        source={up ? 'icon-chevron-up' : 'icon-chevron-down'}
+        height={24}
+        width={24}
+      />
+    </Button>
   );
 }
diff --git a/gui/src/renderer/components/CityRow.tsx b/gui/src/renderer/components/CityRow.tsx
deleted file mode 100644
index 5bb74467aa..0000000000
--- a/gui/src/renderer/components/CityRow.tsx
+++ /dev/null
@@ -1,132 +0,0 @@
-import * as React from 'react';
-import styled from 'styled-components';
-import { colors } from '../../config.json';
-import { compareRelayLocation, RelayLocation } from '../../shared/daemon-rpc-types';
-import Accordion from './Accordion';
-import * as Cell from './Cell';
-import ChevronButton from './ChevronButton';
-import RelayRow from './RelayRow';
-import RelayStatusIndicator from './RelayStatusIndicator';
-
-type RelayRowElement = React.ReactElement<RelayRow['props']>;
-
-interface IProps {
-  name: string;
-  hasActiveRelays: boolean;
-  location: RelayLocation;
-  selected: boolean;
-  expanded: boolean;
-  onSelect?: (location: RelayLocation) => void;
-  onExpand?: (location: RelayLocation, value: boolean) => void;
-  onWillExpand?: (locationRect: DOMRect, expandedContentHeight: number) => void;
-  onTransitionEnd?: () => void;
-  children?: RelayRowElement | RelayRowElement[];
-}
-
-const Button = styled(Cell.CellButton)((props: { selected: boolean }) => ({
-  paddingRight: '16px',
-  paddingLeft: '34px',
-  backgroundColor: !props.selected ? colors.blue40 : undefined,
-}));
-
-const StyledChevronButton = styled(ChevronButton)({
-  marginLeft: '18px',
-});
-
-const Label = styled(Cell.Label)({
-  fontFamily: 'Open Sans',
-  fontWeight: 'normal',
-  fontSize: '16px',
-});
-
-export default class CityRow extends React.Component<IProps> {
-  private buttonRef = React.createRef<HTMLButtonElement>();
-
-  public static compareProps(oldProps: IProps, nextProps: IProps): boolean {
-    if (React.Children.count(oldProps.children) !== React.Children.count(nextProps.children)) {
-      return false;
-    }
-
-    if (
-      oldProps.name !== nextProps.name ||
-      oldProps.hasActiveRelays !== nextProps.hasActiveRelays ||
-      oldProps.selected !== nextProps.selected ||
-      oldProps.expanded !== nextProps.expanded ||
-      !compareRelayLocation(oldProps.location, nextProps.location)
-    ) {
-      return false;
-    }
-
-    const currChildren = React.Children.toArray(oldProps.children || []) as RelayRowElement[];
-    const nextChildren = React.Children.toArray(nextProps.children || []) as RelayRowElement[];
-
-    for (let i = 0; i < currChildren.length; i++) {
-      const currChild = currChildren[i];
-      const nextChild = nextChildren[i];
-
-      if (!RelayRow.compareProps(currChild.props, nextChild.props)) {
-        return false;
-      }
-    }
-
-    return true;
-  }
-
-  public shouldComponentUpdate(nextProps: IProps) {
-    return !CityRow.compareProps(this.props, nextProps);
-  }
-
-  public render() {
-    const hasChildren = React.Children.count(this.props.children) > 1;
-
-    return (
-      <>
-        <Button
-          ref={this.buttonRef}
-          onClick={this.handleClick}
-          disabled={!this.props.hasActiveRelays}
-          selected={this.props.selected}>
-          <RelayStatusIndicator
-            active={this.props.hasActiveRelays}
-            selected={this.props.selected}
-          />
-          <Label>{this.props.name}</Label>
-
-          {hasChildren && (
-            <StyledChevronButton onClick={this.toggleCollapse} up={this.props.expanded} />
-          )}
-        </Button>
-
-        {hasChildren && (
-          <Accordion
-            expanded={this.props.expanded}
-            onWillExpand={this.onWillExpand}
-            onTransitionEnd={this.props.onTransitionEnd}
-            animationDuration={150}>
-            {this.props.children}
-          </Accordion>
-        )}
-      </>
-    );
-  }
-
-  private toggleCollapse = (event: React.MouseEvent) => {
-    if (this.props.onExpand) {
-      this.props.onExpand(this.props.location, !this.props.expanded);
-    }
-    event.stopPropagation();
-  };
-
-  private handleClick = () => {
-    if (this.props.onSelect) {
-      this.props.onSelect(this.props.location);
-    }
-  };
-
-  private onWillExpand = (nextHeight: number) => {
-    const buttonRect = this.buttonRef.current?.getBoundingClientRect();
-    if (buttonRect) {
-      this.props.onWillExpand?.(buttonRect, nextHeight);
-    }
-  };
-}
diff --git a/gui/src/renderer/components/CountryRow.tsx b/gui/src/renderer/components/CountryRow.tsx
deleted file mode 100644
index d09cff0e31..0000000000
--- a/gui/src/renderer/components/CountryRow.tsx
+++ /dev/null
@@ -1,136 +0,0 @@
-import * as React from 'react';
-import styled from 'styled-components';
-import { compareRelayLocation, RelayLocation } from '../../shared/daemon-rpc-types';
-import Accordion from './Accordion';
-import * as Cell from './Cell';
-import ChevronButton from './ChevronButton';
-import CityRow from './CityRow';
-import RelayStatusIndicator from './RelayStatusIndicator';
-
-type CityRowElement = React.ReactElement<CityRow['props']>;
-
-interface IProps {
-  name: string;
-  hasActiveRelays: boolean;
-  location: RelayLocation;
-  selected: boolean;
-  expanded: boolean;
-  onSelect?: (location: RelayLocation) => void;
-  onExpand?: (location: RelayLocation, value: boolean) => void;
-  onWillExpand?: (locationRect: DOMRect, expandedContentHeight: number) => void;
-  onTransitionEnd?: () => void;
-  children?: CityRowElement | CityRowElement[];
-}
-
-const Button = styled(Cell.CellButton)({
-  paddingRight: '16px',
-  // The actual padding is 22px except for the tick icon which has 18.
-  paddingLeft: '18px',
-});
-
-const StyledChevronButton = styled(ChevronButton)({
-  marginLeft: '18px',
-});
-
-const Label = styled(Cell.Label)({
-  fontFamily: 'Open Sans',
-  fontWeight: 'normal',
-  fontSize: '16px',
-});
-
-export default class CountryRow extends React.Component<IProps> {
-  private buttonRef = React.createRef<HTMLButtonElement>();
-
-  public static compareProps(oldProps: IProps, nextProps: IProps) {
-    if (React.Children.count(oldProps.children) !== React.Children.count(nextProps.children)) {
-      return false;
-    }
-
-    if (
-      oldProps.name !== nextProps.name ||
-      oldProps.hasActiveRelays !== nextProps.hasActiveRelays ||
-      oldProps.selected !== nextProps.selected ||
-      oldProps.expanded !== nextProps.expanded ||
-      !compareRelayLocation(oldProps.location, nextProps.location)
-    ) {
-      return false;
-    }
-
-    const currChildren = React.Children.toArray(oldProps.children || []) as CityRowElement[];
-    const nextChildren = React.Children.toArray(nextProps.children || []) as CityRowElement[];
-
-    for (let i = 0; i < currChildren.length; i++) {
-      const currChild = currChildren[i];
-      const nextChild = nextChildren[i];
-
-      if (!CityRow.compareProps(currChild.props, nextChild.props)) {
-        return false;
-      }
-    }
-
-    return true;
-  }
-
-  public shouldComponentUpdate(nextProps: IProps) {
-    return !CountryRow.compareProps(this.props, nextProps);
-  }
-
-  public render() {
-    const childrenArray = React.Children.toArray(this.props.children || []) as CityRowElement[];
-    const numChildren = childrenArray.length;
-    const onlyChild = numChildren === 1 ? childrenArray[0] : undefined;
-    const numOnlyChildChildren = onlyChild
-      ? React.Children.count(onlyChild.props.children || [])
-      : 0;
-    const hasChildren = numChildren > 1 || numOnlyChildChildren > 1;
-
-    return (
-      <>
-        <Button
-          ref={this.buttonRef}
-          onClick={this.handleClick}
-          disabled={!this.props.hasActiveRelays}
-          selected={this.props.selected}>
-          <RelayStatusIndicator
-            active={this.props.hasActiveRelays}
-            selected={this.props.selected}
-          />
-          <Label>{this.props.name}</Label>
-          {hasChildren ? (
-            <StyledChevronButton onClick={this.toggleCollapse} up={this.props.expanded} />
-          ) : null}
-        </Button>
-
-        {hasChildren && (
-          <Accordion
-            expanded={this.props.expanded}
-            onWillExpand={this.onWillExpand}
-            onTransitionEnd={this.props.onTransitionEnd}
-            animationDuration={150}>
-            {this.props.children}
-          </Accordion>
-        )}
-      </>
-    );
-  }
-
-  private toggleCollapse = (event: React.MouseEvent) => {
-    if (this.props.onExpand) {
-      this.props.onExpand(this.props.location, !this.props.expanded);
-    }
-    event.stopPropagation();
-  };
-
-  private handleClick = () => {
-    if (this.props.onSelect) {
-      this.props.onSelect(this.props.location);
-    }
-  };
-
-  private onWillExpand = (nextHeight: number) => {
-    const buttonRect = this.buttonRef.current?.getBoundingClientRect();
-    if (buttonRect) {
-      this.props.onWillExpand?.(buttonRect, nextHeight);
-    }
-  };
-}
diff --git a/gui/src/renderer/components/LocationList.tsx b/gui/src/renderer/components/LocationList.tsx
index bd30d5d621..bc7e2fa4ed 100644
--- a/gui/src/renderer/components/LocationList.tsx
+++ b/gui/src/renderer/components/LocationList.tsx
@@ -9,9 +9,7 @@ import {
 } from '../../shared/daemon-rpc-types';
 import { IRelayLocationRedux } from '../redux/settings/reducers';
 import * as Cell from './Cell';
-import CityRow from './CityRow';
-import CountryRow from './CountryRow';
-import RelayRow from './RelayRow';
+import LocationRow from './LocationRow';
 
 export enum LocationSelectionType {
   relay = 'relay',
@@ -264,13 +262,13 @@ interface IRelayLocationsProps {
   onTransitionEnd?: () => void;
 }
 
-interface ICommonCellProps<T> {
+interface ICommonCellProps {
   location: RelayLocation;
   selected: boolean;
-  ref?: React.Ref<T>;
+  ref?: React.Ref<HTMLDivElement>;
 }
 
-export class RelayLocations extends React.Component<IRelayLocationsProps> {
+export class RelayLocations extends React.PureComponent<IRelayLocationsProps> {
   public render() {
     return (
       <>
@@ -278,51 +276,51 @@ export class RelayLocations extends React.Component<IRelayLocationsProps> {
           const countryLocation: RelayLocation = { country: relayCountry.code };
 
           return (
-            <CountryRow
+            <LocationRow
               key={getLocationKey(countryLocation)}
               name={relayCountry.name}
-              hasActiveRelays={relayCountry.hasActiveRelays}
+              active={relayCountry.hasActiveRelays}
               expanded={this.isExpanded(countryLocation)}
               onSelect={this.handleSelection}
               onExpand={this.handleExpand}
               onWillExpand={this.props.onWillExpand}
               onTransitionEnd={this.props.onTransitionEnd}
-              {...this.getCommonCellProps<CountryRow>(countryLocation)}>
+              {...this.getCommonCellProps(countryLocation)}>
               {relayCountry.cities.map((relayCity) => {
                 const cityLocation: RelayLocation = {
                   city: [relayCountry.code, relayCity.code],
                 };
 
                 return (
-                  <CityRow
+                  <LocationRow
                     key={getLocationKey(cityLocation)}
                     name={relayCity.name}
-                    hasActiveRelays={relayCity.hasActiveRelays}
+                    active={relayCity.hasActiveRelays}
                     expanded={this.isExpanded(cityLocation)}
                     onSelect={this.handleSelection}
                     onExpand={this.handleExpand}
                     onWillExpand={this.props.onWillExpand}
                     onTransitionEnd={this.props.onTransitionEnd}
-                    {...this.getCommonCellProps<CityRow>(cityLocation)}>
+                    {...this.getCommonCellProps(cityLocation)}>
                     {relayCity.relays.map((relay) => {
                       const relayLocation: RelayLocation = {
                         hostname: [relayCountry.code, relayCity.code, relay.hostname],
                       };
 
                       return (
-                        <RelayRow
+                        <LocationRow
                           key={getLocationKey(relayLocation)}
+                          name={relay.hostname}
                           active={relay.active}
-                          hostname={relay.hostname}
                           onSelect={this.handleSelection}
-                          {...this.getCommonCellProps<RelayRow>(relayLocation)}
+                          {...this.getCommonCellProps(relayLocation)}
                         />
                       );
                     })}
-                  </CityRow>
+                  </LocationRow>
                 );
               })}
-            </CountryRow>
+            </LocationRow>
           );
         })}
       </>
@@ -353,12 +351,12 @@ export class RelayLocations extends React.Component<IRelayLocationsProps> {
     }
   };
 
-  private getCommonCellProps<T>(location: RelayLocation): ICommonCellProps<T> {
+  private getCommonCellProps(location: RelayLocation): ICommonCellProps {
     const selected = this.isSelected(location);
     const ref =
       selected && this.props.selectedElementRef ? this.props.selectedElementRef : undefined;
 
-    return { ref: ref as React.Ref<T>, selected, location };
+    return { ref: ref as React.Ref<HTMLDivElement>, selected, location };
   }
 }
 
diff --git a/gui/src/renderer/components/LocationRow.tsx b/gui/src/renderer/components/LocationRow.tsx
new file mode 100644
index 0000000000..48f5982af2
--- /dev/null
+++ b/gui/src/renderer/components/LocationRow.tsx
@@ -0,0 +1,179 @@
+import React, { useCallback, useRef } from 'react';
+import { sprintf } from 'sprintf-js';
+import styled from 'styled-components';
+import { colors } from '../../config.json';
+import { compareRelayLocation, RelayLocation } from '../../shared/daemon-rpc-types';
+import { messages } from '../../shared/gettext';
+import Accordion from './Accordion';
+import * as Cell from './Cell';
+import ChevronButton from './ChevronButton';
+import RelayStatusIndicator from './RelayStatusIndicator';
+
+interface IContainerProps {
+  selected: boolean;
+  disabled: boolean;
+  location: RelayLocation;
+}
+
+const Container = styled(Cell.Container)((props: IContainerProps) => {
+  const background =
+    'hostname' in props.location
+      ? colors.blue20
+      : 'city' in props.location
+      ? colors.blue40
+      : colors.blue;
+  const backgroundHover = 'country' in props.location ? colors.blue80 : colors.blue80;
+
+  return {
+    display: 'flex',
+    // The actual padding is 22px except for the tick icon which has 18.
+    paddingLeft: '18px',
+    marginBottom: '1px',
+    backgroundColor: props.selected ? colors.green : background,
+    ':not(:disabled):hover': {
+      backgroundColor: props.selected
+        ? colors.green
+        : props.disabled
+        ? background
+        : backgroundHover,
+    },
+  };
+});
+
+const Button = styled.button((props: { location: RelayLocation }) => {
+  const paddingLeft = 'hostname' in props.location ? 32 : 'city' in props.location ? 16 : 0;
+
+  return {
+    display: 'flex',
+    alignItems: 'center',
+    flex: 1,
+    border: 'none',
+    background: 'none',
+    padding: `0 0 0 ${paddingLeft}px`,
+    margin: 0,
+  };
+});
+
+const StyledChevronButton = styled(ChevronButton)({
+  marginLeft: '18px',
+});
+
+const Label = styled(Cell.Label)({
+  fontFamily: 'Open Sans',
+  fontWeight: 'normal',
+  fontSize: '16px',
+});
+
+interface IProps {
+  name: string;
+  active: boolean;
+  location: RelayLocation;
+  selected: boolean;
+  expanded?: boolean;
+  onSelect?: (location: RelayLocation) => void;
+  onExpand?: (location: RelayLocation, value: boolean) => void;
+  onWillExpand?: (locationRect: DOMRect, expandedContentHeight: number) => void;
+  onTransitionEnd?: () => void;
+  children?: React.ReactElement<IProps>[];
+}
+
+function LocationRow(props: IProps, ref: React.Ref<HTMLDivElement>) {
+  const hasChildren = props.children !== undefined;
+  const buttonRef = useRef<HTMLButtonElement>() as React.RefObject<HTMLButtonElement>;
+
+  const toggleCollapse = useCallback(
+    (event: React.MouseEvent) => {
+      props.onExpand?.(props.location, !props.expanded);
+      event.stopPropagation();
+    },
+    [props.onExpand, props.expanded, props.location],
+  );
+
+  const handleClick = useCallback(() => props.onSelect?.(props.location), [
+    props.onSelect,
+    props.location,
+  ]);
+
+  const onWillExpand = useCallback(
+    (nextHeight: number) => {
+      const buttonRect = buttonRef.current?.getBoundingClientRect();
+      if (buttonRect) {
+        props.onWillExpand?.(buttonRect, nextHeight);
+      }
+    },
+    [props.onWillExpand],
+  );
+
+  return (
+    <>
+      <Container
+        ref={ref}
+        selected={props.selected}
+        disabled={!props.active}
+        location={props.location}>
+        <Button
+          ref={buttonRef}
+          onClick={handleClick}
+          location={props.location}
+          disabled={!props.active}>
+          <RelayStatusIndicator active={props.active} selected={props.selected} />
+          <Label>{props.name}</Label>
+        </Button>
+        {hasChildren ? (
+          <StyledChevronButton
+            onClick={toggleCollapse}
+            up={props.expanded ?? false}
+            aria-label={sprintf(
+              props.expanded
+                ? messages.pgettext('accessibility', 'Collapse %(location)s')
+                : messages.pgettext('accessibility', 'Expand %(location)s'),
+              { location: props.name },
+            )}
+          />
+        ) : null}
+      </Container>
+
+      {hasChildren && (
+        <Accordion
+          expanded={props.expanded}
+          onWillExpand={onWillExpand}
+          onTransitionEnd={props.onTransitionEnd}
+          animationDuration={150}>
+          {props.children}
+        </Accordion>
+      )}
+    </>
+  );
+}
+
+export default React.memo(React.forwardRef(LocationRow), compareProps);
+
+function compareProps(oldProps: IProps, nextProps: IProps): boolean {
+  return (
+    React.Children.count(oldProps.children) === React.Children.count(nextProps.children) &&
+    oldProps.name === nextProps.name &&
+    oldProps.active === nextProps.active &&
+    oldProps.selected === nextProps.selected &&
+    oldProps.expanded === nextProps.expanded &&
+    oldProps.onSelect === nextProps.onSelect &&
+    oldProps.onExpand === nextProps.onExpand &&
+    oldProps.onWillExpand === nextProps.onWillExpand &&
+    oldProps.onTransitionEnd === nextProps.onTransitionEnd &&
+    compareRelayLocation(oldProps.location, nextProps.location) &&
+    compareChildren(oldProps.children, nextProps.children)
+  );
+}
+
+function compareChildren(
+  oldChildren?: React.ReactElement<IProps>[],
+  nextChildren?: React.ReactElement<IProps>[],
+) {
+  if (oldChildren === undefined || nextChildren === undefined) {
+    return oldChildren === nextChildren;
+  }
+
+  return (
+    oldChildren.length === nextChildren.length &&
+    oldChildren.every((oldChild, i) => compareProps(oldChild.props, nextChildren[i].props))
+  );
+}
diff --git a/gui/src/renderer/components/RelayRow.tsx b/gui/src/renderer/components/RelayRow.tsx
deleted file mode 100644
index 0b3602252b..0000000000
--- a/gui/src/renderer/components/RelayRow.tsx
+++ /dev/null
@@ -1,60 +0,0 @@
-import * as React from 'react';
-import styled from 'styled-components';
-import { colors } from '../../config.json';
-import { compareRelayLocation, RelayLocation } from '../../shared/daemon-rpc-types';
-import * as Cell from './Cell';
-import RelayStatusIndicator from './RelayStatusIndicator';
-
-interface IProps {
-  location: RelayLocation;
-  active: boolean;
-  hostname: string;
-  selected: boolean;
-  onSelect?: (location: RelayLocation) => void;
-}
-
-const Button = styled(Cell.CellButton)((props: { selected: boolean }) => ({
-  paddingRight: 0,
-  paddingLeft: '50px',
-  backgroundColor: !props.selected ? colors.blue20 : undefined,
-}));
-
-const Label = styled(Cell.Label)({
-  fontFamily: 'Open Sans',
-  fontWeight: 'normal',
-  fontSize: '16px',
-});
-
-export default class RelayRow extends React.Component<IProps> {
-  public static compareProps(oldProps: IProps, nextProps: IProps) {
-    return (
-      oldProps.hostname === nextProps.hostname &&
-      oldProps.selected === nextProps.selected &&
-      oldProps.active === nextProps.active &&
-      compareRelayLocation(oldProps.location, nextProps.location)
-    );
-  }
-
-  public shouldComponentUpdate(nextProps: IProps) {
-    return !RelayRow.compareProps(this.props, nextProps);
-  }
-
-  public render() {
-    return (
-      <Button
-        onClick={this.handleClick}
-        selected={this.props.selected}
-        disabled={!this.props.active}>
-        <RelayStatusIndicator active={this.props.active} selected={this.props.selected} />
-
-        <Label>{this.props.hostname}</Label>
-      </Button>
-    );
-  }
-
-  private handleClick = () => {
-    if (this.props.onSelect) {
-      this.props.onSelect(this.props.location);
-    }
-  };
-}