Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(legend): multiline labels with maxLines option #1285

Merged
merged 16 commits into from
Aug 6, 2021
Merged
Show file tree
Hide file tree
Changes from 8 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
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
41 changes: 34 additions & 7 deletions integration/tests/legend_stories.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ describe('Legend stories', () => {
});
it('should 0 legend buffer', async () => {
await common.expectChartAtUrlToMatchScreenshot(
'http://localhost:9001/?path=/story/legend--legend-spacing-buffer&knob-legend buffer value=0',
'http://localhost:9001/?path=/story/legend--legend-spacing-buffer&knob-legend buffer value=0&knob-multiline Legend labels=false',
);
});
it('should have the same order as nested with no indent even if there are repeated labels', async () => {
Expand All @@ -36,6 +36,18 @@ describe('Legend stories', () => {
);
});

it('should correctly render multiline nested legend labels', async () => {
await common.expectChartAtUrlToMatchScreenshot(
'http://localhost:9001/?path=/story/legend--piechart&globals=backgrounds.value:!hex(fff);themes.value:Light&knob-Hide color picker=true&knob-Hide color picker_Legend=true&knob-Inside chart_Legend=true&knob-Legend position=right&knob-Legend position_Legend=right&knob-Multiline_Label options=true&knob-Multiline_Legend=true&knob-Number of series=5&knob-Popover position=leftCenter&knob-Popover position_Legend=leftCenter&knob-Series with long name=3&knob-direction_Legend=vertical&knob-floating columns_Legend=2&knob-hAlign_Legend=right&knob-legend buffer value=80&knob-legend margins=20&knob-long label text_Legend=Non do aliqua veniam dolore ipsum eu aliquip. Culpa in duis amet non velit qui non ullamco sit adipisicing. Ut sunt Lorem mollit exercitation deserunt officia sunt ipsum eu amet.&knob-multiline Legend labels=true&knob-vAlign_Legend=bottom&knob-Partition Layout=sunburst&knob-flatLegend=&knob-showLegendExtra=&knob-legendMaxDepth=2&knob-legendStrategy=key&knob-Multiline=true',
);
});

it('should correctly render very long multiline legend labels', async () => {
await common.expectChartAtUrlToMatchScreenshot(
'http://localhost:9001/?path=/story/legend--legend-spacing-buffer&globals=backgrounds.value:transparent;themes.value:Light&knob-Hide color picker=true&knob-Hide color picker_Legend=true&knob-Inside chart_Legend=true&knob-Legend position=right&knob-Legend position_Legend=right&knob-Multiline=true&knob-Multiline_Label options=true&knob-Multiline_Legend=true&knob-Number of series=5&knob-Partition Layout=sunburst&knob-Popover position=leftCenter&knob-Popover position_Legend=leftCenter&knob-Series with long name=3&knob-direction_Legend=vertical&knob-flatLegend=true&knob-floating columns_Legend=2&knob-hAlign_Legend=right&knob-legend buffer value=80&knob-legend margins=20&knob-legendMaxDepth=3&knob-legendStrategy=key&knob-long label text_Legend=Non do aliqua veniam dolore ipsum eu aliquip. Culpa in duis amet non velit qui non ullamco sit adipisicing. Ut sunt Lorem mollit exercitation deserunt officia sunt ipsum eu amet.&knob-multiline Legend labels=true&knob-showLegendExtra=true&knob-use long labels=true&knob-vAlign_Legend=bottom',
);
});

it('should render color picker on mouse click', async () => {
const action = async () => {
await common.clickMouseRelativeToDOMElement({ left: 0, top: 0 }, '.echLegendItem__color');
Expand Down Expand Up @@ -160,25 +172,40 @@ describe('Legend stories', () => {
},
);
});

describe('Legend inside chart', () => {
const getPositionalUrl = (p1: string, p2: string, others: string = '') =>
`http://localhost:9001/?path=/story/legend--inside-chart&knob-vAlign_Legend=${p1}&knob-hAlign_Legend=${p2}${others}`;
Comment on lines +183 to +184
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixes knob to align with correct vAlign and hAlign properties.


it.each([
[Position.Top, Position.Left],
[Position.Top, Position.Right],
[Position.Bottom, Position.Left],
[Position.Bottom, Position.Right],
])('should correctly display %s %s', async (pos1, pos2) => {
await common.expectChartAtUrlToMatchScreenshot(
`http://localhost:9001/?path=/story/legend--inside-chart&knob-Legend Position[0]=${pos1}&knob-Legend Position[1]=${pos2}&globals=themes.value:Light`,
);
])('should correctly display %s %s', async (p1, p2) => {
await common.expectChartAtUrlToMatchScreenshot(getPositionalUrl(p1, p2, '&globals=themes.value:Light'));
});

it.each([
[Position.Top, Position.Left],
[Position.Top, Position.Right],
[Position.Bottom, Position.Left],
[Position.Bottom, Position.Right],
])('should correctly display %s %s in dark mode', async (p1, p2) => {
await common.expectChartAtUrlToMatchScreenshot(getPositionalUrl(p1, p2, '&globals=themes.value:Dark'));
});

it.each([
[Position.Top, Position.Left],
[Position.Top, Position.Right],
[Position.Bottom, Position.Left],
[Position.Bottom, Position.Right],
])('should correctly display %s %s in dark mode', async (pos1, pos2) => {
])('should correctly display %s %s in multiline mode', async (p1, p2) => {
const longLabel =
'Non do aliqua veniam dolore ipsum eu aliquip. Culpa in duis amet non velit qui non ullamco sit adipisicing. Ut sunt Lorem mollit exercitation deserunt officia sunt ipsum eu amet.';

await common.expectChartAtUrlToMatchScreenshot(
`http://localhost:9001/?path=/story/legend--inside-chart&knob-Legend Position[0]=${pos1}&knob-Legend Position[1]=${pos2}&globals=themes.value:Dark`,
getPositionalUrl(p1, p2, `&knob-multiline label_Legend=true&knob-long label_Legend=${longLabel}`),
);
});
});
Expand Down
32 changes: 16 additions & 16 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,20 @@
},
"scripts": {
"prepack": "echo 'This package is not published, see pacakges/*' && exit 1",
"autoprefix:css": "lerna run --loglevel=silent --scope @elastic/charts autoprefix:css --stream",
"api:check": "lerna run --loglevel=silent --scope @elastic/charts api:check --stream",
"api:check:local": "lerna run --loglevel=silent --scope @elastic/charts api:check:local --stream",
"api:extract": "lerna run --loglevel=silent --scope @elastic/charts api:extract --stream",
"autoprefix:css": "lerna run --loglevel=silent --scope @elastic/charts autoprefix:css --stream --no-prefix",
"api:check": "lerna run --loglevel=silent --scope @elastic/charts api:check --stream --no-prefix",
"api:check:local": "lerna run --loglevel=silent --scope @elastic/charts api:check:local --stream --no-prefix",
"api:extract": "lerna run --loglevel=silent --scope @elastic/charts api:extract --stream --no-prefix",
"backport": "backport",
"build": "lerna run --loglevel=silent --scope @elastic/charts build --stream",
"build:ts": "lerna run --loglevel=silent --scope @elastic/charts build:ts --stream",
"build:css": "lerna run --loglevel=silent --scope @elastic/charts build:css --stream",
"build:clean": "lerna run --loglevel=silent --scope @elastic/charts build:clean --stream",
"build:compile": "lerna run --loglevel=silent --scope @elastic/charts build:compile --stream",
"build:sass": "lerna run --loglevel=silent --scope @elastic/charts build:sass --stream",
"build:check": "lerna run --loglevel=silent --scope @elastic/charts build:check --stream",
"build:watch": "lerna run --loglevel=silent --scope @elastic/charts build:watch --stream",
"concat:sass": "lerna run --loglevel=silent --scope @elastic/charts concat:sass --stream",
"build": "lerna run --loglevel=silent --scope @elastic/charts build --stream --no-prefix",
"build:ts": "lerna run --loglevel=silent --scope @elastic/charts build:ts --stream --no-prefix",
"build:css": "lerna run --loglevel=silent --scope @elastic/charts build:css --stream --no-prefix",
"build:clean": "lerna run --loglevel=silent --scope @elastic/charts build:clean --stream --no-prefix",
"build:compile": "lerna run --loglevel=silent --scope @elastic/charts build:compile --stream --no-prefix",
"build:sass": "lerna run --loglevel=silent --scope @elastic/charts build:sass --stream --no-prefix",
"build:check": "lerna run --loglevel=silent --scope @elastic/charts build:check --stream --no-prefix",
"build:watch": "lerna run --loglevel=silent --scope @elastic/charts build:watch --stream --no-prefix",
"concat:sass": "lerna run --loglevel=silent --scope @elastic/charts concat:sass --stream --no-prefix",
"cz": "git-cz",
"lint": "NODE_ENV=production eslint --quiet --ext .tsx,.ts,.js .",
"lint:fix": "yarn lint --fix",
Expand All @@ -36,8 +36,8 @@
"pq": "pretty-quick",
"semantic-release": "semantic-release",
"start": "yarn storybook",
"storybook": "lerna run --scope charts-storybook start --stream",
"storybook:build": "lerna run --scope charts-storybook build --stream",
"storybook": "lerna run --scope charts-storybook start --stream --no-prefix",
"storybook:build": "lerna run --scope charts-storybook build --stream --no-prefix",
"test": "jest --verbose --config jest.config.js",
"test:tz": "yarn test:tz-utc && yarn test:tz-ny && yarn test:tz-jp",
"test:tz-utc": "TZ=UTC jest --verbose --config=jest.tz.config.js",
Expand All @@ -52,7 +52,7 @@
"test:integration:generate:examples": "./scripts/extract_examples.sh",
"test:integration:generate:page": "./scripts/compile_vrt_page.sh",
"test:integration:server": "cd integration/server && RNG_SEED='elastic-charts' webpack serve --content-base integration/server",
"typecheck:src": "lerna run --loglevel=silent --scope @elastic/charts typecheck --stream",
"typecheck:src": "lerna run --loglevel=silent --scope @elastic/charts typecheck --stream --no-prefix",
"typecheck:all": "tsc -p ./tsconfig.json --noEmit",
"ts:prune": "ts-prune",
"link:kibana": "node ./packages/link_kibana"
Expand Down
6 changes: 6 additions & 0 deletions packages/charts/api/charts.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -1147,6 +1147,11 @@ export interface LegendColorPickerProps {
// @public (undocumented)
export type LegendItemListener = (series: SeriesIdentifier[]) => void;

// @public (undocumented)
export interface LegendLabelOptions {
multiline: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In partition chart, not for the legend, we use a maxRowCount property, which might be useful here and be a slightly more uniform API. So, a value of 1 would be the default, and it could be whatever number incl. Infinity if unbounded. While it's not integral to the task, it feels useful to set some sensible row count, and with a maxRowCount there's no need to break the API in the future, or introduce an additional property, which is only applicable with a multiline: true value. If a row count is deemed useful, we can preemptively cut down on ifs, ternaries and union types 😸

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah so you are thinking change multiline option to maxRowCount?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done here d8be979

}

// @public
export type LegendPath = LegendPathElement[];

Expand Down Expand Up @@ -1204,6 +1209,7 @@ export type LegendStrategy = $Values<typeof LegendStrategy>;
// @public (undocumented)
export interface LegendStyle {
horizontalHeight: number;
labelOptions?: LegendLabelOptions;
margin: number;
spacingBuffer: number;
verticalWidth: number;
Expand Down
28 changes: 15 additions & 13 deletions packages/charts/src/components/__snapshots__/chart.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -24,21 +24,23 @@ exports[`Chart should render the legend name test 1`] = `
<div className=\\"echLegend echLegend--debug echLegend--vertical echLegend--right echLegend--top\\" style={{...}}>
<div style={{...}} className=\\"echLegendListContainer\\">
<ul style={{...}} className=\\"echLegendList\\">
<LegendItem item={{...}} positionConfig={{...}} totalItems={1} extraValues={{...}} showExtra={false} onMouseOut={[undefined]} onMouseOver={[undefined]} onClick={[undefined]} clearTemporaryColorsAction={[Function (anonymous)]} setPersistedColorAction={[Function (anonymous)]} setTemporaryColorAction={[Function (anonymous)]} mouseOutAction={[Function (anonymous)]} mouseOverAction={[Function (anonymous)]} toggleDeselectSeriesAction={[Function (anonymous)]} colorPicker={[undefined]} action={[undefined]}>
<LegendItem item={{...}} positionConfig={{...}} totalItems={1} extraValues={{...}} showExtra={false} onMouseOut={[undefined]} onMouseOver={[undefined]} onClick={[undefined]} clearTemporaryColorsAction={[Function (anonymous)]} setPersistedColorAction={[Function (anonymous)]} setTemporaryColorAction={[Function (anonymous)]} mouseOutAction={[Function (anonymous)]} mouseOverAction={[Function (anonymous)]} toggleDeselectSeriesAction={[Function (anonymous)]} colorPicker={[undefined]} action={[undefined]} labelOptions={[undefined]}>
<li className=\\"echLegendItem echLegendItem--vertical\\" onMouseEnter={[Function (anonymous)]} onMouseLeave={[Function (anonymous)]} style={[undefined]} data-ech-series-name=\\"test\\">
<div className=\\"background\\" />
<ForwardRef color=\\"#1EA593\\" seriesName=\\"test\\" isSeriesHidden={false} hasColorPicker={false} onClick={[undefined]} pointStyle={[undefined]}>
<div className=\\"echLegendItem__color\\" title=\\"series color\\">
<LegendIcon pointStyle={[undefined]} color=\\"#1EA593\\" ariaLabel=\\"series color: #1EA593\\">
<svg width={16} height={16} aria-label=\\"series color: #1EA593\\">
<g transform=\\"\\\\n translate(8, 8)\\\\n rotate(0)\\\\n \\">
<path d=\\"M -3.5 0 a 3.5,3.5 0 1,0 7,0 a 3.5,3.5 0 1,0 -7,0\\" stroke=\\"#1EA593\\" strokeWidth={1} fill=\\"#1EA593\\" opacity={1} />
</g>
</svg>
</LegendIcon>
</div>
</ForwardRef>
<Label label=\\"test\\" isToggleable={false} onClick={[undefined]} isSeriesHidden={false}>
<div className=\\"colorWrapper\\">
<ForwardRef color=\\"#1EA593\\" seriesName=\\"test\\" isSeriesHidden={false} hasColorPicker={false} onClick={[undefined]} pointStyle={[undefined]}>
<div className=\\"echLegendItem__color\\" title=\\"series color\\">
<LegendIcon pointStyle={[undefined]} color=\\"#1EA593\\" ariaLabel=\\"series color: #1EA593\\">
<svg width={16} height={16} aria-label=\\"series color: #1EA593\\">
<g transform=\\"\\\\n translate(8, 8)\\\\n rotate(0)\\\\n \\">
<path d=\\"M -3.5 0 a 3.5,3.5 0 1,0 7,0 a 3.5,3.5 0 1,0 -7,0\\" stroke=\\"#1EA593\\" strokeWidth={1} fill=\\"#1EA593\\" opacity={1} />
</g>
</svg>
</LegendIcon>
</div>
</ForwardRef>
</div>
<Label label=\\"test\\" options={[undefined]} isToggleable={false} onClick={[undefined]} isSeriesHidden={false}>
<div className=\\"echLegendItem__label\\" title=\\"test\\" onClick={[undefined]}>
test
</div>
Expand Down
Loading