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.

Sign up for GitHub

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

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

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
aff29d4
feat(legend): multiline labels
nickofthyme Aug 4, 2021
477f611
fix: api changes
nickofthyme Aug 5, 2021
b60160f
test: update jest snapshots
nickofthyme Aug 5, 2021
5b2bc29
Merge branch 'master' into multiline-legend-labels
nickofthyme Aug 5, 2021
bec8643
test: add vrts for new multiline option
nickofthyme Aug 5, 2021
1859545
Merge remote-tracking branch 'origin' into multiline-legend-labels
nickofthyme Aug 5, 2021
d6fb516
Merge remote-tracking branch 'origin' into multiline-legend-labels
nickofthyme Aug 5, 2021
fcd45ca
Merge remote-tracking branch 'origin/multiline-legend-labels' into mu…
nickofthyme Aug 5, 2021
88ebac5
fix: align to top per marcos comments
nickofthyme Aug 5, 2021
bdee07e
fix: labels with long words, now hyphenated
nickofthyme Aug 5, 2021
d8be979
refactor: use max line prop instead of multiline boolean, add vrts
nickofthyme Aug 5, 2021
6800bc2
test: update snapshots
nickofthyme Aug 5, 2021
5959a18
test: update vrt screenshots
nickofthyme Aug 5, 2021
575b6da
fix: api changes
nickofthyme Aug 6, 2021
697f73d
fix: test and linting issues
nickofthyme Aug 6, 2021
f796ae3
fix: attempted fix of flaky vrt
nickofthyme Aug 6, 2021
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
32 changes: 16 additions & 16 deletions package.json
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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