Skip to content

Commit dae23c3

Browse files
authored
Merge pull request #586 from adobe/clamoureux/axisSecondGranularity
feat: switch primary label for second granularity to minutes
2 parents 8a95a60 + c63fd1c commit dae23c3

File tree

2 files changed

+19
-8
lines changed

2 files changed

+19
-8
lines changed

packages/react-spectrum-charts/src/stories/components/Axis/Axis.test.tsx

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -73,19 +73,27 @@ describe('Axis', () => {
7373
const chart = await findChart();
7474
expect(chart).toBeInTheDocument();
7575

76-
expect(screen.getByText('Dec 2')).toBeInTheDocument();
77-
const timeLabels = screen.getAllByText(/\d+:\d+:\d+ [AP]M/);
76+
// Check for minute-based time labels (primary label)
77+
const timeLabels = screen.getAllByText(/\d+:\d+ [AP]M/);
7878
expect(timeLabels.length).toBeGreaterThan(0);
79+
80+
// Check for second labels (secondary label)
81+
const secondLabels = screen.getAllByText(/^\d{1,2}$/); // Just digits for seconds
82+
expect(secondLabels.length).toBeGreaterThan(0);
7983
});
8084

8185
test('Second renders properly', async () => {
8286
render(<SecondGranularity {...SecondGranularity.args} />);
8387
const chart = await findChart();
8488
expect(chart).toBeInTheDocument();
8589

86-
expect(screen.getByText('Dec 2')).toBeInTheDocument();
87-
const timeLabels = screen.getAllByText(/\d+:\d+:\d+ [AP]M/);
90+
// Check for minute-based time labels (primary label)
91+
const timeLabels = screen.getAllByText(/\d+:\d+ [AP]M/);
8892
expect(timeLabels.length).toBeGreaterThan(0);
93+
94+
// Check for second labels (secondary label)
95+
const secondLabels = screen.getAllByText(/^\d{1,2}$/); // Just digits for seconds
96+
expect(secondLabels.length).toBeGreaterThan(0);
8997
});
9098

9199
test('Minute renders properly', async () => {
@@ -162,10 +170,13 @@ describe('Axis', () => {
162170
const chart = await findChart();
163171
expect(chart).toBeInTheDocument();
164172

165-
// Day and time are in the same label for this configuration.
166-
expect(screen.getByText(/Dec 2.+\d+:\d+:\d+ [AP]M/)).toBeInTheDocument();
167-
const timeLabels = screen.getAllByText(/\d+:\d+:\d+ [AP]M/);
173+
// Check for time labels (minutes)
174+
const timeLabels = screen.getAllByText(/\d+:\d+ [AP]M/);
168175
expect(timeLabels.length).toBeGreaterThan(0);
176+
177+
// Check for second labels
178+
const secondLabels = screen.getAllByText(/^\d{1,2}$/);
179+
expect(secondLabels.length).toBeGreaterThan(0);
169180
});
170181

171182
test('Minute renders properly', async () => {

packages/vega-spec-builder/src/axis/axisLabelUtils.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ export const getTimeLabelFormats = (
4848
): { secondaryLabelFormat: string; primaryLabelFormat: string; tickCount: TickCount } => {
4949
switch (granularity) {
5050
case 'second':
51-
return { secondaryLabelFormat: '%-I:%M:%S %p', primaryLabelFormat: '%b %-d', tickCount: 'second' };
51+
return { secondaryLabelFormat: '%S', primaryLabelFormat: '%-I:%M %p', tickCount: 'second' };
5252
case 'minute':
5353
return { secondaryLabelFormat: '%-I:%M %p', primaryLabelFormat: '%b %-d', tickCount: 'minute' };
5454
case 'hour':

0 commit comments

Comments
 (0)