Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions web/src/components/ChartSectionCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { cn } from 'cn';
import { Button } from '@/components/ui/button';
import { fetchApiText, type ChartSection, type ChartSpec, type Manifest } from '../api';
import { downloadCsvText } from '../csv';
import { chartViewName } from '../format';
import { usePrintMode } from '../printContext';
import { ChartSectionTitle } from './charts/leading';
import { CopyLinkButton } from './CopyLinkButton';
Expand Down Expand Up @@ -66,7 +67,7 @@ function Figure({
return (
<figure
hidden={hidden}
aria-label={`${chart.title} — ${active.label}`}
aria-label={chartViewName(chart.title, active.label)}
className={cn(
'm-0 min-w-0 rounded-xl border bg-background/40 p-3',
(slide || fullRow) && 'col-span-full',
Expand All @@ -86,7 +87,7 @@ function Figure({
<Suspense
fallback={
<p role="status" data-print-pending className="p-10 text-center text-muted-foreground">
Loading chart: {chart.title} ({active.label})…
Loading chart: {chartViewName(chart.title, active.label)}…
</p>
}
>
Expand Down
5 changes: 3 additions & 2 deletions web/src/components/InteractiveChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { RotateCwIcon } from 'lucide-react';
import { Button } from '@/components/ui/button';
import type { ChartDocument, ChartVariant, Manifest } from '../api';
import { fetchChartDocument } from '../chartData';
import { chartViewName } from '../format';
import { EventsView } from './charts/EventsView';
import { MatrixView } from './charts/MatrixView';
import { NetworkView } from './charts/NetworkView';
Expand Down Expand Up @@ -83,7 +84,7 @@ export default function InteractiveChart({
data-print-pending
className="flex h-[340px] items-center justify-center text-sm text-muted-foreground"
>
Loading interactive chart: {title} ({variant.label})…
Loading interactive chart: {chartViewName(title, variant.label)}…
</div>
</>
);
Expand All @@ -96,7 +97,7 @@ export default function InteractiveChart({
data-print-error
className="print-chart-status mb-4 flex flex-wrap items-center gap-3 rounded-lg border p-4 text-sm"
>
Could not load chart data: {title} ({variant.label}).
Could not load chart data: {chartViewName(title, variant.label)}.
<Button
variant="outline"
size="sm"
Expand Down
10 changes: 10 additions & 0 deletions web/src/format.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,13 @@ export function stamp(iso: string): string {
export function dateStamp(iso: string): string {
return stamp(iso).slice(0, 10);
}

/**
* A chart view's name: its title, then the variant label when that adds something.
*
* A single-view chart's only label is its own title, so joining the two would
* have a screen reader announce "Activity heatmap — Activity heatmap".
*/
export function chartViewName(title: string, label: string): string {
return label === title ? title : `${title} — ${label}`;
}
17 changes: 9 additions & 8 deletions web/src/test/app.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -206,8 +206,10 @@ describe('Organisation diversity card (#435)', () => {
return await screen.findByText('Organisation diversity');
};

const chartVariant = (title: string) =>
screen.getByRole('figure', { name: new RegExp(`^${title} —`) }).getAttribute('aria-label');
// A figure is named "<title> — <variant>", or just its title when it has one view.
const figureNamed = (title: string) =>
screen.getByRole('figure', { name: new RegExp(`^${title}( —|$)`) });
const chartVariant = (title: string) => figureNamed(title).getAttribute('aria-label');

it('gives the card one role axis, so every role-tabbed chart switches together', async () => {
await openDiversity();
Expand All @@ -223,8 +225,9 @@ describe('Organisation diversity card (#435)', () => {

expect(chartVariant('Role-holders by organisation')).toContain('Committers');
expect(chartVariant('Single-employer repos by org')).toContain('Committers');
// The chart with no role axis is untouched by the card's tabs.
expect(chartVariant('Single-employer teams by org')).toContain('Single-employer teams by org');
// The chart with no role axis is untouched by the card's tabs, and its one
// view is named once rather than "<title> — <title>".
expect(chartVariant('Single-employer teams by org')).toBe('Single-employer teams by org');
});

// Out-of-range tabs clamp to the last one; anything else falls back to the first.
Expand All @@ -244,11 +247,9 @@ describe('Organisation diversity card (#435)', () => {

it('leads an odd run of half-width charts with a two-row chart', async () => {
await openDiversity();
const figure = (title: string) =>
screen.getByRole('figure', { name: new RegExp(`^${title} —`) });
// Three half-width charts: the first spans two rows, the others stack beside it.
expect(figure('Role-holders by organisation')).toHaveClass('lg:row-span-2');
expect(figure('Single-employer teams by org')).not.toHaveClass('lg:row-span-2');
expect(figureNamed('Role-holders by organisation')).toHaveClass('lg:row-span-2');
expect(figureNamed('Single-employer teams by org')).not.toHaveClass('lg:row-span-2');
});

it('leaves a chart with its own variant set on its own tabs', async () => {
Expand Down
13 changes: 12 additions & 1 deletion web/src/test/format.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/** Timestamps are labelled UTC wherever they appear, so they must be in UTC. */

import { describe, expect, it } from 'vitest';
import { dateStamp, stamp } from '../format';
import { chartViewName, dateStamp, stamp } from '../format';

describe('stamp', () => {
it('keeps a UTC timestamp as-is', () => {
Expand Down Expand Up @@ -43,6 +43,17 @@ describe('dateStamp', () => {
});
});

describe('chartViewName', () => {
it('names the variant after the title', () => {
expect(chartViewName('Contributors', 'By month')).toBe('Contributors — By month');
});

it('does not repeat a single-view chart’s title', () => {
// A chart with one view is labelled with its own title.
expect(chartViewName('Activity heatmap', 'Activity heatmap')).toBe('Activity heatmap');
});
});

describe('readFigure', () => {
it('draws shares and parts of a whole as meters, and counts with separators', async () => {
const { readFigure } = await import('../metricFigure');
Expand Down
4 changes: 2 additions & 2 deletions web/src/test/printComponents.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,12 @@ describe('Chart printing', () => {
<InteractiveChart variant={variant} title="Contributors" provenance={MANIFEST.provenance} />,
);
expect(screen.getByRole('status')).toHaveAttribute('data-print-pending');
expect(screen.getByRole('status')).toHaveTextContent('Contributors (All)');
expect(screen.getByRole('status')).toHaveTextContent('Contributors — All');

await act(async () => fail());
const alert = await screen.findByRole('alert');
expect(alert).toHaveAttribute('data-print-error');
expect(alert).toHaveTextContent('Could not load chart data: Contributors (All).');
expect(alert).toHaveTextContent('Could not load chart data: Contributors — All.');
expect(document.querySelector('[data-print-pending]')).toBeNull();
vi.unstubAllGlobals();
});
Expand Down
Loading