Skip to content

Commit

Permalink
accessibilityLayer should respect a reversed XAxis (#4225)
Browse files Browse the repository at this point in the history
<!--- Provide a general summary of your changes in the Title above -->

## Description

<!--- Describe your changes in detail -->

The AccessibilityManager (which manages the behavior for the
`accessibilityLayer` prop) will take into account if there is an `<XAxis
reversed={true} />` child. If so, the left/right arrow keys will
reverse, so that they align with the chart's visuals.

## Related Issue

<!--- This project only accepts pull requests related to open issues -->
<!--- If suggesting a new feature or change, please discuss it in an
issue first -->
<!--- If fixing a bug, there should be an issue describing it with steps
to reproduce -->
<!--- Please link to the issue here: -->
#4214

## Motivation and Context

<!--- Why is this change required? What problem does it solve? -->

## How Has This Been Tested?

<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, and the tests you ran
to -->
<!--- see how your change affects other areas of the code, etc. -->
Tested manually, and added test cases 

## Screenshots (if appropriate):

## Types of changes

<!--- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)

## Checklist:

<!--- Go over all the following points, and put an `x` in all the boxes
that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->

- [x] My code follows the code style of this project.
- [NA] My change requires a change to the documentation.
- [NA] I have updated the documentation accordingly.
- [x] I have added tests to cover my changes.
- [NA] I have added a storybook story or extended an existing story to
show my changes
- [x] All new and existing tests passed.

---------

Co-authored-by: Coltin Kifer <ckifer@amazon.com>
  • Loading branch information
julianna-langston and Coltin Kifer committed Feb 29, 2024
1 parent 1ce9b2f commit 1fd1c77
Show file tree
Hide file tree
Showing 5 changed files with 231 additions and 3 deletions.
14 changes: 11 additions & 3 deletions src/chart/AccessibilityManager.ts
Expand Up @@ -11,6 +11,8 @@ interface InitiableOptions {
container?: HTMLElement;
layout?: LayoutType;
offset?: ContainerOffset;
/* Is the chart oriented left-to-right? true = left-to-right, false = right-to-left */
ltr?: boolean;
}

export class AccessibilityManager {
Expand All @@ -26,18 +28,22 @@ export class AccessibilityManager {

private offset: InitiableOptions['offset'];

private ltr = true;

public setDetails({
coordinateList = null,
container = null,
layout = null,
offset = null,
mouseHandlerCallback = null,
ltr = null,
}: InitiableOptions) {
this.coordinateList = coordinateList ?? this.coordinateList ?? [];
this.container = container ?? this.container;
this.layout = layout ?? this.layout;
this.offset = offset ?? this.offset;
this.mouseHandlerCallback = mouseHandlerCallback ?? this.mouseHandlerCallback;
this.ltr = ltr ?? this.ltr;

// Keep activeIndex in the bounds between 0 and the last coordinate index
this.activeIndex = Math.min(Math.max(this.activeIndex, 0), this.coordinateList.length - 1);
Expand All @@ -55,16 +61,18 @@ export class AccessibilityManager {
return;
}

switch (e.key) {
case 'ArrowRight': {
switch (`${this.ltr ? 'ltr' : 'rtl'}-${e.key}`) {
case 'ltr-ArrowRight':
case 'rtl-ArrowLeft': {
if (this.layout !== 'horizontal') {
return;
}
this.activeIndex = Math.min(this.activeIndex + 1, this.coordinateList.length - 1);
this.spoofMouse();
break;
}
case 'ArrowLeft': {
case 'ltr-ArrowLeft':
case 'rtl-ArrowRight': {
if (this.layout !== 'horizontal') {
return;
}
Expand Down
9 changes: 9 additions & 0 deletions src/chart/generateCategoricalChart.tsx
Expand Up @@ -50,6 +50,7 @@ import {
getStackGroupsByAxisId,
getTicksOfAxis,
getTooltipItem,
isAxisLTR,
isCategoricalAxis,
parseDomainOfCategoryAxis,
parseErrorBarsOfAxis,
Expand Down Expand Up @@ -1089,6 +1090,8 @@ export const generateCategoricalChart = ({
coordinateList: this.state.tooltipTicks,
mouseHandlerCallback: this.triggeredAfterMouseMove,
layout: this.props.layout,
// Check all (0+) <XAxis /> elements to see if ANY have reversed={true}. If so, this will be treated as an RTL chart
ltr: isAxisLTR(this.state.xAxisMap),
});
this.displayDefaultTooltip();
}
Expand Down Expand Up @@ -1170,6 +1173,12 @@ export const generateCategoricalChart = ({
});
}

if (this.state.xAxisMap !== prevState.xAxisMap) {
this.accessibilityManager.setDetails({
ltr: isAxisLTR(this.state.xAxisMap),
});
}

if (this.props.layout !== prevProps.layout) {
this.accessibilityManager.setDetails({
layout: this.props.layout,
Expand Down
12 changes: 12 additions & 0 deletions src/util/ChartUtils.ts
Expand Up @@ -40,6 +40,7 @@ import {
StackOffsetType,
Margin,
ChartOffset,
XAxisMap,
} from './types';
import { getLegendProps } from './getLegendProps';

Expand Down Expand Up @@ -1332,3 +1333,14 @@ export const getTooltipItem = (graphicalItem: ReactElement, payload: any) => {
hide,
};
};

export const isAxisLTR = (axisMap: XAxisMap) => {
const axes = Object.values(axisMap ?? {});
// If there are no <XAxis /> elements in the chart, then the chart is left-to-right (returning true).
if (axes.length === 0) {
return true;
}
// If there are any cases of reversed=true, then the chart is right-to-left (returning false).
// Otherwise, the chart is left-to-right (returning true)
return !axes.some(({ reversed }) => reversed);
};
148 changes: 148 additions & 0 deletions test/chart/AccessibilityLayer.spec.tsx
Expand Up @@ -48,6 +48,29 @@ describe('AccessibilityLayer', () => {
expect(tooltip).toHaveTextContent('Page A');
});

test('accessibilityLayer works, even without *Axis elements', () => {
const { container } = render(
<AreaChart width={100} height={50} data={data} accessibilityLayer>
<Area type="monotone" dataKey="uv" stroke="#ff7300" fill="#ff7300" />
<Tooltip />
</AreaChart>,
);

// Confirm that the tooltip container exists, but isn't displaying anything
const tooltip = container.querySelector('.recharts-tooltip-wrapper');
expect(tooltip?.textContent).toBe('');

// Once the chart receives focus, the tooltip should display
container.querySelector('svg')?.focus();
expect(tooltip).toHaveTextContent('uv : 400');

// Use keyboard to move around
fireEvent.keyDown(document.querySelector('svg') as SVGSVGElement, {
key: 'ArrowRight',
});
expect(tooltip).toHaveTextContent('uv : 300');
});

test('Chart updates when it receives left/right arrow keystrokes', () => {
const mockMouseMovements = vi.fn();

Expand Down Expand Up @@ -172,6 +195,83 @@ describe('AccessibilityLayer', () => {
expect(mockMouseMovements.mock.instances).toHaveLength(0);
});

test('Left/right arrow pays attention to if the XAxis is reversed', () => {
const mockMouseMovements = vi.fn();

const { container } = render(
<AreaChart width={100} height={50} data={data} accessibilityLayer onMouseMove={mockMouseMovements}>
<Area type="monotone" dataKey="uv" stroke="#ff7300" fill="#ff7300" />
<Tooltip />
<Legend />
<XAxis dataKey="name" reversed />
<YAxis />
</AreaChart>,
);

const svg = container.querySelector('svg');
assertNotNull(svg);
const tooltip = container.querySelector('.recharts-tooltip-wrapper');

expect(tooltip?.textContent).toBe('');
expect(mockMouseMovements).toHaveBeenCalledTimes(0);

// Once the chart receives focus, the tooltip should display
svg.focus();
expect(tooltip).toHaveTextContent('Page A');
expect(mockMouseMovements).toHaveBeenCalledTimes(1);

// Ignore right arrow when you're already at the right
fireEvent.keyDown(svg, {
key: 'ArrowRight',
});
expect(tooltip).toHaveTextContent('Page A');
expect(mockMouseMovements).toHaveBeenCalledTimes(2);

// Respect left arrow when there's something to the left
fireEvent.keyDown(svg, {
key: 'ArrowLeft',
});
expect(tooltip).toHaveTextContent('Page B');
expect(mockMouseMovements).toHaveBeenCalledTimes(3);

// Page C
fireEvent.keyDown(svg, {
key: 'ArrowLeft',
});

// Page D
fireEvent.keyDown(svg, {
key: 'ArrowLeft',
});

fireEvent.keyDown(svg, {
key: 'ArrowLeft',
});
expect(tooltip).toHaveTextContent('Page E');
expect(mockMouseMovements).toHaveBeenCalledTimes(6);

// Ignore left arrow when you're already at the left
fireEvent.keyDown(svg, {
key: 'ArrowLeft',
});
expect(tooltip).toHaveTextContent('Page F');
expect(mockMouseMovements).toHaveBeenCalledTimes(7);

// Respect right arrow when there's something to the right
fireEvent.keyDown(svg, {
key: 'ArrowRight',
});
expect(tooltip).toHaveTextContent('Page E');
expect(mockMouseMovements).toHaveBeenCalledTimes(8);

// Chart ignores non-arrow keys
fireEvent.keyDown(svg, {
key: 'a',
});
expect(tooltip).toHaveTextContent('Page E');
expect(mockMouseMovements).toHaveBeenCalledTimes(8);
});

const Expand = () => {
const [width, setWidth] = useState(6);
const myData = data.slice(0, width);
Expand Down Expand Up @@ -233,6 +333,7 @@ describe('AccessibilityLayer', () => {
key: 'ArrowRight',
});

// Page F
fireEvent.keyDown(svg, {
key: 'ArrowRight',
});
Expand Down Expand Up @@ -394,4 +495,51 @@ describe('AccessibilityLayer', () => {
});
}).not.toThrowError();
});

const DirectionSwitcher = () => {
const [reversed, setReversed] = useState(false);

return (
<div>
<button type="button" onClick={() => setReversed(!reversed)}>
Change directions
</button>

<AreaChart width={100} height={50} data={data} accessibilityLayer>
<Area type="monotone" dataKey="uv" stroke="#ff7300" fill="#ff7300" />
<Tooltip />
<Legend />
<XAxis dataKey="name" reversed={reversed} />
<YAxis orientation={reversed ? 'right' : 'left'} />
</AreaChart>
</div>
);
};

test('AccessibilityLayer respects dynamic changes to the XAxis orientation', () => {
const { container } = render(<DirectionSwitcher />);

const svg = container.querySelector('svg');
assertNotNull(svg);
const tooltip = container.querySelector('.recharts-tooltip-wrapper');

expect(tooltip?.textContent).toBe('');

svg.focus();
expect(tooltip).toHaveTextContent('Page A');

fireEvent.keyDown(svg, {
key: 'ArrowRight',
});
expect(tooltip).toHaveTextContent('Page B');

const button = container.querySelector('BUTTON') as HTMLButtonElement;
fireEvent.click(button);

// Key events should now go in reverse
fireEvent.keyDown(svg, {
key: 'ArrowRight',
});
expect(tooltip).toHaveTextContent('Page A');
});
});
51 changes: 51 additions & 0 deletions test/util/ChartUtils.spec.tsx
Expand Up @@ -15,6 +15,7 @@ import {
parseSpecifiedDomain,
getTicksOfAxis,
getLegendProps,
isAxisLTR,
} from '../../src/util/ChartUtils';
import { BaseAxisProps, DataKey } from '../../src/util/types';

Expand Down Expand Up @@ -682,3 +683,53 @@ describe('exports for backwards-compatibility', () => {
expect(getLegendProps).toBeInstanceOf(Function);
});
});

test('isLTR', () => {
// Axis with reversed=false
expect(
isAxisLTR({
0: { reversed: false },
}),
).toBeTruthy();
// Axis with reversed=true
expect(
isAxisLTR({
0: { reversed: true },
}),
).toBeFalsy();
// Custom XAxisId, reversed=false
expect(
isAxisLTR({
custom: { reversed: false },
}),
).toBeTruthy();
// Custom XAxisId, reversed=true
expect(
isAxisLTR({
custom: { reversed: true },
}),
).toBeFalsy();
// Multiple axes, both reversed=true
expect(
isAxisLTR({
0: { reversed: true },
1: { reversed: true },
}),
).toBeFalsy();
// Multiple axes, both reversed=false
expect(
isAxisLTR({
0: { reversed: false },
1: { reversed: false },
}),
).toBeTruthy();
// Multiple axes, different reversed values
expect(
isAxisLTR({
0: { reversed: true },
1: { reversed: false },
}),
).toBeFalsy();
// Empty set of axes
expect(isAxisLTR({})).toBeTruthy();
});

0 comments on commit 1fd1c77

Please sign in to comment.