From 8218d81f0eea12a7cde1954cc667c2fc80013244 Mon Sep 17 00:00:00 2001 From: kay delaney <45561153+kaydelaney@users.noreply.github.com> Date: Tue, 1 Feb 2022 15:33:21 +0000 Subject: [PATCH] AnnoListPanel: Add keyboard accessibility (#44280) --- .../grafana-ui/src/components/Tags/Tag.tsx | 7 +- .../src/components/Tags/TagList.tsx | 41 +++--- .../plugins/panel/annolist/AnnoListPanel.tsx | 62 ++++++-- .../panel/annolist/AnnotationListItem.tsx | 136 +++++++++--------- .../panel/annolist/AnnotationListItemTags.tsx | 49 ------- 5 files changed, 149 insertions(+), 146 deletions(-) delete mode 100644 public/app/plugins/panel/annolist/AnnotationListItemTags.tsx diff --git a/packages/grafana-ui/src/components/Tags/Tag.tsx b/packages/grafana-ui/src/components/Tags/Tag.tsx index c4c05dc9c48..01fc15e68cc 100644 --- a/packages/grafana-ui/src/components/Tags/Tag.tsx +++ b/packages/grafana-ui/src/components/Tags/Tag.tsx @@ -3,6 +3,8 @@ import { cx, css } from '@emotion/css'; import { GrafanaTheme } from '@grafana/data'; import { useTheme } from '../../themes'; import { getTagColor, getTagColorsFromName } from '../../utils'; +import { IconName } from '../../types/icon'; +import { Icon } from '../Icon/Icon'; /** * @public @@ -12,12 +14,13 @@ export type OnTagClick = (name: string, event: React.MouseEvent) => export interface Props extends Omit, 'onClick'> { /** Name of the tag to display */ name: string; + icon?: IconName; /** Use constant color from TAG_COLORS. Using index instead of color directly so we can match other styling. */ colorIndex?: number; onClick?: OnTagClick; } -export const Tag = forwardRef(({ name, onClick, className, colorIndex, ...rest }, ref) => { +export const Tag = forwardRef(({ name, onClick, icon, className, colorIndex, ...rest }, ref) => { const theme = useTheme(); const styles = getTagStyles(theme, name, colorIndex); @@ -32,10 +35,12 @@ export const Tag = forwardRef(({ name, onClick, className, c return onClick ? ( ) : ( + {icon && } {name} ); diff --git a/packages/grafana-ui/src/components/Tags/TagList.tsx b/packages/grafana-ui/src/components/Tags/TagList.tsx index 6d82cc2a64b..25e6825f1f6 100644 --- a/packages/grafana-ui/src/components/Tags/TagList.tsx +++ b/packages/grafana-ui/src/components/Tags/TagList.tsx @@ -1,8 +1,9 @@ -import React, { FC, memo } from 'react'; +import React, { forwardRef, memo } from 'react'; import { css, cx } from '@emotion/css'; import { OnTagClick, Tag } from './Tag'; import { useTheme2 } from '../../themes'; import { GrafanaTheme2 } from '@grafana/data'; +import { IconName } from '../../types/icon'; export interface Props { displayMax?: number; @@ -12,24 +13,30 @@ export interface Props { className?: string; /** aria-label for the `i`-th Tag component */ getAriaLabel?: (name: string, i: number) => string; + /** Icon to show next to tag label */ + icon?: IconName; } -export const TagList: FC = memo(({ displayMax, tags, onClick, className, getAriaLabel }) => { - const theme = useTheme2(); - const styles = getStyles(theme, Boolean(displayMax && displayMax > 0)); - const numTags = tags.length; - const tagsToDisplay = displayMax ? tags.slice(0, displayMax) : tags; - return ( -
    - {tagsToDisplay.map((tag, i) => ( -
  • - -
    • - ))} - {displayMax && displayMax > 0 && numTags - 1 > 0 && + {numTags - 1}} -
- ); -}); +export const TagList = memo( + forwardRef(({ displayMax, tags, icon, onClick, className, getAriaLabel }, ref) => { + const theme = useTheme2(); + const styles = getStyles(theme, Boolean(displayMax && displayMax > 0)); + const numTags = tags.length; + const tagsToDisplay = displayMax ? tags.slice(0, displayMax) : tags; + return ( +
    + {tagsToDisplay.map((tag, i) => ( +
  • + +
    • + ))} + {displayMax && displayMax > 0 && numTags - 1 > 0 && ( + + {numTags - 1} + )} +
+ ); + }) +); TagList.displayName = 'TagList'; diff --git a/public/app/plugins/panel/annolist/AnnoListPanel.tsx b/public/app/plugins/panel/annolist/AnnoListPanel.tsx index c858d51171d..705377ce072 100644 --- a/public/app/plugins/panel/annolist/AnnoListPanel.tsx +++ b/public/app/plugins/panel/annolist/AnnoListPanel.tsx @@ -17,10 +17,10 @@ import { AbstractList } from '@grafana/ui/src/components/List/AbstractList'; import { getDashboardSrv } from 'app/features/dashboard/services/DashboardSrv'; import appEvents from 'app/core/app_events'; import { AnnotationListItem } from './AnnotationListItem'; -import { AnnotationListItemTags } from './AnnotationListItemTags'; -import { CustomScrollbar, stylesFactory } from '@grafana/ui'; +import { CustomScrollbar, stylesFactory, TagList } from '@grafana/ui'; import { css } from '@emotion/css'; import { Subscription } from 'rxjs'; +import { FocusScope } from '@react-aria/focus'; interface UserInfo { id?: number; @@ -39,6 +39,7 @@ interface State { export class AnnoListPanel extends PureComponent { style = getStyles(config.theme); subs = new Subscription(); + tagListRef = React.createRef(); constructor(props: Props) { super(props); @@ -181,9 +182,30 @@ export class AnnoListPanel extends PureComponent { } onTagClick = (tag: string, remove?: boolean) => { + if (!remove && this.state.queryTags.includes(tag)) { + return; + } + const queryTags = remove ? this.state.queryTags.filter((item) => item !== tag) : [...this.state.queryTags, tag]; - this.setState({ queryTags }); + // Logic to ensure keyboard focus isn't lost when the currently + // focused tag is removed + let nextTag: HTMLElement | undefined = undefined; + if (remove) { + const focusedTag = document.activeElement; + const dataTagId = focusedTag?.getAttribute('data-tag-id'); + if (this.tagListRef.current?.contains(focusedTag) && dataTagId) { + const parsedTagId = Number.parseInt(dataTagId, 10); + const possibleNextTag = + this.tagListRef.current.querySelector(`[data-tag-id="${parsedTagId + 1}"]`) ?? + this.tagListRef.current.querySelector(`[data-tag-id="${parsedTagId - 1}"]`); + if (possibleNextTag instanceof HTMLElement) { + nextTag = possibleNextTag; + } + } + } + + this.setState({ queryTags }, () => nextTag?.focus()); }; onUserClick = (anno: AnnotationEvent) => { @@ -202,10 +224,6 @@ export class AnnoListPanel extends PureComponent { }); }; - renderTags = (tags?: string[], remove?: boolean): JSX.Element | null => { - return ; - }; - renderItem = (anno: AnnotationEvent, index: number): JSX.Element => { const { options } = this.props; const dashboard = getDashboardSrv().getCurrent(); @@ -242,14 +260,25 @@ export class AnnoListPanel extends PureComponent { return ( {hasFilter && ( -
- Filter:   +
+ Filter: {queryUser && ( {queryUser.email} )} - {queryTags.length > 0 && this.renderTags(queryTags, true)} + {queryTags.length > 0 && ( + + this.onTagClick(tag, true)} + getAriaLabel={(name) => `Remove ${name} tag`} + className={this.style.tagList} + ref={this.tagListRef} + /> + + )}
)} @@ -269,4 +298,17 @@ const getStyles = stylesFactory((theme: GrafanaTheme) => ({ width: 100%; height: calc(100% - 30px); `, + filter: css({ + display: 'flex', + padding: `0px ${theme.spacing.xs}`, + b: { + paddingRight: theme.spacing.sm, + }, + }), + tagList: css({ + justifyContent: 'flex-start', + 'li > button': { + paddingLeft: '3px', + }, + }), })); diff --git a/public/app/plugins/panel/annolist/AnnotationListItem.tsx b/public/app/plugins/panel/annolist/AnnotationListItem.tsx index 7bc1ab178c5..43a5d3a0e32 100644 --- a/public/app/plugins/panel/annolist/AnnotationListItem.tsx +++ b/public/app/plugins/panel/annolist/AnnotationListItem.tsx @@ -1,9 +1,8 @@ import React, { FC, MouseEvent } from 'react'; -import { css, cx } from '@emotion/css'; +import { css } from '@emotion/css'; import { AnnotationEvent, DateTimeInput, GrafanaTheme2, PanelProps } from '@grafana/data'; -import { styleMixins, Tooltip, useStyles2 } from '@grafana/ui'; +import { Card, TagList, Tooltip, useStyles2 } from '@grafana/ui'; import { AnnoOptions } from './types'; -import { AnnotationListItemTags } from './AnnotationListItemTags'; interface Props extends Pick, 'options'> { annotation: AnnotationEvent; @@ -24,8 +23,7 @@ export const AnnotationListItem: FC = ({ const styles = useStyles2(getStyles); const { showUser, showTags, showTime } = options; const { text, login, email, avatarUrl, tags, time, timeEnd } = annotation; - const onItemClick = (e: MouseEvent) => { - e.stopPropagation(); + const onItemClick = () => { onClick(annotation); }; const onLoginClick = () => { @@ -36,20 +34,32 @@ export const AnnotationListItem: FC = ({ const showTimeStampEnd = timeEnd && timeEnd !== time && showTime; return ( -
- -
- {text} - {showTimeStamp ? : null} - {showTimeStampEnd ? - : null} - {showTimeStampEnd ? : null} -
-
- {showAvatar ? : null} - {showTags ? : null} -
- -
+ + + {text} + + {showTimeStamp && ( + + + {showTimeStampEnd && ( + <> + - + {' '} + + )} + + )} + {showAvatar && ( + + + + )} + {showTags && tags && ( + + onTagClick(tag, false)} /> + + )} + ); }; @@ -74,13 +84,11 @@ const Avatar: FC = ({ onClick, avatarUrl, login, email }) => { ); return ( -
- - - avatar icon - - -
+ + + ); }; @@ -101,48 +109,38 @@ const TimeStamp: FC = ({ time, formatDate }) => { function getStyles(theme: GrafanaTheme2) { return { - pointer: css` - cursor: pointer; - `, - item: css` - margin: ${theme.spacing(0.5)}; - padding: ${theme.spacing(1)}; - ${styleMixins.listItem(theme)}// display: flex; - `, - title: css` - flex-basis: 80%; - `, - link: css` - display: flex; - - .fa { - padding-top: ${theme.spacing(0.5)}; - } - - .fa-star { - color: ${theme.v1.palette.orange}; - } - `, - login: css` - align-self: center; - flex: auto; - display: flex; - justify-content: flex-end; - font-size: ${theme.typography.bodySmall.fontSize}; - `, - time: css` - margin-left: ${theme.spacing(1)}; - margin-right: ${theme.spacing(1)} - font-size: ${theme.typography.bodySmall.fontSize}; - color: ${theme.colors.text.secondary}; - `, - avatar: css` - padding: ${theme.spacing(0.5)}; - img { - border-radius: 50%; - width: ${theme.spacing(2)}; - height: ${theme.spacing(2)}; - } - `, + card: css({ + gridTemplateAreas: `"Heading Description Meta Tags"`, + gridTemplateColumns: 'auto 1fr auto auto', + padding: theme.spacing(1), + margin: theme.spacing(0.5), + width: 'inherit', + }), + meta: css({ + margin: 0, + position: 'relative', + justifyContent: 'end', + }), + timestamp: css({ + margin: 0, + alignSelf: 'center', + }), + time: css({ + marginLeft: theme.spacing(1), + marginRight: theme.spacing(1), + fontSize: theme.typography.bodySmall.fontSize, + color: theme.colors.text.secondary, + }), + avatar: css({ + border: 'none', + background: 'inherit', + margin: 0, + padding: theme.spacing(0.5), + img: { + borderRadius: '50%', + width: theme.spacing(2), + height: theme.spacing(2), + }, + }), }; } diff --git a/public/app/plugins/panel/annolist/AnnotationListItemTags.tsx b/public/app/plugins/panel/annolist/AnnotationListItemTags.tsx deleted file mode 100644 index d8469a06102..00000000000 --- a/public/app/plugins/panel/annolist/AnnotationListItemTags.tsx +++ /dev/null @@ -1,49 +0,0 @@ -import React, { FC, MouseEvent, useCallback } from 'react'; - -import { GrafanaTheme } from '@grafana/data'; -import { css } from '@emotion/css'; -import { useStyles } from '@grafana/ui'; - -import { TagBadge } from '../../../core/components/TagFilter/TagBadge'; - -interface Props { - tags?: string[]; - remove?: boolean; - onClick: (tag: string, remove?: boolean) => void; -} - -export const AnnotationListItemTags: FC = ({ tags, remove, onClick }) => { - const styles = useStyles(getStyles); - const onTagClicked = useCallback( - (e: MouseEvent, tag: string) => { - e.stopPropagation(); - onClick(tag, remove); - }, - [onClick, remove] - ); - - if (!tags || !tags.length) { - return null; - } - - return ( - <> - {tags.map((tag) => { - return ( - onTagClicked(e, tag)} className={styles.pointer}> - - - ); - })} - - ); -}; - -function getStyles(theme: GrafanaTheme) { - return { - pointer: css` - cursor: pointer; - padding: ${theme.spacing.xxs}; - `, - }; -}