feat(legend): multiline labels with maxLines option #1285
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,26 +9,30 @@ | |
| import classNames from 'classnames'; | ||
| import React, { MouseEventHandler } from 'react'; | ||
|
|
||
| import { LegendLabelOptions } from '../../utils/themes/theme'; | ||
|
|
||
| interface LabelProps { | ||
| label: string; | ||
| isSeriesHidden?: boolean; | ||
| isToggleable?: boolean; | ||
| onClick?: MouseEventHandler; | ||
| options?: LegendLabelOptions; | ||
|
There was a problem hiding this comment. Minor: same here, maybe all such optionality like can go away except for the single public API enty point. I admit to having done plenty of this in code I wrote, more recently thinking that it'd be more economical to reify user input right away, and then avoid having to worry about partials, optionals etc. We can think about removing question marks from non- There was a problem hiding this comment. Done here d8be979 |
||
| } | ||
| /** | ||
| * Label component used to display text in legend item | ||
| * @internal | ||
| */ | ||
| export function Label({ label, isToggleable, onClick, isSeriesHidden, options }: LabelProps) { | ||
| const labelClassNames = classNames('echLegendItem__label', { | ||
| 'echLegendItem__label--clickable': Boolean(onClick), | ||
| 'echLegendItem__label--multiline': options?.multiline, | ||
| }); | ||
|
|
||
| return isToggleable ? ( | ||
| <button | ||
| type="button" | ||
| className={labelClassNames} | ||
| title={options?.multiline ? '' : label} // full text already visible | ||
|
There was a problem hiding this comment. I don't think the title is necessary if the full label is visible |
||
| onClick={onClick} | ||
| aria-label={ | ||
| isSeriesHidden ? `${label}; Activate to show series in graph` : `${label}; Activate to hide series in graph` | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,6 +30,7 @@ import { | |
| } from '../../state/actions/legend'; | ||
| import { Color, LayoutDirection } from '../../utils/common'; | ||
| import { deepEqual } from '../../utils/fast_deep_equal'; | ||
| import { LegendLabelOptions } from '../../utils/themes/theme'; | ||
| import { Color as ItemColor } from './color'; | ||
| import { renderExtra } from './extra'; | ||
| import { Label as ItemLabel } from './label'; | ||
|
|
@@ -45,6 +46,7 @@ export interface LegendItemProps { | |
| positionConfig: LegendPositionConfig; | ||
| extraValues: Map<string, LegendItemExtraValues>; | ||
| showExtra: boolean; | ||
| labelOptions?: LegendLabelOptions; | ||
|
There was a problem hiding this comment. Nit: could it be relatively easily solved such that it's only the user facing config API where this is optional, but we can rely on its presence everywhere downstream? Ie. one question mark only There was a problem hiding this comment. You saying There was a problem hiding this comment. Yes, it's an internal type, by this time the reification (augmentation with defaults etc.) of the user input should've happened (if we have shared buy-in of how it often simplifies the implementation and prevents us from possibly handling the non-specified nullish case at multiple places, so also a DRY principle) There was a problem hiding this comment. Done here d8be979 |
||
| colorPicker?: LegendColorPicker; | ||
| action?: LegendAction; | ||
| onClick?: LegendItemListener; | ||
|
|
@@ -163,7 +165,16 @@ export class LegendListItem extends Component<LegendItemProps, LegendItemState> | |
| } | ||
|
|
||
| render() { | ||
| const { | ||
| extraValues, | ||
| item, | ||
| showExtra, | ||
| colorPicker, | ||
| totalItems, | ||
| action: Action, | ||
| positionConfig, | ||
| labelOptions, | ||
| } = this.props; | ||
| const { color, isSeriesHidden, isItemHidden, seriesIdentifiers, label, pointStyle } = item; | ||
|
|
||
| if (isItemHidden) return null; | ||
|
|
@@ -189,17 +200,20 @@ export class LegendListItem extends Component<LegendItemProps, LegendItemState> | |
| data-ech-series-name={label} | ||
| > | ||
| <div className="background" /> | ||
| <div className="colorWrapper"> | ||
| <ItemColor | ||
| ref={this.colorRef} | ||
| color={color} | ||
| seriesName={label} | ||
| isSeriesHidden={isSeriesHidden} | ||
| hasColorPicker={hasColorPicker} | ||
| onClick={this.handleColorClick(hasColorPicker)} | ||
| pointStyle={pointStyle} | ||
| /> | ||
| </div> | ||
|
Comment on lines
+203
to
+213
There was a problem hiding this comment. This is to prevent jitter when using |
||
| <ItemLabel | ||
| label={label} | ||
| options={labelOptions} | ||
| isToggleable={totalItems > 1 && item.isToggleable} | ||
| onClick={this.handleLabelClick(seriesIdentifiers)} | ||
| isSeriesHidden={isSeriesHidden} | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes the focus size of the action the same as the color dot