Code Insights: Improve dashboard card Voice Over navigation (#43970)

* Update react-grid-layout and its types packages

* Move out grid related styles from the main css entry point

* Revert "Move out grid related styles from the main css entry point"

This reverts commit edc6965819.

* Improve dashboard items aria roles

* Run yarn format

* Fix insight card title labels

* Run yarn dedupe

* Fix focus management integration tests

* Adjust insight card role attribute

* Revert changes about specific li element attributes/props

* Fix insight card aria label
This commit is contained in:
Vova Kulikov 2022-11-08 11:33:53 -03:00 committed by GitHub
parent 17b9276e3f
commit fbff34be5e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 104 additions and 96 deletions

View File

@ -1,4 +1,4 @@
import React, { forwardRef, useEffect } from 'react'
import { HTMLAttributes, forwardRef, useEffect } from 'react'
import { useMergeRefs } from 'use-callback-ref'
@ -10,17 +10,17 @@ import { Insight, isBackendInsight } from '../../../core'
import { BackendInsightView } from './backend-insight/BackendInsight'
import { BuiltInInsight } from './built-in-insight/BuiltInInsight'
export interface SmartInsightProps extends TelemetryProps, React.HTMLAttributes<HTMLElement> {
export interface SmartInsightProps extends TelemetryProps, HTMLAttributes<HTMLElement> {
insight: Insight
resizing?: boolean
}
/**
* Render smart insight with (gql or extension api) fetcher and independent mutation
* Render smart insight with ф(gql or extension api) fetcher and independent mutation
* actions.
*/
export const SmartInsight = forwardRef<HTMLElement, SmartInsightProps>((props, reference) => {
const { insight, resizing = false, telemetryService, ...otherProps } = props
const { insight, resizing = false, telemetryService, ...attributes } = props
const mergedReference = useMergeRefs([reference])
const search = useSearchParameters()
@ -41,7 +41,7 @@ export const SmartInsight = forwardRef<HTMLElement, SmartInsightProps>((props, r
insight={insight}
resizing={resizing}
telemetryService={telemetryService}
{...otherProps}
{...attributes}
/>
)
}
@ -49,11 +49,11 @@ export const SmartInsight = forwardRef<HTMLElement, SmartInsightProps>((props, r
// Lang-stats insight is handled by built-in fetchers
return (
<BuiltInInsight
innerRef={mergedReference}
insight={insight}
resizing={resizing}
telemetryService={telemetryService}
innerRef={mergedReference}
{...otherProps}
{...attributes}
/>
)
})

View File

@ -41,7 +41,7 @@ interface BackendInsightProps extends TelemetryProps, HTMLAttributes<HTMLElement
}
export const BackendInsightView = forwardRef<HTMLElement, BackendInsightProps>((props, ref) => {
const { telemetryService, insight, resizing, children, ...otherProps } = props
const { telemetryService, insight, resizing, children, className, ...attributes } = props
const { currentDashboard, dashboards } = useContext(InsightContext)
const { createInsight, updateInsight } = useContext(CodeInsightsBackendContext)
@ -50,9 +50,11 @@ export const BackendInsightView = forwardRef<HTMLElement, BackendInsightProps>((
const { wasEverVisible, isVisible } = useVisibility(cardElementRef)
const seriesToggleState = useSeriesToggle()
// Original insight filters values that are stored in setting subject with insight
// configuration object, They are updated whenever the user clicks update/save button
const [originalInsightFilters, setOriginalInsightFilters] = useState(insight.filters)
// Live valid filters from filter form. They are updated whenever the user is changing
// filter value in filters fields.
const [filters, setFilters] = useState<InsightFilters>(originalInsightFilters)
@ -160,13 +162,14 @@ export const BackendInsightView = forwardRef<HTMLElement, BackendInsightProps>((
return (
<InsightCard
{...otherProps}
role="listitem"
ref={cardElementRef}
data-testid={`insight-card.${insight.id}`}
aria-label="Insight card"
className={classNames(otherProps.className, { [styles.cardWithFilters]: isFiltersOpen })}
aria-label={`${insight.title} insight`}
className={classNames(className, { [styles.cardWithFilters]: isFiltersOpen })}
onMouseEnter={trackMouseEnter}
onMouseLeave={trackMouseLeave}
{...attributes}
>
<InsightCardHeader
title={
@ -174,7 +177,6 @@ export const BackendInsightView = forwardRef<HTMLElement, BackendInsightProps>((
to={`${window.location.origin}/insights/insight/${insight.id}`}
target="_blank"
rel="noopener noreferrer"
aria-label="Go to the insight page"
>
{insight.title}
</Link>

View File

@ -1,10 +1,10 @@
import React, { Ref, useContext, useMemo, useRef, useState } from 'react'
import React, { Ref, useContext, useMemo, useState } from 'react'
import { useMergeRefs } from 'use-callback-ref'
import { ErrorAlert } from '@sourcegraph/branded/src/components/alerts'
import { TelemetryProps } from '@sourcegraph/shared/src/telemetry/telemetryService'
import { Link, useDeepMemo, ParentSize } from '@sourcegraph/wildcard'
import { Link, ParentSize, useDeepMemo } from '@sourcegraph/wildcard'
import { useSeriesToggle } from '../../../../../../insights/utils/use-series-toggle'
import { CodeInsightsBackendContext, LangStatsInsight } from '../../../../core'
@ -43,19 +43,16 @@ interface BuiltInInsightProps extends TelemetryProps, React.HTMLAttributes<HTMLE
* main work thread instead of using Extension API.
*/
export function BuiltInInsight(props: BuiltInInsightProps): React.ReactElement {
const { insight, resizing, telemetryService, innerRef, ...otherProps } = props
const { insight, resizing, telemetryService, innerRef, children, ...attributes } = props
const { getBuiltInInsightData } = useContext(CodeInsightsBackendContext)
const { currentDashboard, dashboards } = useContext(InsightContext)
const seriesToggleState = useSeriesToggle()
const insightCardReference = useRef<HTMLDivElement>(null)
const mergedInsightCardReference = useMergeRefs([insightCardReference, innerRef])
const cardRef = useMergeRefs([innerRef])
const cachedInsight = useDeepMemo(insight)
const { state, isVisible } = useInsightData(
useMemo(() => () => getBuiltInInsightData({ insight: cachedInsight }), [getBuiltInInsightData, cachedInsight]),
insightCardReference
cardRef
)
// Visual line chart settings
@ -68,12 +65,13 @@ export function BuiltInInsight(props: BuiltInInsightProps): React.ReactElement {
return (
<InsightCard
{...otherProps}
ref={mergedInsightCardReference}
role="listitem"
ref={cardRef}
data-testid={`insight-card.${insight.id}`}
aria-label="Insight card"
aria-label={`${insight.title} insight`}
onMouseEnter={trackMouseEnter}
onMouseLeave={trackMouseLeave}
{...attributes}
>
<InsightCardHeader
title={
@ -81,7 +79,6 @@ export function BuiltInInsight(props: BuiltInInsightProps): React.ReactElement {
to={`${window.location.origin}/insights/insight/${insight.id}`}
target="_blank"
rel="noopener noreferrer"
aria-label="Go to the insight page"
>
{insight.title}
</Link>
@ -140,7 +137,7 @@ export function BuiltInInsight(props: BuiltInInsightProps): React.ReactElement {
{
// Passing children props explicitly to render any top-level content like
// resize-handler from the react-grid-layout library
isVisible && otherProps.children
isVisible && children
}
</InsightCard>
)

View File

@ -1,34 +1,30 @@
import React, { PropsWithChildren, useCallback, useMemo } from 'react'
import { FC, PropsWithChildren, useCallback, useLayoutEffect, useMemo, useRef } from 'react'
import classNames from 'classnames'
import { noop } from 'lodash'
import {
Layout,
Layout as ReactGridLayout,
Layouts,
Layouts as ReactGridLayouts,
Responsive,
WidthProvider,
Responsive as ResponsiveGridLayout,
} from 'react-grid-layout'
import { isFirefox } from '@sourcegraph/common'
import { useMeasure } from '@sourcegraph/wildcard'
import styles from './ViewGrid.module.scss'
// TODO use a method to get width that also triggers when file explorer is closed
// (WidthProvider only listens to window resize events)
const ResponsiveGridLayout = WidthProvider(Responsive)
export const BREAKPOINTS_NAMES = ['xs', 'sm', 'md', 'lg'] as const
export type BreakpointName = typeof BREAKPOINTS_NAMES[number]
/** Minimum size in px after which a breakpoint is active. */
export const BREAKPOINTS: Record<BreakpointName, number> = { xs: 0, sm: 576, md: 768, lg: 992 } // no xl because TreePage's max-width is the xl breakpoint.
export const BREAKPOINTS: Record<BreakpointName, number> = { xs: 0, sm: 576, md: 768, lg: 992 }
export const COLUMNS: Record<BreakpointName, number> = { xs: 1, sm: 6, md: 8, lg: 12 }
export const DEFAULT_ITEMS_PER_ROW: Record<BreakpointName, number> = { xs: 1, sm: 2, md: 2, lg: 3 }
export const MIN_WIDTHS: Record<BreakpointName, number> = { xs: 1, sm: 2, md: 3, lg: 3 }
export const DEFAULT_HEIGHT = 3.25
export const ROW_HEIGHT = 6 * 16 // 6rem
export const CONTAINER_PADDING: [number, number] = [0, 0]
export const GRID_MARGIN: [number, number] = [12, 12]
const DEFAULT_VIEWS_LAYOUT_GENERATOR = (viewIds: string[]): ReactGridLayouts =>
Object.fromEntries(
@ -64,9 +60,7 @@ export type ViewGridProps =
layouts?: never
}
| {
/**
* Sets custom layout for react-grid-layout library.
*/
/** Sets custom layout for react-grid-layout library. */
layouts: ReactGridLayouts
viewIds?: never
}
@ -75,21 +69,17 @@ interface ViewGridCommonProps {
/** Custom classname for root element of the grid. */
className?: string
onLayoutChange?: (currentLayout: Layout[], allLayouts: Layouts) => void
onResizeStart?: (newItem: Layout) => void
onResizeStop?: (newItem: Layout) => void
onDragStart?: (newItem: Layout) => void
onLayoutChange?: (currentLayout: ReactGridLayout[], allLayouts: ReactGridLayouts) => void
onResizeStart?: (newItem: ReactGridLayout) => void
onResizeStop?: (newItem: ReactGridLayout) => void
onDragStart?: (newItem: ReactGridLayout) => void
}
/**
* Renders drag and drop and resizable views grid.
*/
export const ViewGrid: React.FunctionComponent<
React.PropsWithChildren<PropsWithChildren<ViewGridProps & ViewGridCommonProps>>
> = props => {
/** Renders drag and drop and resizable views grid. */
export const ViewGrid: FC<PropsWithChildren<ViewGridProps & ViewGridCommonProps>> = props => {
const {
layouts,
viewIds = [],
viewIds,
children,
className,
onLayoutChange,
@ -98,6 +88,11 @@ export const ViewGrid: React.FunctionComponent<
onDragStart = noop,
} = props
const gridRef = useRef<HTMLDivElement>(null)
const [, { width }] = useMeasure(gridRef.current)
const gridLayouts = useMemo(() => layouts ?? DEFAULT_VIEWS_LAYOUT_GENERATOR(viewIds), [layouts, viewIds])
const handleResizeStart: ReactGridLayout.ItemCallback = useCallback(
(_layout, item, newItem) => onResizeStart(newItem),
[onResizeStart]
@ -122,25 +117,32 @@ export const ViewGrid: React.FunctionComponent<
// Back to css transforms when this bug will be resolved in Firefox.
const useCSSTransforms = useMemo(() => !isFirefox(), [])
useLayoutEffect(() => {
// React grid layout doesn't expose API in order to override rendered elements
// (like as='ul' prop). Internally it always renders div element, we can't just
// render UL element, so we have to tune aria-role attribute manually here
gridRef.current?.setAttribute('role', 'list')
}, [])
return (
<div className={classNames(className, styles.viewGrid)}>
<ResponsiveGridLayout
measureBeforeMount={true}
breakpoints={BREAKPOINTS}
layouts={layouts ?? DEFAULT_VIEWS_LAYOUT_GENERATOR(viewIds)}
cols={COLUMNS}
autoSize={true}
rowHeight={6 * 16}
containerPadding={[0, 0]}
useCSSTransforms={useCSSTransforms}
margin={[12, 12]}
onResizeStart={handleResizeStart}
onResizeStop={handleResizeStop}
onLayoutChange={onLayoutChange}
onDragStart={handleDragStart}
>
{children}
</ResponsiveGridLayout>
</div>
<ResponsiveGridLayout
breakpoints={BREAKPOINTS}
cols={COLUMNS}
rowHeight={ROW_HEIGHT}
containerPadding={CONTAINER_PADDING}
margin={GRID_MARGIN}
innerRef={gridRef}
width={width}
autoSize={true}
layouts={gridLayouts}
useCSSTransforms={useCSSTransforms}
onResizeStart={handleResizeStart}
onResizeStop={handleResizeStop}
onDragStart={handleDragStart}
onLayoutChange={onLayoutChange}
className={classNames(className, styles.viewGrid)}
>
{width && children}
</ResponsiveGridLayout>
)
}

View File

@ -44,12 +44,12 @@ describe('Code insights [Insight Card] should has a proper focus management ', (
await driver.page.goto(driver.sourcegraphBaseUrl + '/insights/dashboards/DASHBOARD_WITH_SEARCH')
await driver.page.waitForSelector('[aria-label="Insight card"]')
await driver.page.focus('[aria-label="Insight card"]')
await driver.page.waitForSelector('[aria-label="Search Based insight"]')
await driver.page.focus('[aria-label="Search Based insight"]')
await driver.page.keyboard.press(Key.Tab)
assert.strictEqual(
await hasFocus(driver, '[aria-label="Go to the insight page"]'),
await hasFocus(driver, '[aria-label="Search Based insight"] h2 a'),
true,
'Insight title should be focused'
)
@ -98,12 +98,12 @@ describe('Code insights [Insight Card] should has a proper focus management ', (
await driver.page.goto(driver.sourcegraphBaseUrl + '/insights/dashboards/DASHBOARD_WITH_LANG_INSIGHT')
await driver.page.waitForSelector('[aria-label="Insight card"]')
await driver.page.focus('[aria-label="Insight card"]')
await driver.page.waitForSelector('[aria-label="Lang Stats insight"]')
await driver.page.focus('[aria-label="Lang Stats insight"]')
await driver.page.keyboard.press(Key.Tab)
assert.strictEqual(
await hasFocus(driver, '[aria-label="Go to the insight page"]'),
await hasFocus(driver, '[aria-label="Lang Stats insight"] h2 a'),
true,
'Insight title should be focused'
)

View File

@ -208,7 +208,7 @@
"@types/react-calendar": "^3.5.2",
"@types/react-circular-progressbar": "1.0.2",
"@types/react-dom": "18.0.2",
"@types/react-grid-layout": "1.3.0",
"@types/react-grid-layout": "1.3.2",
"@types/react-router": "^5.1.18",
"@types/react-router-dom": "^5.3.3",
"@types/recharts": "1.8.23",
@ -459,7 +459,7 @@
"react-dom-confetti": "^0.1.4",
"react-elm-components": "^1.1.0",
"react-focus-lock": "^2.7.1",
"react-grid-layout": "1.3.0",
"react-grid-layout": "1.3.4",
"react-resizable": "^3.0.4",
"react-router": "^5.2.0",
"react-router-dom": "^5.2.0",

View File

@ -8397,12 +8397,12 @@ __metadata:
languageName: node
linkType: hard
"@types/react-grid-layout@npm:1.3.0":
version: 1.3.0
resolution: "@types/react-grid-layout@npm:1.3.0"
"@types/react-grid-layout@npm:1.3.2":
version: 1.3.2
resolution: "@types/react-grid-layout@npm:1.3.2"
dependencies:
"@types/react": "*"
checksum: 94243bce2c7005dafad165f6f9329e4e77e09eed3ab8380b010474d75e4314856da43ff006f90c26a3f69e3d5398bbec145dbbd8619361abbc068ed413b791a6
checksum: 190492acb69186c651bb99f19028dcd4c65129eae7de6efd41f51b6a6711af490e7b99ad7156b8731b11ecf2ec7c22bcf13c782bfe65be4b4e5c3362b97095bf
languageName: node
linkType: hard
@ -12307,7 +12307,7 @@ __metadata:
languageName: node
linkType: hard
"classnames@npm:2.3.1, classnames@npm:^2.2.5, classnames@npm:^2.2.6, classnames@npm:^2.3.1":
"classnames@npm:^2.2.5, classnames@npm:^2.2.6, classnames@npm:^2.3.1":
version: 2.3.1
resolution: "classnames@npm:2.3.1"
checksum: 14db8889d56c267a591f08b0834989fe542d47fac659af5a539e110cc4266694e8de86e4e3bbd271157dbd831361310a8293e0167141e80b0f03a0f175c80960
@ -12561,6 +12561,13 @@ __metadata:
languageName: node
linkType: hard
"clsx@npm:^1.1.1":
version: 1.2.1
resolution: "clsx@npm:1.2.1"
checksum: 30befca8019b2eb7dbad38cff6266cf543091dae2825c856a62a8ccf2c3ab9c2907c4d12b288b73101196767f66812365400a227581484a05f968b0307cfaf12
languageName: node
linkType: hard
"co@npm:^4.6.0":
version: 4.6.0
resolution: "co@npm:4.6.0"
@ -26674,14 +26681,14 @@ __metadata:
languageName: node
linkType: hard
"prop-types@npm:15.x, prop-types@npm:^15.0.0, prop-types@npm:^15.5.10, prop-types@npm:^15.5.8, prop-types@npm:^15.6.0, prop-types@npm:^15.6.1, prop-types@npm:^15.6.2, prop-types@npm:^15.7.2":
version: 15.7.2
resolution: "prop-types@npm:15.7.2"
"prop-types@npm:15.x, prop-types@npm:^15.0.0, prop-types@npm:^15.5.10, prop-types@npm:^15.5.8, prop-types@npm:^15.6.0, prop-types@npm:^15.6.1, prop-types@npm:^15.6.2, prop-types@npm:^15.7.2, prop-types@npm:^15.8.1":
version: 15.8.1
resolution: "prop-types@npm:15.8.1"
dependencies:
loose-envify: ^1.4.0
object-assign: ^4.1.1
react-is: ^16.8.1
checksum: 5eef82fdda64252c7e75aa5c8cc28a24bbdece0f540adb60ce67c205cf978a5bd56b83e4f269f91c6e4dcfd80b36f2a2dec24d362e278913db2086ca9c6f9430
react-is: ^16.13.1
checksum: c056d3f1c057cb7ff8344c645450e14f088a915d078dcda795041765047fa080d38e5d626560ccaac94a4e16e3aa15f3557c1a9a8d1174530955e992c675e459
languageName: node
linkType: hard
@ -27173,19 +27180,19 @@ pvutils@latest:
languageName: node
linkType: hard
"react-grid-layout@npm:1.3.0":
version: 1.3.0
resolution: "react-grid-layout@npm:1.3.0"
"react-grid-layout@npm:1.3.4":
version: 1.3.4
resolution: "react-grid-layout@npm:1.3.4"
dependencies:
classnames: 2.3.1
clsx: ^1.1.1
lodash.isequal: ^4.0.0
prop-types: ^15.0.0
prop-types: ^15.8.1
react-draggable: ^4.0.0
react-resizable: ^3.0.4
peerDependencies:
react: ">= 16.3.0"
react-dom: ">= 16.3.0"
checksum: 955c135dcc14e86f573141e75b211e08e68f1d4f4ea2d0849652410dfc41715d89507abf1d28c2de4326b201f2e4138a2c2796561e917a2ee8b932adf40864bb
checksum: f56c8c452acd9588edf1dc6996a4ea14d9f669d77f6b2ebd50146eaeeb9325c83f5ca44b66bac2b8c24f9cb2ec7ed49396350435991255c3b31e21b8a2e3d243
languageName: node
linkType: hard
@ -27220,7 +27227,7 @@ pvutils@latest:
languageName: node
linkType: hard
"react-is@npm:^16.6.0, react-is@npm:^16.7.0, react-is@npm:^16.8.1, react-is@npm:^16.8.6, react-is@npm:^16.9.0":
"react-is@npm:^16.13.1, react-is@npm:^16.6.0, react-is@npm:^16.7.0, react-is@npm:^16.8.6, react-is@npm:^16.9.0":
version: 16.13.1
resolution: "react-is@npm:16.13.1"
checksum: f7a19ac3496de32ca9ae12aa030f00f14a3d45374f1ceca0af707c831b2a6098ef0d6bdae51bd437b0a306d7f01d4677fcc8de7c0d331eb47ad0f46130e53c5f
@ -28699,7 +28706,7 @@ pvutils@latest:
"@types/react-calendar": ^3.5.2
"@types/react-circular-progressbar": 1.0.2
"@types/react-dom": 18.0.2
"@types/react-grid-layout": 1.3.0
"@types/react-grid-layout": 1.3.2
"@types/react-resizable": ^3.0.2
"@types/react-router": ^5.1.18
"@types/react-router-dom": ^5.3.3
@ -28881,7 +28888,7 @@ pvutils@latest:
react-dom-confetti: ^0.1.4
react-elm-components: ^1.1.0
react-focus-lock: ^2.7.1
react-grid-layout: 1.3.0
react-grid-layout: 1.3.4
react-refresh: ^0.10.0
react-resizable: ^3.0.4
react-router: ^5.2.0