Skip to content

Commit

Permalink
feat(legend): change click on item behaviour (#2431)
Browse files Browse the repository at this point in the history
feat(legend): change click on item behaviour (#2431)
    
Clicking on a legend item has been improved to isolate a series on first click, show all series on second click.
Also, when clicking on a hidden series now will add the selected series into the visible set.
    
BREAKING CHANGE: The legend modifier key has been changed to CTRL (or CMD on Mac) from SHIFT. The SHIFT key will no longer have any effect on click.
  • Loading branch information
dej611 authored May 17, 2024
1 parent e448bd7 commit b03bdd0
Show file tree
Hide file tree
Showing 9 changed files with 135 additions and 90 deletions.
29 changes: 15 additions & 14 deletions e2e/page_objects/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ interface ElementBBox {
height: number;
}

interface KeyboardKey {
type KeyboardKey = {
key: string;
count: number;
}
};

interface ClickOptions {
/**
Expand Down Expand Up @@ -217,6 +217,13 @@ export class CommonPage {
];
}

getModifierKey = (page: Page) => async () => {
const isMac = await page.evaluate(() => {
return navigator.userAgent.includes('Mac');
});
return isMac ? 'Meta' : 'Control';
};

/**
* Toggle element visibility
* @param selector
Expand Down Expand Up @@ -351,18 +358,12 @@ export class CommonPage {
*/
// eslint-disable-next-line class-methods-use-this
pressKey = (page: Page) => async (key: string, count: number) => {
if (key === 'tab') {
let i = 0;
while (i < count) {
await page.keyboard.press('Tab');
i++;
}
} else if (key === 'enter') {
let i = 0;
while (i < count) {
await page.keyboard.press('Enter');
i++;
}
// capitalize some keys
const keyName = ['tab', 'enter'].includes(key) ? `${key.charAt(0).toUpperCase()}${key.slice(1)}` : key;
let i = 0;
while (i < count) {
await page.keyboard.press(keyName);
i++;
}
};

Expand Down
4 changes: 4 additions & 0 deletions e2e/tests/area_stories.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,9 @@ test.describe('Area series stories', () => {

test('shows only positive values when hiding negative one', async ({ page }) => {
const action = async () => {
await page.keyboard.down(await common.getModifierKey(page)());
await page.click('.echLegendItem:nth-child(2) .echLegendItem__label');
await page.keyboard.up(await common.getModifierKey(page)());
};
await common.expectChartAtUrlToMatchScreenshot(page)(
'http://localhost:9001/?path=/story/area-chart--with-negative-and-positive&knob-Y scale=log',
Expand All @@ -74,7 +76,9 @@ test.describe('Area series stories', () => {

test('shows only negative values when hiding positive one', async ({ page }) => {
const action = async () => {
await page.keyboard.down(await common.getModifierKey(page)());
await page.click('.echLegendItem:nth-child(1) .echLegendItem__label');
await page.keyboard.up(await common.getModifierKey(page)());
};
await common.expectChartAtUrlToMatchScreenshot(page)(
'http://localhost:9001/?path=/story/area-chart--with-negative-and-positive&knob-Y scale=log',
Expand Down
2 changes: 1 addition & 1 deletion e2e/tests/interactions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ test.describe('Interactions', () => {
count: 2,
},
{
key: 'enter',
key: `${await common.getModifierKey(page)()}+Enter`,
count: 1,
},
],
Expand Down
4 changes: 2 additions & 2 deletions e2e/tests/legend_stories.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ test.describe('Legend stories', () => {
count: 2,
},
{
key: 'enter',
key: `${await common.getModifierKey(page)()}+Enter`,
count: 1,
},
],
Expand All @@ -174,7 +174,7 @@ test.describe('Legend stories', () => {
// Make the first index legend item hidden
await page.keyboard.press('Tab');
await page.keyboard.press('Tab');
await page.keyboard.press('Enter');
await page.keyboard.press(`${await common.getModifierKey(page)()}+Enter`);

const hiddenResults: number[] = [];
// Filter the labels
Expand Down
2 changes: 2 additions & 0 deletions e2e/tests/mixed_stories.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,8 @@ test.describe('Mixed series stories', () => {

test('should show area chart with toggled series and mouse over', async ({ page }) => {
const action = async () => {
// hold the meta/control key to hide rather than isolate
await page.keyboard.down(await common.getModifierKey(page)());
await page.click('.echLegendItem:nth-child(2) .echLegendItem__label');
};
await common.expectChartWithMouseAtUrlToMatchScreenshot(page)(
Expand Down
136 changes: 80 additions & 56 deletions packages/charts/src/chart_types/xy_chart/legend/legend.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,31 @@ describe('Legends', () => {
beforeEach(() => {
store = MockStore.default();
});

function addBarSeries(n: number) {
const colors = ['red', 'blue', 'green', 'violet', 'orange', 'yellow', 'brown', 'black', 'white', 'gray'];
MockStore.addSpecs(
[
...Array.from({ length: n }, (_, i) =>
MockSeriesSpec.bar({
id: `spec${i + 1}`,
data: [
{
x: 0,
y: 1,
},
],
}),
),
MockGlobalSpec.settings({
showLegend: true,
theme: { colors: { vizColors: colors.slice(0, n) } },
}),
],
store,
);
}

it('compute legend for a single series', () => {
MockStore.addSpecs(
[
Expand Down Expand Up @@ -229,73 +254,72 @@ describe('Legends', () => {
});

it('default all series legend items to visible when deselectedDataSeries is null', () => {
MockStore.addSpecs(
[
MockSeriesSpec.bar({
id: 'spec1',
data: [
{
x: 0,
y: 1,
},
],
}),
MockSeriesSpec.bar({
id: 'spec2',
data: [
{
x: 0,
y: 1,
},
],
}),
MockGlobalSpec.settings({
showLegend: true,
theme: { colors: { vizColors: ['red', 'blue'] } },
}),
],
store,
);
addBarSeries(3);
const legend = computeLegendSelector(store.getState());

const visibility = legend.map((item) => !item.isSeriesHidden);

expect(visibility).toEqual([true, true]);
expect(visibility).toEqual([true, true, true]);
});
it('selectively sets series to visible when there are deselectedDataSeries items', () => {
MockStore.addSpecs(
[
MockSeriesSpec.bar({
id: 'spec1',
data: [
{
x: 0,
y: 1,
},
],
}),
MockSeriesSpec.bar({
id: 'spec2',
data: [
{
x: 0,
y: 1,
},
],
}),
MockGlobalSpec.settings({
showLegend: true,
theme: { colors: { vizColors: ['red', 'blue'] } },
}),
],
store,
);
addBarSeries(3);
const { key, specId } = computeSeriesDomainsSelector(store.getState()).formattedDataSeries[0]!;

store.dispatch(onToggleDeselectSeriesAction([{ key, specId }]));
const legend = computeLegendSelector(store.getState());
const visibility = legend.map((item) => !item.isSeriesHidden);
expect(visibility).toEqual([false, true]);
// only the clicked item should be visible
expect(visibility).toEqual([true, false, false]);
});
it('resets all series to be visible when clicking again the only visible item', () => {
addBarSeries(3);
const { key, specId } = computeSeriesDomainsSelector(store.getState()).formattedDataSeries[0]!;
// click the first item
store.dispatch(onToggleDeselectSeriesAction([{ key, specId }]));
// now click again the same item
store.dispatch(onToggleDeselectSeriesAction([{ key, specId }]));
const legend = computeLegendSelector(store.getState());
const visibility = legend.map((item) => !item.isSeriesHidden);
expect(visibility).toEqual([true, true, true]);
});
it('makes it visible when a hidden series is clicked', () => {
addBarSeries(3);
const { key, specId } = computeSeriesDomainsSelector(store.getState()).formattedDataSeries[0]!;
// click the first item
store.dispatch(onToggleDeselectSeriesAction([{ key, specId }]));
const { key: otherKey, specId: otherSpecId } = computeSeriesDomainsSelector(store.getState())
.formattedDataSeries[1]!;
// now click the second item (now hidden)
store.dispatch(onToggleDeselectSeriesAction([{ key: otherKey, specId: otherSpecId }]));
const legend = computeLegendSelector(store.getState());
const visibility = legend.map((item) => !item.isSeriesHidden);
expect(visibility).toEqual([true, true, false]);
});
it('makes it hidden the clicked series if there are more than one series visible', () => {
addBarSeries(3);
const { key, specId } = computeSeriesDomainsSelector(store.getState()).formattedDataSeries[0]!;
// click the first item
store.dispatch(onToggleDeselectSeriesAction([{ key, specId }]));
const { key: otherKey, specId: otherSpecId } = computeSeriesDomainsSelector(store.getState())
.formattedDataSeries[1]!;
// now click the second item (now hidden)
store.dispatch(onToggleDeselectSeriesAction([{ key: otherKey, specId: otherSpecId }]));
// ...and click again this second item to make it hidden
store.dispatch(onToggleDeselectSeriesAction([{ key: otherKey, specId: otherSpecId }]));
const legend = computeLegendSelector(store.getState());
const visibility = legend.map((item) => !item.isSeriesHidden);
expect(visibility).toEqual([true, false, false]);
});
it('make it possible to hide all series using meta key on the only visible item', () => {
addBarSeries(3);
const { key, specId } = computeSeriesDomainsSelector(store.getState()).formattedDataSeries[0]!;
// click the first item
store.dispatch(onToggleDeselectSeriesAction([{ key, specId }]));
// now click again with the meta key enabled
store.dispatch(onToggleDeselectSeriesAction([{ key, specId }], true));
const legend = computeLegendSelector(store.getState());
const visibility = legend.map((item) => !item.isSeriesHidden);
expect(visibility).toEqual([false, false, false]);
});
it('returns the right series name for a color series', () => {
const seriesIdentifier1 = {
Expand Down
11 changes: 8 additions & 3 deletions packages/charts/src/components/legend/label.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ interface LabelProps {
options: LegendLabelOptions;
}

const isAppleDevice = typeof window !== 'undefined' && /Mac|iPhone|iPad/.test(window.navigator.userAgent);

/**
* Label component used to display text in legend item
* @internal
Expand All @@ -32,10 +34,13 @@ export function Label({ label, isToggleable, onToggle, isSeriesHidden, options }
'echLegendItem__label--multiline': maxLines > 1,
});

const onClick: MouseEventHandler = useCallback(({ shiftKey }) => onToggle?.(shiftKey), [onToggle]);
const onClick: MouseEventHandler = useCallback(
({ metaKey, ctrlKey }) => onToggle?.(isAppleDevice ? metaKey : ctrlKey),
[onToggle],
);
const onKeyDown: KeyboardEventHandler = useCallback(
({ key, shiftKey }) => {
if (key === ' ' || key === 'Enter') onToggle?.(shiftKey);
({ key, metaKey, ctrlKey }) => {
if (key === ' ' || key === 'Enter') onToggle?.(isAppleDevice ? metaKey : ctrlKey);
},
[onToggle],
);
Expand Down
6 changes: 3 additions & 3 deletions packages/charts/src/state/actions/legend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ interface LegendItemOutAction {
export interface ToggleDeselectSeriesAction {
type: typeof ON_TOGGLE_DESELECT_SERIES;
legendItemIds: SeriesIdentifier[];
negate: boolean;
metaKey: boolean;
}

/** @internal */
Expand All @@ -63,9 +63,9 @@ export function onLegendItemOutAction(): LegendItemOutAction {
/** @internal */
export function onToggleDeselectSeriesAction(
legendItemIds: SeriesIdentifier[],
negate = false,
metaKey = false,
): ToggleDeselectSeriesAction {
return { type: ON_TOGGLE_DESELECT_SERIES, legendItemIds, negate };
return { type: ON_TOGGLE_DESELECT_SERIES, legendItemIds, metaKey };
}

/** @internal */
Expand Down
31 changes: 20 additions & 11 deletions packages/charts/src/state/reducers/interactions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ export function interactionsReducer(
*/

function toggleDeselectedDataSeries(
{ legendItemIds, negate }: ToggleDeselectSeriesAction,
{ legendItemIds, metaKey }: ToggleDeselectSeriesAction,
deselectedDataSeries: SeriesIdentifier[],
legendItems: LegendItem[],
) {
Expand All @@ -273,19 +273,28 @@ function toggleDeselectedDataSeries(
const legendItemsKeys = legendItems.map(({ seriesIdentifiers }) => seriesIdentifiers);

const alreadyDeselected = actionSeriesKeys.every((key) => deselectedDataSeriesKeys.has(key));
const keepOnlyNonActionSeries = ({ key }: SeriesIdentifier) => !actionSeriesKeys.includes(key);

// todo consider branch simplifications
if (negate) {
return alreadyDeselected || deselectedDataSeries.length !== legendItemsKeys.length - 1
? legendItems
.flatMap(({ seriesIdentifiers }) => seriesIdentifiers)
.filter(({ key }) => !actionSeriesKeys.includes(key))
: legendItemIds;
} else {
// when a meta key (CTRL or Mac Cmd ⌘) add or remove the clicked item from the visible list
if (metaKey) {
return alreadyDeselected
? deselectedDataSeries.filter(({ key }) => !actionSeriesKeys.includes(key))
: [...deselectedDataSeries, ...legendItemIds];
? deselectedDataSeries.filter(keepOnlyNonActionSeries)
: deselectedDataSeries.concat(legendItemIds);
}
// when a hidden series is clicked, make it visible
if (alreadyDeselected) {
return deselectedDataSeries.filter(keepOnlyNonActionSeries);
}
// if the clicked item is the only visible series, make all series visible (reset)
if (deselectedDataSeries.length === legendItemsKeys.length - 1) {
return [];
}
// at this point either a visible series was clicked:
// * if there's at least one hidden series => add it to the hidden list
// * otherwise hide everything but the clicked item (isolate it)
return deselectedDataSeries.length
? deselectedDataSeries.concat(legendItemIds)
: legendItemsKeys.flat().filter(keepOnlyNonActionSeries);
}

function getDrilldownData(globalState: GlobalChartState) {
Expand Down

0 comments on commit b03bdd0

Please sign in to comment.