From 44f15ee274d60f68aba41e56eb9e427d8e6e84e7 Mon Sep 17 00:00:00 2001 From: Blake Vandercar Date: Wed, 11 Sep 2024 12:40:06 -0600 Subject: [PATCH 01/10] feat(a11y): change role structure to tree/treeitems, change keypress actions to match. --- src/DataRenderer.test.tsx | 172 ++++++++++++++++++-------------------- src/DataRenderer.tsx | 117 +++++++++++++++++--------- src/index.test.tsx | 34 +++++++- src/index.tsx | 20 ++++- 4 files changed, 209 insertions(+), 134 deletions(-) diff --git a/src/DataRenderer.test.tsx b/src/DataRenderer.test.tsx index 751a4bb..4871a4b 100644 --- a/src/DataRenderer.test.tsx +++ b/src/DataRenderer.test.tsx @@ -4,7 +4,7 @@ import { allExpanded, collapseAllNested, defaultStyles } from './index'; import { render, screen, fireEvent } from '@testing-library/react'; import '@testing-library/jest-dom'; -const commonProps: JsonRenderProps = { +const commonProps: Omit, 'outerRef'> = { lastElement: false, level: 0, style: { @@ -30,20 +30,32 @@ const commonProps: JsonRenderProps = { field: undefined }; +const WrappedDataRenderer = (testProps: Partial>) => { + const ref = React.useRef(null); + return ( +
+ +
+ ); +}; + const collapseAll = () => false; const testButtonsCollapsed = () => { const buttons = screen.getAllByRole('button', { hidden: true }); - expect(buttons.length).toBe(2); + expect(buttons).toHaveLength(1); expect(buttons[0]).toHaveClass('expand-icon-light'); - expect(buttons[1]).toHaveClass('collapsed-content-light'); + expect(buttons[0]).not.toHaveClass('collapse-icon-light'); + expect(buttons[0]).toHaveAttribute('aria-expanded', 'false'); return buttons; }; const testButtonsExpanded = () => { const buttons = screen.getAllByRole('button', { hidden: true }); - expect(buttons.length).toBe(1); + expect(buttons).toHaveLength(1); expect(buttons[0]).toHaveClass('collapse-icon-light'); + expect(buttons[0]).not.toHaveClass('expand-icon-light'); + expect(buttons[0]).toHaveAttribute('aria-expanded', 'true'); return buttons; }; @@ -53,39 +65,28 @@ const testButtonsIfEmpty = () => { }).toThrow(); }; -const testClickableNodeCollapsed = () => { - const buttons = screen.getAllByRole('button', { hidden: true }); - expect(buttons.length).toBe(4); - expect(buttons[0]).toHaveClass('collapse-icon-light'); - expect(buttons[1]).toHaveClass('expand-icon-light'); - expect(buttons[2]).toHaveClass('clickable-label-light'); - expect(buttons[3]).toHaveClass('collapsed-content-light'); - return buttons; -}; - describe('DataRender', () => { it('should render booleans: true', () => { - render(); + render(); expect(screen.getByText(/test:/)).toBeInTheDocument(); expect(screen.getByText('true')).toBeInTheDocument(); }); it('should render booleans: false', () => { - render(); + render(); expect(screen.getByText(/test:/)).toBeInTheDocument(); expect(screen.getByText('false')).toBeInTheDocument(); }); it('should render strings', () => { - render(); + render(); expect(screen.getByText(/test:/)).toBeInTheDocument(); expect(screen.getByText(`"string"`)).toBeInTheDocument(); }); it('should render strings without quotes', () => { render( - @@ -97,8 +98,7 @@ describe('DataRender', () => { it('should render strings with quotes if noQuotesForStringValues is undefined', () => { render( - @@ -109,8 +109,7 @@ describe('DataRender', () => { it('should render field names without quotes if quotesForFieldNames is undefined', () => { render( - @@ -120,8 +119,7 @@ describe('DataRender', () => { it('should render field names with quotes if quotesForFieldNames is true', () => { render( - @@ -130,120 +128,116 @@ describe('DataRender', () => { }); it('should render numbers', () => { - render(); + render(); expect(screen.getByText(/test:/)).toBeInTheDocument(); expect(screen.getByText('42')).toBeInTheDocument(); }); it('should render bigints', () => { - render(); + render(); expect(screen.getByText(/test:/)).toBeInTheDocument(); expect(screen.getByText('42n')).toBeInTheDocument(); }); it('should render dates', () => { - render(); + render(); expect(screen.getByText(/test:/)).toBeInTheDocument(); expect(screen.getByText('1970-01-01T00:00:00.000Z')).toBeInTheDocument(); }); it('should render nulls', () => { - render(); + render(); expect(screen.getByText(/test:/)).toBeInTheDocument(); expect(screen.getByText('null')).toBeInTheDocument(); }); it('should render undefineds', () => { - render(); + render(); expect(screen.getByText(/test:/)).toBeInTheDocument(); expect(screen.getByText('undefined')).toBeInTheDocument(); }); it('should render unknown types', () => { - render(); + render(); expect(screen.getByText(/test:/)).toBeInTheDocument(); expect(screen.getByText(/2020/)).toBeInTheDocument(); }); it('should render object with empty key string', () => { - render(); + render(); expect(screen.getByText(/"":/)).toBeInTheDocument(); expect(screen.getByText(/empty key/)).toBeInTheDocument(); }); it('should render empty objects', () => { - render(); + render(); expect(screen.getByText('{')).toBeInTheDocument(); expect(screen.getByText('}')).toBeInTheDocument(); }); it('should render nested empty objects', () => { - render(); + render(); expect(screen.getByText('nested:')).toBeInTheDocument(); expect(screen.getByText('{')).toBeInTheDocument(); expect(screen.getByText('}')).toBeInTheDocument(); }); it('should not expand empty objects', () => { - render(); + render(); testButtonsIfEmpty(); }); it('should not collapse empty objects', () => { - render(); + render(); testButtonsIfEmpty(); }); it('should render empty arrays', () => { - render(); + render(); expect(screen.getByText('[')).toBeInTheDocument(); expect(screen.getByText(']')).toBeInTheDocument(); }); it('should render nested empty arrays', () => { - render(); + render(); expect(screen.getByText('nested:')).toBeInTheDocument(); expect(screen.getByText('[')).toBeInTheDocument(); expect(screen.getByText(']')).toBeInTheDocument(); }); it('should not expand empty arrays', () => { - render(); + render(); testButtonsIfEmpty(); }); it('should not collapse empty arrays', () => { - render(); + render(); testButtonsIfEmpty(); }); it('should render arrays', () => { - render(); + render(); expect(screen.getByText('1')).toBeInTheDocument(); expect(screen.getByText('2')).toBeInTheDocument(); expect(screen.getByText('3')).toBeInTheDocument(); }); it('should render arrays with key', () => { - render(); + render(); expect(screen.getByText('1')).toBeInTheDocument(); expect(screen.getByText('2')).toBeInTheDocument(); expect(screen.getByText('3')).toBeInTheDocument(); }); it('should render nested objects', () => { - render(); + render(); expect(screen.getByText(/test:/)).toBeInTheDocument(); expect(screen.getByText('123')).toBeInTheDocument(); }); it('should render nested objects collapsed', () => { render( - + ); expect(screen.getByText(/obj/)).toBeInTheDocument(); expect(screen.queryByText(/test:/)).not.toBeInTheDocument(); @@ -252,53 +246,43 @@ describe('DataRender', () => { it('should render nested objects collapsed and expand it once property changed', () => { const { rerender } = render( - + ); expect(screen.getByText(/obj/)).toBeInTheDocument(); expect(screen.queryByText(/test:/)).not.toBeInTheDocument(); expect(screen.queryByText('123')).not.toBeInTheDocument(); - rerender( - - ); + rerender(); expect(screen.getByText(/obj/)).toBeInTheDocument(); expect(screen.queryByText(/test:/)).toBeInTheDocument(); expect(screen.queryByText('123')).toBeInTheDocument(); }); it('should render nested arrays collapsed', () => { - render( - - ); + render(); expect(screen.queryByText(/test:/)).toBeInTheDocument(); expect(screen.queryByText('123')).not.toBeInTheDocument(); }); it('should render nested arrays collapsed and expand it once property changed', () => { const { rerender } = render( - + ); expect(screen.queryByText(/test:/)).toBeInTheDocument(); expect(screen.queryByText('123')).not.toBeInTheDocument(); - rerender( - - ); + rerender(); expect(screen.queryByText(/test:/)).toBeInTheDocument(); expect(screen.queryByText('123')).toBeInTheDocument(); }); it('should render top arrays collapsed', () => { - render(); + render(); expect(screen.queryByText('123')).not.toBeInTheDocument(); }); it('should collapse and expand objects by clicking on icon', () => { - render(); + render(); expect(screen.getByText(/test:/)).toBeInTheDocument(); let buttons = testButtonsExpanded(); fireEvent.click(buttons[0]); @@ -309,9 +293,9 @@ describe('DataRender', () => { }); it('should collapse and expand objects by clicking on node', () => { - render( - @@ -320,9 +304,15 @@ describe('DataRender', () => { // open the 'test' node by clicking the icon expect(screen.queryByText(/test:/)).not.toBeInTheDocument(); expect(screen.queryByText(/child/)).not.toBeInTheDocument(); - const buttons = testButtonsCollapsed(); + let buttons = testButtonsCollapsed(); fireEvent.click(buttons[0]); - testClickableNodeCollapsed(); + + buttons = screen.getAllByRole('button', { hidden: true }); + expect(buttons.length).toBe(2); + expect(buttons[0]).toHaveClass('collapse-icon-light'); + expect(buttons[1]).toHaveClass('expand-icon-light'); + expect(container.getElementsByClassName('clickable-label-light')).toHaveLength(1); + expect(container.getElementsByClassName('collapsed-content-light')).toHaveLength(1); expect(screen.getByText(/test:/)).toBeInTheDocument(); expect(screen.queryByText(/child/)).not.toBeInTheDocument(); fireEvent.click(buttons[0]); @@ -331,16 +321,19 @@ describe('DataRender', () => { }); it('should expand objects by clicking on collapsed content', () => { - render(); + const { container } = render( + + ); expect(screen.queryByText(/test:/)).not.toBeInTheDocument(); - const buttons = testButtonsCollapsed(); - fireEvent.click(buttons[1]); + testButtonsCollapsed(); + const collapsedContent = container.getElementsByClassName(commonProps.style.collapsedContent); + fireEvent.click(collapsedContent[0]); testButtonsExpanded(); expect(screen.getByText(/test:/)).toBeInTheDocument(); }); it('should collapse and expand arrays by clicking on icon', () => { - render(); + render(); expect(screen.getByText('1')).toBeInTheDocument(); let buttons = testButtonsExpanded(); fireEvent.click(buttons[0]); @@ -351,53 +344,54 @@ describe('DataRender', () => { }); it('should expand arrays by clicking on collapsed content', () => { - render(); + const { container } = render( + + ); expect(screen.queryByText('1')).not.toBeInTheDocument(); - const buttons = testButtonsCollapsed(); - fireEvent.click(buttons[1]); + testButtonsCollapsed(); + const collapsedContent = container.getElementsByClassName(commonProps.style.collapsedContent); + fireEvent.click(collapsedContent[0]); testButtonsExpanded(); expect(screen.getByText('1')).toBeInTheDocument(); }); - it('should expand objects by pressing Spacebar on icon', () => { - render(); + it('should expand objects by pressing ArrowRight on icon', () => { + render(); expect(screen.queryByText(/test:/)).not.toBeInTheDocument(); const buttons = testButtonsCollapsed(); - fireEvent.keyDown(buttons[0], { key: ' ', code: 'Space' }); + fireEvent.keyDown(buttons[0], { key: 'ArrowRight', code: 'ArrowRight' }); testButtonsExpanded(); expect(screen.getByText(/test:/)).toBeInTheDocument(); }); it('should not expand objects by pressing other keys on icon', () => { - render(); + render(); expect(screen.queryByText(/test:/)).not.toBeInTheDocument(); const buttons = testButtonsCollapsed(); fireEvent.keyDown(buttons[0], { key: 'Enter', code: 'Enter' }); + fireEvent.keyDown(buttons[0], { key: ' ', code: 'Space' }); testButtonsCollapsed(); expect(screen.queryByText(/test:/)).not.toBeInTheDocument(); }); - it('should expand arrays by pressing Spacebar on icon', () => { - render( - - ); + it('should expand arrays by pressing ArrowRight on icon', () => { + render(); const buttons = testButtonsCollapsed(); expect(screen.queryByText(/test/)).not.toBeInTheDocument(); expect(screen.queryByText(/array/)).not.toBeInTheDocument(); - fireEvent.keyDown(buttons[0], { key: ' ', code: 'Space' }); + fireEvent.keyDown(buttons[0], { key: 'ArrowRight', code: 'ArrowRight' }); testButtonsExpanded(); expect(screen.getByText(/test/)).toBeInTheDocument(); expect(screen.getByText(/array/)).toBeInTheDocument(); }); it('should not expand arrays by pressing other keys on icon', () => { - render( - - ); + render(); const buttons = testButtonsCollapsed(); expect(screen.queryByText(/test:/)).not.toBeInTheDocument(); expect(screen.queryByText(/array/)).not.toBeInTheDocument(); fireEvent.keyDown(buttons[0], { key: 'Enter', code: 'Enter' }); + fireEvent.keyDown(buttons[0], { key: ' ', code: 'Space' }); testButtonsCollapsed(); expect(screen.queryByText(/test:/)).not.toBeInTheDocument(); expect(screen.queryByText(/array/)).not.toBeInTheDocument(); diff --git a/src/DataRenderer.tsx b/src/DataRenderer.tsx index bfa4cee..4ecda24 100644 --- a/src/DataRenderer.tsx +++ b/src/DataRenderer.tsx @@ -21,27 +21,26 @@ export interface StyleProps { quotesForFieldNames?: boolean; } -export interface JsonRenderProps { - field?: string; - value: T; +interface CommonRenderProps { lastElement: boolean; level: number; style: StyleProps; shouldExpandNode: (level: number, value: any, field?: string) => boolean; clickToExpandNode: boolean; + outerRef: React.RefObject; } -export interface ExpandableRenderProps { +export interface JsonRenderProps extends CommonRenderProps { + field?: string; + value: T; +} + +export interface ExpandableRenderProps extends CommonRenderProps { field: string | undefined; value: Array | object; data: Array<[string | undefined, any]>; openBracket: string; closeBracket: string; - lastElement: boolean; - level: number; - style: StyleProps; - shouldExpandNode: (level: number, value: any, field?: string) => boolean; - clickToExpandNode: boolean; } function quoteString(value: string, quoted = false) { @@ -52,6 +51,16 @@ function quoteString(value: string, quoted = false) { return value; } +export function getButtonElements(outerElement: HTMLDivElement) { + return Array.from(outerElement.querySelectorAll('[role=button]')); +} + +export function setButtonTabIndex(buttonElements: HTMLElement[], index: number) { + buttonElements.forEach((buttonElement, i) => { + buttonElement.tabIndex = i === index ? 0 : -1; + }); +} + function ExpandableObject({ field, value, @@ -62,12 +71,16 @@ function ExpandableObject({ level, style, shouldExpandNode, - clickToExpandNode + clickToExpandNode, + outerRef }: ExpandableRenderProps) { + // follows tree example for role structure and keypress actions: https://www.w3.org/WAI/ARIA/apg/patterns/treeview/examples/treeview-1a/ + const shouldExpandNodeCalledRef = React.useRef(false); const [expanded, toggleExpanded, setExpanded] = useBool(() => shouldExpandNode(level, value, field) ); + const expanderButtonRef = React.useRef(null); React.useEffect(() => { if (!shouldExpandNodeCalledRef.current) { @@ -85,33 +98,60 @@ function ExpandableObject({ const lastIndex = data.length - 1; const onKeyDown = (e: React.KeyboardEvent) => { - if (e.key === ' ') { + if ((e.key === 'ArrowRight' && !expanded) || (e.key === 'ArrowLeft' && expanded)) { e.preventDefault(); toggleExpanded(); } + + if (e.key === 'ArrowUp' || e.key === 'ArrowDown') { + e.preventDefault(); + const direction = e.key === 'ArrowUp' ? -1 : 1; + const outerElement = outerRef.current; + if (!outerElement) return; + const selectedElement = outerElement.querySelectorAll('[tabindex="0"]')[0]; + if (!selectedElement) return; + + const buttonElements = getButtonElements(outerElement); + const currentIndex = buttonElements.indexOf(selectedElement); + if (currentIndex < 0) return; + + const nextIndex = (currentIndex + direction + buttonElements.length) % buttonElements.length; // auto-wrap + setButtonTabIndex(buttonElements, nextIndex); + buttonElements[nextIndex].focus(); + } + }; + + const onClick = () => { + toggleExpanded(); + const outerElement = outerRef.current; + if (!outerElement) return; + const buttonElement = expanderButtonRef.current; + if (!buttonElement) return; + const buttonElements = getButtonElements(outerElement); + const currentIndex = buttonElements.indexOf(buttonElement); + setButtonTabIndex(buttonElements, currentIndex); }; return ( -
+
{(field || field === '') && (clickToExpandNode ? ( - + // don't apply role="button" or tabIndex even though has onClick, because has same + // function as the +/- expander button (so just expose that button to keyboard and a11y tree) + // eslint-disable-next-line jsx-a11y/no-static-element-interactions + {quoteString(field, style.quotesForFieldNames)}: ) : ( @@ -120,7 +160,7 @@ function ExpandableObject({ {openBracket} {expanded ? ( -
+
    {data.map((dataElement, index) => ( ))} -
+ ) : ( - + // don't apply role="button" or tabIndex even though has onClick, because has same + // function as the +/- expander button (so just expose that button to keyboard and a11y tree) + // eslint-disable-next-line jsx-a11y/no-static-element-interactions + )} {closeBracket} @@ -163,7 +198,7 @@ export interface EmptyRenderProps { function EmptyObject({ field, openBracket, closeBracket, lastElement, style }: EmptyRenderProps) { return ( -
+
{(field || field === '') && ( {quoteString(field, style.quotesForFieldNames)}: )} @@ -181,7 +216,8 @@ function JsonObject({ lastElement, shouldExpandNode, clickToExpandNode, - level + level, + outerRef }: JsonRenderProps) { if (Object.keys(value).length === 0) { return EmptyObject({ @@ -203,7 +239,8 @@ function JsonObject({ style, shouldExpandNode, clickToExpandNode, - data: Object.keys(value).map((key) => [key, value[key as keyof typeof value]]) + data: Object.keys(value).map((key) => [key, value[key as keyof typeof value]]), + outerRef }); } @@ -214,7 +251,8 @@ function JsonArray({ lastElement, level, shouldExpandNode, - clickToExpandNode + clickToExpandNode, + outerRef }: JsonRenderProps>) { if (value.length === 0) { return EmptyObject({ @@ -236,7 +274,8 @@ function JsonArray({ style, shouldExpandNode, clickToExpandNode, - data: value.map((element) => [undefined, element]) + data: value.map((element) => [undefined, element]), + outerRef }); } @@ -274,7 +313,7 @@ function JsonPrimitiveValue({ } return ( -
+
{(field || field === '') && ( {quoteString(field, style.quotesForFieldNames)}: )} diff --git a/src/index.test.tsx b/src/index.test.tsx index 3dedf65..e0933e7 100644 --- a/src/index.test.tsx +++ b/src/index.test.tsx @@ -1,7 +1,8 @@ import * as React from 'react'; import { JsonView, defaultStyles, allExpanded, collapseAllNested } from '.'; -import { render, screen } from '@testing-library/react'; +import { fireEvent, render, screen } from '@testing-library/react'; import '@testing-library/jest-dom'; +import { StyleProps } from './DataRenderer'; describe('JsonView', () => { it('should render object', () => { @@ -17,7 +18,7 @@ describe('JsonView', () => { }); it('should render object with incomplete style object', () => { - render(); + render(); expect(screen.getByText(/test/)).toBeInTheDocument(); expect(screen.getByText('true')).toBeInTheDocument(); }); @@ -44,4 +45,33 @@ describe('JsonView', () => { expect(collapseAllNested(2)).toBeFalsy(); expect(collapseAllNested(3)).toBeFalsy(); }); + + it('should go to next node on ArrowDown, prev node with ArrowUp, tabindex should change', () => { + render(); + const buttons = screen.getAllByRole('button', { hidden: true }); + expect(buttons).toHaveLength(3); + + expect(buttons[0].tabIndex).toEqual(0); + expect(buttons[1].tabIndex).toEqual(-1); + expect(buttons[2].tabIndex).toEqual(-1); + + buttons[0].focus(); + expect(buttons[0]).toHaveFocus(); + + fireEvent.keyDown(buttons[0], { key: 'ArrowDown', code: 'ArrowDown' }); + expect(buttons[0]).not.toHaveFocus(); + expect(buttons[1]).toHaveFocus(); + expect(buttons[2]).not.toHaveFocus(); + expect(buttons[0].tabIndex).toEqual(-1); + expect(buttons[1].tabIndex).toEqual(0); + expect(buttons[2].tabIndex).toEqual(-1); + + fireEvent.keyDown(buttons[1], { key: 'ArrowUp', code: 'ArrowUp' }); + expect(buttons[0]).toHaveFocus(); + expect(buttons[1]).not.toHaveFocus(); + expect(buttons[2]).not.toHaveFocus(); + expect(buttons[0].tabIndex).toEqual(0); + expect(buttons[1].tabIndex).toEqual(-1); + expect(buttons[2].tabIndex).toEqual(-1); + }); }); diff --git a/src/index.tsx b/src/index.tsx index 828d974..aa0b157 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -1,8 +1,8 @@ import * as React from 'react'; -import DataRender, { StyleProps } from './DataRenderer'; +import DataRender, { getButtonElements, setButtonTabIndex, StyleProps } from './DataRenderer'; import styles from './styles.module.css'; -export interface Props { +export interface Props extends React.AriaAttributes { data: Object | Array; style?: StyleProps; shouldExpandNode?: (level: number, value: any, field?: string) => boolean; @@ -54,10 +54,21 @@ export const JsonView = ({ data, style = defaultStyles, shouldExpandNode = allExpanded, - clickToExpandNode = false + clickToExpandNode = false, + ...ariaAttrs }: Props) => { + const outerRef = React.useRef(null); + + React.useEffect(() => { + const outerElement = outerRef.current; + if (!outerElement) return; + const buttonElements = getButtonElements(outerElement); + // on first render, set first button to tabIndex=0. + setButtonTabIndex(buttonElements, 0); + }, [outerRef]); + return ( -
+
); From c5acb3e888c2e9b936a01840ec1633908c072d8b Mon Sep 17 00:00:00 2001 From: Blake Vandercar Date: Wed, 11 Sep 2024 12:51:14 -0600 Subject: [PATCH 02/10] style: rename --- src/DataRenderer.tsx | 6 +++--- src/index.tsx | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/DataRenderer.tsx b/src/DataRenderer.tsx index 4ecda24..dd5c856 100644 --- a/src/DataRenderer.tsx +++ b/src/DataRenderer.tsx @@ -55,7 +55,7 @@ export function getButtonElements(outerElement: HTMLDivElement) { return Array.from(outerElement.querySelectorAll('[role=button]')); } -export function setButtonTabIndex(buttonElements: HTMLElement[], index: number) { +export function setTabbableButton(buttonElements: HTMLElement[], index: number) { buttonElements.forEach((buttonElement, i) => { buttonElement.tabIndex = i === index ? 0 : -1; }); @@ -116,7 +116,7 @@ function ExpandableObject({ if (currentIndex < 0) return; const nextIndex = (currentIndex + direction + buttonElements.length) % buttonElements.length; // auto-wrap - setButtonTabIndex(buttonElements, nextIndex); + setTabbableButton(buttonElements, nextIndex); buttonElements[nextIndex].focus(); } }; @@ -129,7 +129,7 @@ function ExpandableObject({ if (!buttonElement) return; const buttonElements = getButtonElements(outerElement); const currentIndex = buttonElements.indexOf(buttonElement); - setButtonTabIndex(buttonElements, currentIndex); + setTabbableButton(buttonElements, currentIndex); }; return ( diff --git a/src/index.tsx b/src/index.tsx index aa0b157..6cf7956 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -1,5 +1,5 @@ import * as React from 'react'; -import DataRender, { getButtonElements, setButtonTabIndex, StyleProps } from './DataRenderer'; +import DataRender, { getButtonElements, setTabbableButton, StyleProps } from './DataRenderer'; import styles from './styles.module.css'; export interface Props extends React.AriaAttributes { @@ -59,12 +59,12 @@ export const JsonView = ({ }: Props) => { const outerRef = React.useRef(null); + // on first render, set first button to tabIndex=0. React.useEffect(() => { const outerElement = outerRef.current; if (!outerElement) return; const buttonElements = getButtonElements(outerElement); - // on first render, set first button to tabIndex=0. - setButtonTabIndex(buttonElements, 0); + setTabbableButton(buttonElements, 0); }, [outerRef]); return ( From 7d38fa30cdfc74ad80d2799b5b3a8677e958d58d Mon Sep 17 00:00:00 2001 From: Blake Vandercar Date: Wed, 11 Sep 2024 14:01:48 -0600 Subject: [PATCH 03/10] add aria-expanded prop --- src/DataRenderer.tsx | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/DataRenderer.tsx b/src/DataRenderer.tsx index dd5c856..cddd4ee 100644 --- a/src/DataRenderer.tsx +++ b/src/DataRenderer.tsx @@ -133,7 +133,12 @@ function ExpandableObject({ }; return ( -
+
+
{(field || field === '') && ( {quoteString(field, style.quotesForFieldNames)}: )} @@ -313,7 +318,7 @@ function JsonPrimitiveValue({ } return ( -
+
{(field || field === '') && ( {quoteString(field, style.quotesForFieldNames)}: )} From 853422ccc48d49f1b88346eb6a27484905da1e79 Mon Sep 17 00:00:00 2001 From: Blake Vandercar Date: Wed, 11 Sep 2024 14:13:31 -0600 Subject: [PATCH 04/10] default aria-label --- src/index.tsx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/index.tsx b/src/index.tsx index 6cf7956..f461250 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -68,7 +68,13 @@ export const JsonView = ({ }, [outerRef]); return ( -
+
Date: Wed, 11 Sep 2024 14:40:28 -0600 Subject: [PATCH 05/10] refactor to shorten --- src/DataRenderer.tsx | 24 +++++++++++------------- src/index.tsx | 5 ++--- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/src/DataRenderer.tsx b/src/DataRenderer.tsx index cddd4ee..d8b369e 100644 --- a/src/DataRenderer.tsx +++ b/src/DataRenderer.tsx @@ -51,7 +51,9 @@ function quoteString(value: string, quoted = false) { return value; } -export function getButtonElements(outerElement: HTMLDivElement) { +export function getButtonElements(outerRef: React.RefObject) { + const outerElement = outerRef.current; + if (!outerElement) return; return Array.from(outerElement.querySelectorAll('[role=button]')); } @@ -106,13 +108,10 @@ function ExpandableObject({ if (e.key === 'ArrowUp' || e.key === 'ArrowDown') { e.preventDefault(); const direction = e.key === 'ArrowUp' ? -1 : 1; - const outerElement = outerRef.current; - if (!outerElement) return; - const selectedElement = outerElement.querySelectorAll('[tabindex="0"]')[0]; - if (!selectedElement) return; - const buttonElements = getButtonElements(outerElement); - const currentIndex = buttonElements.indexOf(selectedElement); + const buttonElements = getButtonElements(outerRef); + if (!buttonElements) return; + const currentIndex = buttonElements.findIndex((el) => el.tabIndex === 0); if (currentIndex < 0) return; const nextIndex = (currentIndex + direction + buttonElements.length) % buttonElements.length; // auto-wrap @@ -123,12 +122,11 @@ function ExpandableObject({ const onClick = () => { toggleExpanded(); - const outerElement = outerRef.current; - if (!outerElement) return; - const buttonElement = expanderButtonRef.current; - if (!buttonElement) return; - const buttonElements = getButtonElements(outerElement); - const currentIndex = buttonElements.indexOf(buttonElement); + const buttonElements = getButtonElements(outerRef); + if (!buttonElements) return; + const currentButtonElement = expanderButtonRef.current; + if (!currentButtonElement) return; + const currentIndex = buttonElements.indexOf(currentButtonElement); setTabbableButton(buttonElements, currentIndex); }; diff --git a/src/index.tsx b/src/index.tsx index f461250..0059aff 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -61,9 +61,8 @@ export const JsonView = ({ // on first render, set first button to tabIndex=0. React.useEffect(() => { - const outerElement = outerRef.current; - if (!outerElement) return; - const buttonElements = getButtonElements(outerElement); + const buttonElements = getButtonElements(outerRef); + if (!buttonElements) return; setTabbableButton(buttonElements, 0); }, [outerRef]); From 324d253dbd7823455e451fc921d30ae796da570c Mon Sep 17 00:00:00 2001 From: Blake Vandercar Date: Thu, 12 Sep 2024 23:46:09 -0600 Subject: [PATCH 06/10] improve test coverage --- src/DataRenderer.test.tsx | 28 ++++++++++++++++++++++++---- src/DataRenderer.tsx | 8 +++----- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/src/DataRenderer.test.tsx b/src/DataRenderer.test.tsx index 4871a4b..c2ca94b 100644 --- a/src/DataRenderer.test.tsx +++ b/src/DataRenderer.test.tsx @@ -1,5 +1,5 @@ import * as React from 'react'; -import DataRender, { JsonRenderProps } from './DataRenderer'; +import DataRender, { getButtonElements, JsonRenderProps } from './DataRenderer'; import { allExpanded, collapseAllNested, defaultStyles } from './index'; import { render, screen, fireEvent } from '@testing-library/react'; import '@testing-library/jest-dom'; @@ -311,6 +311,8 @@ describe('DataRender', () => { expect(buttons.length).toBe(2); expect(buttons[0]).toHaveClass('collapse-icon-light'); expect(buttons[1]).toHaveClass('expand-icon-light'); + expect(buttons[0].tabIndex).toEqual(0); + expect(buttons[1].tabIndex).toEqual(-1); expect(container.getElementsByClassName('clickable-label-light')).toHaveLength(1); expect(container.getElementsByClassName('collapsed-content-light')).toHaveLength(1); expect(screen.getByText(/test:/)).toBeInTheDocument(); @@ -355,13 +357,19 @@ describe('DataRender', () => { expect(screen.getByText('1')).toBeInTheDocument(); }); - it('should expand objects by pressing ArrowRight on icon', () => { + it('should expand objects by pressing ArrowRight on icon, collapse objects by pressing ArrowLeft on icon', () => { render(); - expect(screen.queryByText(/test:/)).not.toBeInTheDocument(); + const buttons = testButtonsCollapsed(); + expect(screen.queryByText(/test:/)).not.toBeInTheDocument(); + fireEvent.keyDown(buttons[0], { key: 'ArrowRight', code: 'ArrowRight' }); testButtonsExpanded(); expect(screen.getByText(/test:/)).toBeInTheDocument(); + + fireEvent.keyDown(buttons[0], { key: 'ArrowLeft', code: 'ArrowLeft' }); + testButtonsCollapsed(); + expect(screen.queryByText(/test:/)).not.toBeInTheDocument(); }); it('should not expand objects by pressing other keys on icon', () => { @@ -374,15 +382,22 @@ describe('DataRender', () => { expect(screen.queryByText(/test:/)).not.toBeInTheDocument(); }); - it('should expand arrays by pressing ArrowRight on icon', () => { + it('should expand arrays by pressing ArrowRight on icon, collapse arrays by pressing ArrowLeft on icon', () => { render(); + const buttons = testButtonsCollapsed(); expect(screen.queryByText(/test/)).not.toBeInTheDocument(); expect(screen.queryByText(/array/)).not.toBeInTheDocument(); + fireEvent.keyDown(buttons[0], { key: 'ArrowRight', code: 'ArrowRight' }); testButtonsExpanded(); expect(screen.getByText(/test/)).toBeInTheDocument(); expect(screen.getByText(/array/)).toBeInTheDocument(); + + fireEvent.keyDown(buttons[0], { key: 'ArrowLeft', code: 'ArrowLeft' }); + testButtonsCollapsed(); + expect(screen.queryByText(/test/)).not.toBeInTheDocument(); + expect(screen.queryByText(/array/)).not.toBeInTheDocument(); }); it('should not expand arrays by pressing other keys on icon', () => { @@ -396,4 +411,9 @@ describe('DataRender', () => { expect(screen.queryByText(/test:/)).not.toBeInTheDocument(); expect(screen.queryByText(/array/)).not.toBeInTheDocument(); }); + + it('getButtonElements returns undefined if no outerRef', () => { + const emptyRef = React.createRef(); + expect(getButtonElements(emptyRef)).toBeUndefined(); + }); }); diff --git a/src/DataRenderer.tsx b/src/DataRenderer.tsx index d8b369e..353deb9 100644 --- a/src/DataRenderer.tsx +++ b/src/DataRenderer.tsx @@ -100,12 +100,10 @@ function ExpandableObject({ const lastIndex = data.length - 1; const onKeyDown = (e: React.KeyboardEvent) => { - if ((e.key === 'ArrowRight' && !expanded) || (e.key === 'ArrowLeft' && expanded)) { + if (e.key === 'ArrowRight' || e.key === 'ArrowLeft') { e.preventDefault(); - toggleExpanded(); - } - - if (e.key === 'ArrowUp' || e.key === 'ArrowDown') { + setExpanded(e.key === 'ArrowRight'); + } else if (e.key === 'ArrowUp' || e.key === 'ArrowDown') { e.preventDefault(); const direction = e.key === 'ArrowUp' ? -1 : 1; From 48565801c125ae7d0b483baaaf893b81b91e2cce Mon Sep 17 00:00:00 2001 From: Blake Vandercar Date: Mon, 16 Sep 2024 09:20:20 -0600 Subject: [PATCH 07/10] refactor: maybe improve performance --- src/DataRenderer.test.tsx | 7 +------ src/DataRenderer.tsx | 40 ++++++++++++++++++--------------------- src/index.tsx | 9 +++++---- 3 files changed, 24 insertions(+), 32 deletions(-) diff --git a/src/DataRenderer.test.tsx b/src/DataRenderer.test.tsx index c2ca94b..2d57e13 100644 --- a/src/DataRenderer.test.tsx +++ b/src/DataRenderer.test.tsx @@ -1,5 +1,5 @@ import * as React from 'react'; -import DataRender, { getButtonElements, JsonRenderProps } from './DataRenderer'; +import DataRender, { JsonRenderProps } from './DataRenderer'; import { allExpanded, collapseAllNested, defaultStyles } from './index'; import { render, screen, fireEvent } from '@testing-library/react'; import '@testing-library/jest-dom'; @@ -411,9 +411,4 @@ describe('DataRender', () => { expect(screen.queryByText(/test:/)).not.toBeInTheDocument(); expect(screen.queryByText(/array/)).not.toBeInTheDocument(); }); - - it('getButtonElements returns undefined if no outerRef', () => { - const emptyRef = React.createRef(); - expect(getButtonElements(emptyRef)).toBeUndefined(); - }); }); diff --git a/src/DataRenderer.tsx b/src/DataRenderer.tsx index 353deb9..80a9874 100644 --- a/src/DataRenderer.tsx +++ b/src/DataRenderer.tsx @@ -51,18 +51,6 @@ function quoteString(value: string, quoted = false) { return value; } -export function getButtonElements(outerRef: React.RefObject) { - const outerElement = outerRef.current; - if (!outerElement) return; - return Array.from(outerElement.querySelectorAll('[role=button]')); -} - -export function setTabbableButton(buttonElements: HTMLElement[], index: number) { - buttonElements.forEach((buttonElement, i) => { - buttonElement.tabIndex = i === index ? 0 : -1; - }); -} - function ExpandableObject({ field, value, @@ -107,25 +95,33 @@ function ExpandableObject({ e.preventDefault(); const direction = e.key === 'ArrowUp' ? -1 : 1; - const buttonElements = getButtonElements(outerRef); - if (!buttonElements) return; + if (!outerRef.current) return; + const buttonElements = Array.from( + outerRef.current.querySelectorAll('[role=button]') + ); const currentIndex = buttonElements.findIndex((el) => el.tabIndex === 0); if (currentIndex < 0) return; const nextIndex = (currentIndex + direction + buttonElements.length) % buttonElements.length; // auto-wrap - setTabbableButton(buttonElements, nextIndex); + buttonElements[currentIndex].tabIndex = -1; + buttonElements[nextIndex].tabIndex = 0; buttonElements[nextIndex].focus(); } }; const onClick = () => { toggleExpanded(); - const buttonElements = getButtonElements(outerRef); - if (!buttonElements) return; - const currentButtonElement = expanderButtonRef.current; - if (!currentButtonElement) return; - const currentIndex = buttonElements.indexOf(currentButtonElement); - setTabbableButton(buttonElements, currentIndex); + + const buttonElement = expanderButtonRef.current; + if (!buttonElement) return; + const prevButtonElement = outerRef.current?.querySelector( + '[role=button][tabindex="0"]' + ); + if (prevButtonElement) { + prevButtonElement.tabIndex = -1; + } + buttonElement.tabIndex = 0; + buttonElement.focus(); }; return ( @@ -144,7 +140,7 @@ function ExpandableObject({ aria-expanded={expanded} aria-controls={expanded ? contentsId : undefined} ref={expanderButtonRef} - // tabIndex gets set on this component by setButtonTabIndex + // tabIndex=0 gets set by js tabIndex={-1} /> {(field || field === '') && diff --git a/src/index.tsx b/src/index.tsx index 0059aff..5ee87c6 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -1,5 +1,5 @@ import * as React from 'react'; -import DataRender, { getButtonElements, setTabbableButton, StyleProps } from './DataRenderer'; +import DataRender, { StyleProps } from './DataRenderer'; import styles from './styles.module.css'; export interface Props extends React.AriaAttributes { @@ -61,9 +61,10 @@ export const JsonView = ({ // on first render, set first button to tabIndex=0. React.useEffect(() => { - const buttonElements = getButtonElements(outerRef); - if (!buttonElements) return; - setTabbableButton(buttonElements, 0); + const firstButton = outerRef.current?.querySelector('[role=button]'); + if (firstButton) { + firstButton.tabIndex = 0; + } }, [outerRef]); return ( From 37c81f8034d6cb7c7b6f57e7c30b072d2dd6c197 Mon Sep 17 00:00:00 2001 From: Blake Vandercar Date: Mon, 16 Sep 2024 10:22:46 -0600 Subject: [PATCH 08/10] base first button tabIndex=0 off of level prop, not querySelector --- src/DataRenderer.test.tsx | 10 ++++++++++ src/DataRenderer.tsx | 5 +++-- src/index.test.tsx | 4 +++- src/index.tsx | 9 --------- 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/DataRenderer.test.tsx b/src/DataRenderer.test.tsx index 2d57e13..4b9ad10 100644 --- a/src/DataRenderer.test.tsx +++ b/src/DataRenderer.test.tsx @@ -411,4 +411,14 @@ describe('DataRender', () => { expect(screen.queryByText(/test:/)).not.toBeInTheDocument(); expect(screen.queryByText(/array/)).not.toBeInTheDocument(); }); + + it('only one item with tabindex=0 if level=0, none if level>0', () => { + const data = { test: [1, 2, 3], test2: [1, 2, 3], test3: { a: 'b', c: { d: '1', a: 2 } } }; + + const { container, rerender } = render(); + expect(container.querySelectorAll('[tabindex="0"]')).toHaveLength(1); + + rerender(); + expect(container.querySelectorAll('[tabindex="0"]')).toHaveLength(0); + }); }); diff --git a/src/DataRenderer.tsx b/src/DataRenderer.tsx index 80a9874..11abb40 100644 --- a/src/DataRenderer.tsx +++ b/src/DataRenderer.tsx @@ -23,6 +23,7 @@ export interface StyleProps { interface CommonRenderProps { lastElement: boolean; + /** There should only be one node with `level==0`. */ level: number; style: StyleProps; shouldExpandNode: (level: number, value: any, field?: string) => boolean; @@ -140,8 +141,8 @@ function ExpandableObject({ aria-expanded={expanded} aria-controls={expanded ? contentsId : undefined} ref={expanderButtonRef} - // tabIndex=0 gets set by js - tabIndex={-1} + // tabIndex gets changed by js. There should only be one node (top level) with level==0. + tabIndex={level === 0 ? 0 : -1} /> {(field || field === '') && (clickToExpandNode ? ( diff --git a/src/index.test.tsx b/src/index.test.tsx index e0933e7..7fcc48c 100644 --- a/src/index.test.tsx +++ b/src/index.test.tsx @@ -47,7 +47,9 @@ describe('JsonView', () => { }); it('should go to next node on ArrowDown, prev node with ArrowUp, tabindex should change', () => { - render(); + const { container } = render(); + + expect(container.querySelectorAll('[tabindex="0"]')).toHaveLength(1); const buttons = screen.getAllByRole('button', { hidden: true }); expect(buttons).toHaveLength(3); diff --git a/src/index.tsx b/src/index.tsx index 5ee87c6..9d133fc 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -58,15 +58,6 @@ export const JsonView = ({ ...ariaAttrs }: Props) => { const outerRef = React.useRef(null); - - // on first render, set first button to tabIndex=0. - React.useEffect(() => { - const firstButton = outerRef.current?.querySelector('[role=button]'); - if (firstButton) { - firstButton.tabIndex = 0; - } - }, [outerRef]); - return (
Date: Mon, 16 Sep 2024 14:49:44 -0600 Subject: [PATCH 09/10] fix typo --- src/hooks.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hooks.ts b/src/hooks.ts index 4e44802..a0ac35c 100644 --- a/src/hooks.ts +++ b/src/hooks.ts @@ -5,9 +5,9 @@ export function useBool( ): [boolean, () => void, (value: boolean) => void] { const [value, setValue] = useState(initialValueCreator()); - const tooggle = () => setValue((currentValue) => !currentValue); + const toggle = () => setValue((currentValue) => !currentValue); - return [value, tooggle, setValue]; + return [value, toggle, setValue]; } let componentId = 1; From 4c640da14488be2b28cf57ea7f8ff872c06c122f Mon Sep 17 00:00:00 2001 From: Blake Vandercar Date: Wed, 25 Sep 2024 09:56:47 -0600 Subject: [PATCH 10/10] don't use Array.from --- src/DataRenderer.tsx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/DataRenderer.tsx b/src/DataRenderer.tsx index 11abb40..19c6455 100644 --- a/src/DataRenderer.tsx +++ b/src/DataRenderer.tsx @@ -97,10 +97,11 @@ function ExpandableObject({ const direction = e.key === 'ArrowUp' ? -1 : 1; if (!outerRef.current) return; - const buttonElements = Array.from( - outerRef.current.querySelectorAll('[role=button]') - ); - const currentIndex = buttonElements.findIndex((el) => el.tabIndex === 0); + const buttonElements = outerRef.current.querySelectorAll('[role=button]'); + let currentIndex = -1; + buttonElements.forEach((el, i) => { + if (el.tabIndex === 0) currentIndex = i; + }); if (currentIndex < 0) return; const nextIndex = (currentIndex + direction + buttonElements.length) % buttonElements.length; // auto-wrap