Browse Source

feat(ui): Require aria-label on buttons without children [INGEST-866] (#31390)

Matej Minar 3 years ago
parent
commit
c4f76893c6

+ 6 - 4
static/app/components/actions/actionLink.tsx

@@ -2,6 +2,8 @@ import * as React from 'react';
 import styled from '@emotion/styled';
 import classNames from 'classnames';
 
+import {t} from 'sentry/locale';
+
 import ActionButton from './button';
 import ConfirmableAction from './confirmableAction';
 
@@ -27,11 +29,11 @@ type ConfirmableActionProps = React.ComponentProps<typeof ConfirmableAction>;
 
 type CommonProps = Omit<
   ConfirmableActionProps,
-  'onConfirm' | 'confirmText' | 'children' | 'stopPropagation' | 'priority'
+  'onConfirm' | 'confirmText' | 'children' | 'stopPropagation' | 'priority' | 'children'
 > & {
   title: string;
+  children: React.ReactChild;
   onAction?: () => void;
-  children?: React.ReactNode;
   disabled?: boolean;
   className?: string;
   shouldConfirm?: boolean;
@@ -41,7 +43,7 @@ type CommonProps = Omit<
 
 type Props = CommonProps &
   ({type?: 'button'} & Partial<
-    Omit<React.ComponentProps<typeof StyledActionButton>, 'as'>
+    Omit<React.ComponentProps<typeof StyledActionButton>, 'as' | 'children'>
   >);
 
 export default function ActionLink({
@@ -59,7 +61,7 @@ export default function ActionLink({
   ...props
 }: Props) {
   const actionCommonProps = {
-    ['aria-label']: title,
+    ['aria-label']: typeof title === 'string' ? title : t('Actions'),
     className: classNames(className, {disabled}),
     onClick: disabled ? undefined : onAction,
     disabled,

+ 1 - 0
static/app/components/actions/ignore.tsx

@@ -139,6 +139,7 @@ const IgnoreActions = ({
           <StyledActionButton
             disabled={disabled}
             icon={<IconChevron direction="down" size="xs" />}
+            aria-label={t('Ignore options')}
           />
         }
         alwaysRenderMenu

+ 13 - 4
static/app/components/button.tsx

@@ -17,6 +17,16 @@ import {Theme} from 'sentry/utils/theme';
  */
 type ButtonElement = HTMLButtonElement & HTMLAnchorElement & any;
 
+type ConditionalAriaLabel =
+  | {
+      children: Omit<React.ReactNode, 'null' | 'undefined' | 'boolean'>;
+      'aria-label'?: string;
+    }
+  | {
+      'aria-label': string;
+      children?: null | boolean;
+    };
+
 type Props = {
   priority?: 'default' | 'primary' | 'danger' | 'link' | 'success' | 'form';
   size?: 'zero' | 'xsmall' | 'small';
@@ -30,7 +40,6 @@ type Props = {
   external?: boolean;
   borderless?: boolean;
   translucentBorder?: boolean;
-  'aria-label'?: string;
   tooltipProps?: Omit<Tooltip['props'], 'children' | 'title' | 'skipWrapper'>;
   onClick?: (e: React.MouseEvent) => void;
   forwardRef?: React.Ref<ButtonElement>;
@@ -38,8 +47,7 @@ type Props = {
 
   // This is only used with `<ButtonBar>`
   barId?: string;
-  children?: React.ReactNode;
-};
+} & ConditionalAriaLabel;
 
 type ButtonProps = Omit<React.HTMLProps<ButtonElement>, keyof Props | 'ref' | 'label'> &
   Props;
@@ -84,7 +92,8 @@ function BaseButton({
     return disabled ? undefined : prop;
   }
 
-  // For `aria-label`
+  // Fallbacking aria-label to string children is not necessary as screen readers natively understand that scenario.
+  // Leaving it here for a bunch of our tests that query by aria-label.
   const screenReaderLabel =
     ariaLabel || (typeof children === 'string' ? children : undefined);
 

+ 6 - 1
static/app/components/clippedBox.tsx

@@ -123,7 +123,12 @@ class ClippedBox extends React.PureComponent<Props, State> {
         {children}
         {isClipped && (
           <ClipFade>
-            <Button onClick={this.reveal} priority="primary" size="xsmall">
+            <Button
+              onClick={this.reveal}
+              priority="primary"
+              size="xsmall"
+              aria-label={btnText ?? t('Show More')}
+            >
               {btnText}
             </Button>
           </ClipFade>

+ 7 - 1
static/app/components/confirm.tsx

@@ -301,7 +301,12 @@ class ConfirmModal extends React.Component<ModalProps, ModalState> {
                 defaultOnClick: this.handleClose,
               })
             ) : (
-              <Button onClick={this.handleClose}>{cancelText}</Button>
+              <Button
+                onClick={this.handleClose}
+                aria-label={typeof cancelText === 'string' ? cancelText : t('Cancel')}
+              >
+                {cancelText}
+              </Button>
             )}
             {renderConfirmButton ? (
               renderConfirmButton({
@@ -315,6 +320,7 @@ class ConfirmModal extends React.Component<ModalProps, ModalState> {
                 priority={priority}
                 onClick={this.handleConfirm}
                 autoFocus
+                aria-label={typeof confirmText === 'string' ? confirmText : t('Confirm')}
               >
                 {confirmText}
               </Button>

+ 5 - 1
static/app/components/createAlertButton.tsx

@@ -182,7 +182,10 @@ function IncompatibleQueryAlert({
   );
 }
 
-type CreateAlertFromViewButtonProps = React.ComponentProps<typeof Button> & {
+type CreateAlertFromViewButtonProps = Omit<
+  React.ComponentProps<typeof Button>,
+  'aria-label'
+> & {
   className?: string;
   projects: Project[];
   /**
@@ -315,6 +318,7 @@ function CreateAlertFromViewButton({
       organization={organization}
       onClick={handleClick}
       to={to}
+      aria-label={t('Create Alert')}
       {...buttonProps}
     />
   );

+ 1 - 1
static/app/components/datePageFilter.tsx

@@ -60,7 +60,7 @@ type Props = {
   /**
    * Override defaults from DEFAULT_RELATIVE_PERIODS_PAGE_FILTER
    */
-  relativeOptions?: Record<string, React.ReactNode>;
+  relativeOptions?: Record<string, React.ReactChild>;
   /**
    * Reset these URL params when we fire actions (custom routing only)
    */

+ 7 - 5
static/app/components/discoverButton.tsx

@@ -2,11 +2,9 @@ import * as React from 'react';
 
 import Button from 'sentry/components/button';
 import DiscoverFeature from 'sentry/components/discover/discoverFeature';
+import {t} from 'sentry/locale';
 
-type Props = React.PropsWithChildren<{
-  className?: string;
-}> &
-  React.ComponentProps<typeof Button>;
+type Props = Omit<React.ComponentProps<typeof Button>, 'aria-label'>;
 
 /**
  * Provide a button that turns itself off if the current organization
@@ -16,7 +14,11 @@ function DiscoverButton({children, ...buttonProps}: Props) {
   return (
     <DiscoverFeature>
       {({hasFeature}) => (
-        <Button disabled={!hasFeature} {...buttonProps}>
+        <Button
+          disabled={!hasFeature}
+          aria-label={t('Open in Discover')}
+          {...buttonProps}
+        >
           {children}
         </Button>
       )}

+ 1 - 0
static/app/components/events/interfaces/debugMeta-v2/debugImageDetails/candidate/actions.tsx

@@ -120,6 +120,7 @@ function Actions({
                       icon={<IconDelete size="xs" />}
                       size="xsmall"
                       disabled={!hasAccess}
+                      aria-label={t('Delete')}
                     />
                   </Confirm>
                 </Tooltip>

+ 7 - 7
static/app/components/events/interfaces/debugMeta/debugImage.tsx

@@ -247,13 +247,13 @@ const DebugImage = React.memo(
 
             return (
               <ImageActions>
-                <Tooltip title={t('Search for debug files in settings')}>
-                  <Button
-                    size="xsmall"
-                    icon={<IconSearch size="xs" />}
-                    to={settingsUrl}
-                  />
-                </Tooltip>
+                <Button
+                  size="xsmall"
+                  icon={<IconSearch size="xs" />}
+                  to={settingsUrl}
+                  title={t('Search for debug files in settings')}
+                  aria-label={t('Search for debug files in settings')}
+                />
               </ImageActions>
             );
           }}

Some files were not shown because too many files changed in this diff