From b59fb13b9e6b0af0d8ee9347535b2d5dbe06aedf Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Wed, 14 Aug 2019 13:12:50 -0500 Subject: [PATCH 1/9] feat(legend): remove ui legend toggle Remove UI legend toggle. showLegend prop on settings unchanged. #268 --- .../xy_chart/store/chart_state.test.ts | 8 ---- src/chart_types/xy_chart/store/chart_state.ts | 10 +---- src/components/chart.tsx | 2 - src/components/legend/_index.scss | 1 - src/components/legend/_legend.scss | 4 -- src/components/legend/_legend_button.scss | 23 ---------- src/components/legend/legend.tsx | 17 +------ src/components/legend/legend_button.tsx | 44 ------------------- .../react_canvas/reactive_chart.tsx | 6 +-- 9 files changed, 5 insertions(+), 110 deletions(-) delete mode 100644 src/components/legend/_legend_button.scss delete mode 100644 src/components/legend/legend_button.tsx diff --git a/src/chart_types/xy_chart/store/chart_state.test.ts b/src/chart_types/xy_chart/store/chart_state.test.ts index 6a30bc6e7a..d60c76487d 100644 --- a/src/chart_types/xy_chart/store/chart_state.test.ts +++ b/src/chart_types/xy_chart/store/chart_state.test.ts @@ -108,14 +108,6 @@ describe('Chart Store', () => { expect(axesTicks.get(AXIS_ID)).not.toBeUndefined(); }); - test('can toggle legend visibility', () => { - store.toggleLegendCollapsed(); - expect(store.legendCollapsed.get()).toBe(true); - - store.toggleLegendCollapsed(); - expect(store.legendCollapsed.get()).toBe(false); - }); - test('can set legend visibility', () => { store.showLegend.set(false); store.setShowLegend(true); diff --git a/src/chart_types/xy_chart/store/chart_state.ts b/src/chart_types/xy_chart/store/chart_state.ts index f464807810..5c074f36aa 100644 --- a/src/chart_types/xy_chart/store/chart_state.ts +++ b/src/chart_types/xy_chart/store/chart_state.ts @@ -221,15 +221,9 @@ export class ChartStore { canDataBeAnimated = false; showLegend = observable.box(false); - legendCollapsed = observable.box(false); legendPosition: Position | undefined; showLegendDisplayValue = observable.box(true); - toggleLegendCollapsed = action(() => { - this.legendCollapsed.set(!this.legendCollapsed.get()); - this.computeChart(); - }); - /** * determine if crosshair cursor should be visible based on cursor position and brush enablement */ @@ -852,7 +846,7 @@ export class ChartStore { this.chartTheme, this.axesTicksDimensions, this.axesSpecs, - this.showLegend.get() && !this.legendCollapsed.get(), + this.showLegend.get(), this.legendPosition, ); @@ -891,7 +885,7 @@ export class ChartStore { this.chartDimensions, this.chartTheme, this.chartRotation, - this.showLegend.get() && !this.legendCollapsed.get(), + this.showLegend.get(), this.axesSpecs, this.axesTicksDimensions, xDomain, diff --git a/src/components/chart.tsx b/src/components/chart.tsx index 26ef547c89..f5c548c586 100644 --- a/src/components/chart.tsx +++ b/src/components/chart.tsx @@ -9,7 +9,6 @@ import { ChartResizer } from './chart_resizer'; import { Crosshair } from './crosshair'; import { Highlighter } from './highlighter'; import { Legend } from './legend/legend'; -import { LegendButton } from './legend/legend_button'; import { ReactiveChart as ReactChart } from './react_canvas/reactive_chart'; // import { ReactiveChart as SVGChart } from './svg/reactive_chart'; import { Tooltips } from './tooltips'; @@ -60,7 +59,6 @@ export class Chart extends React.Component { - diff --git a/src/components/legend/_index.scss b/src/components/legend/_index.scss index 8caba8f13e..48c6d3a719 100644 --- a/src/components/legend/_index.scss +++ b/src/components/legend/_index.scss @@ -1,5 +1,4 @@ @import 'variables'; @import 'legend'; -@import 'legend_button'; @import 'legend_list'; @import 'legend_item'; diff --git a/src/components/legend/_legend.scss b/src/components/legend/_legend.scss index 32703da480..e1cc7c1ec0 100644 --- a/src/components/legend/_legend.scss +++ b/src/components/legend/_legend.scss @@ -6,10 +6,6 @@ overflow-y: hidden; } -.echLegend--collapsed { - display: none; -} - .echLegend--debug { background: red; } diff --git a/src/components/legend/_legend_button.scss b/src/components/legend/_legend_button.scss deleted file mode 100644 index e634285241..0000000000 --- a/src/components/legend/_legend_button.scss +++ /dev/null @@ -1,23 +0,0 @@ -.echLegendButton { - padding: $euiSizeXS; - line-height: 1; - opacity: 0.35; - border-radius: $euiBorderRadius; - background-color: $euiColorEmptyShade; - position: absolute; - bottom: 0; - left: 0; - transition: - opacity $euiAnimSpeedFast $euiAnimSlightResistance, - background-color $euiAnimSpeedFast $euiAnimSlightResistance; - - &:hover, - &:focus { - opacity: 1; - background-color: $euiFocusBackgroundColor; - } -} - -.echLegendButton--isOpen { - opacity: 1; -} diff --git a/src/components/legend/legend.tsx b/src/components/legend/legend.tsx index 79049e824e..18cdec197e 100644 --- a/src/components/legend/legend.tsx +++ b/src/components/legend/legend.tsx @@ -14,28 +14,15 @@ interface LegendProps { class LegendComponent extends React.Component { static displayName = 'Legend'; - onCollapseLegend = () => { - this.props.chartStore!.toggleLegendCollapsed(); - }; - render() { const { legendId } = this.props; - const { - initialized, - legendItems, - legendPosition, - showLegend, - legendCollapsed, - debug, - chartTheme, - } = this.props.chartStore!; + const { initialized, legendItems, legendPosition, showLegend, debug, chartTheme } = this.props.chartStore!; if (!showLegend.get() || !initialized.get() || legendItems.size === 0 || legendPosition === undefined) { return null; } const legendClasses = classNames('echLegend', `echLegend--${legendPosition}`, { - 'echLegend--collapsed': legendCollapsed.get(), 'echLegend--debug': debug, }); let paddingStyle; @@ -46,7 +33,7 @@ class LegendComponent extends React.Component { }; } return ( -
+
{[...legendItems.values()].map(this.renderLegendElement)}
diff --git a/src/components/legend/legend_button.tsx b/src/components/legend/legend_button.tsx deleted file mode 100644 index 7fd0cd9466..0000000000 --- a/src/components/legend/legend_button.tsx +++ /dev/null @@ -1,44 +0,0 @@ -import classNames from 'classnames'; -import { inject, observer } from 'mobx-react'; -import React from 'react'; -import { ChartStore } from '../../chart_types/xy_chart/store/chart_state'; -import { Icon } from '../icons/icon'; - -interface LegendButtonProps { - chartStore?: ChartStore; - legendId: string; -} - -class LegendButtonComponent extends React.Component { - static displayName = 'Legend'; - onCollapseLegend = () => { - this.props.chartStore!.toggleLegendCollapsed(); - }; - - render() { - const { initialized, legendItems, legendCollapsed, showLegend } = this.props.chartStore!; - - if (!showLegend.get() || !initialized.get() || legendItems.size === 0) { - return null; - } - const isOpen = !legendCollapsed.get(); - const className = classNames('echLegendButton', { - 'echLegendButton--isOpen': isOpen, - }); - return ( - - ); - } -} - -export const LegendButton = inject('chartStore')(observer(LegendButtonComponent)); diff --git a/src/components/react_canvas/reactive_chart.tsx b/src/components/react_canvas/reactive_chart.tsx index a347d2da96..06fa004f4a 100644 --- a/src/components/react_canvas/reactive_chart.tsx +++ b/src/components/react_canvas/reactive_chart.tsx @@ -354,13 +354,11 @@ class Chart extends React.Component { debug, setCursorPosition, isChartEmpty, - legendCollapsed, legendPosition, chartTheme, } = this.props.chartStore!; if (isChartEmpty) { - const isLegendCollapsed = legendCollapsed.get(); const { verticalWidth, horizontalHeight } = chartTheme.legend; const paddingStyle = @@ -372,11 +370,9 @@ class Chart extends React.Component { ? { paddingTop: horizontalHeight } : { paddingTop: -horizontalHeight }; - const style = isLegendCollapsed ? undefined : paddingStyle; - return (
-

No data to display

+

No data to display

); } From e409eb29fedc33a7875f6803d0477ef1f6e22d0a Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Wed, 14 Aug 2019 22:13:39 -0500 Subject: [PATCH 2/9] fix(legend): allow content to set size --- .playground/playgroud.tsx | 5 +- src/chart_types/xy_chart/store/chart_state.ts | 21 +++++--- src/chart_types/xy_chart/utils/axis_utils.ts | 27 +--------- src/chart_types/xy_chart/utils/dimensions.ts | 32 ++---------- src/components/_container.scss | 16 ++++-- src/components/_global.scss | 3 ++ src/components/_index.scss | 1 + src/components/chart.tsx | 50 +++++++++++++------ src/components/legend/_legend.scss | 34 ++++--------- src/components/legend/_legend_item.scss | 5 +- src/components/legend/legend.tsx | 45 +++++++++++++---- .../react_canvas/reactive_chart.tsx | 11 ++-- src/specs/settings.tsx | 5 +- src/specs/specs_parser.tsx | 3 +- 14 files changed, 134 insertions(+), 124 deletions(-) create mode 100644 src/components/_global.scss diff --git a/.playground/playgroud.tsx b/.playground/playgroud.tsx index 4a9f51fa9f..d093bd93ad 100644 --- a/.playground/playgroud.tsx +++ b/.playground/playgroud.tsx @@ -16,7 +16,7 @@ import { KIBANA_METRICS } from '../src/utils/data_samples/test_dataset_kibana'; export class Playground extends React.Component { render() { - return <>{this.renderChart(Position.Right)}; + return <>{this.renderChart(Position.Top)}; } renderChart(legendPosition: Position) { const theme = mergeWithDefaultTheme({ @@ -33,7 +33,6 @@ export class Playground extends React.Component { }, }, }); - console.log(theme.areaSeriesStyle); return (
@@ -87,7 +86,7 @@ export class Playground extends React.Component { }} /> (Position.Right); showLegendDisplayValue = observable.box(true); /** @@ -760,7 +761,7 @@ export class ChartStore { } computeChart() { - this.initialized.set(false); + this.chartInitialized.set(false); // compute only if parent dimensions are computed if (this.parentDimensions.width === 0 || this.parentDimensions.height === 0) { return; @@ -806,6 +807,14 @@ export class ChartStore { this.deselectedDataSeries, ); + if (!this.legendInitialized.get()) { + this.legendInitialized.set(true); + + if (this.legendItems.size > 0 && this.showLegend.get()) { + return; + } + } + this.isChartEmpty = isAllSeriesDeselected(this.legendItems); const { xDomain, yDomain, formattedDataSeries } = this.seriesDomainsAndData; @@ -846,8 +855,6 @@ export class ChartStore { this.chartTheme, this.axesTicksDimensions, this.axesSpecs, - this.showLegend.get(), - this.legendPosition, ); this.chartTransform = computeChartTransform(this.chartDimensions, this.chartRotation); @@ -885,14 +892,12 @@ export class ChartStore { this.chartDimensions, this.chartTheme, this.chartRotation, - this.showLegend.get(), this.axesSpecs, this.axesTicksDimensions, xDomain, yDomain, totalBarsInCluster, this.enableHistogramMode.get(), - this.legendPosition, barsPadding, ); // tslint:disable-next-line:no-console @@ -920,6 +925,6 @@ export class ChartStore { // temporary disabled until // https://github.com/elastic/elastic-charts/issues/89 and https://github.com/elastic/elastic-charts/issues/41 this.canDataBeAnimated = false; - this.initialized.set(true); + this.chartInitialized.set(true); } } diff --git a/src/chart_types/xy_chart/utils/axis_utils.ts b/src/chart_types/xy_chart/utils/axis_utils.ts index 46d1f83218..150794e65f 100644 --- a/src/chart_types/xy_chart/utils/axis_utils.ts +++ b/src/chart_types/xy_chart/utils/axis_utils.ts @@ -531,18 +531,15 @@ export function getAxisTicksPositions( chartDimensions: Dimensions, chartTheme: Theme, chartRotation: Rotation, - showLegend: boolean, axisSpecs: Map, axisDimensions: Map, xDomain: XDomain, yDomain: YDomain[], totalGroupsCount: number, enableHistogramMode: boolean, - legendPosition?: Position, barsPadding?: number, ) { const { chartPaddings, chartMargins } = chartTheme; - const legendStyle = chartTheme.legend; const axisPositions: Map = new Map(); const axisVisibleTicks: Map = new Map(); const axisTicks: Map = new Map(); @@ -552,27 +549,7 @@ export function getAxisTicksPositions( let cumBottomSum = chartPaddings.bottom; let cumLeftSum = 0; let cumRightSum = chartPaddings.right; - if (showLegend) { - switch (legendPosition) { - case Position.Left: - cumLeftSum += legendStyle.verticalWidth; - break; - // case Position.Right: - // cumRightSum += legendStyle.verticalWidth; - // break; - // case Position.Bottom: - // cumBottomSum += legendStyle.horizontalHeight; - // break; - case Position.Top: - cumTopSum += legendStyle.horizontalHeight; - break; - } - } - // console.log({cumRightSum}); - // let cumTopSum = showLegend ? legendStyle.horizontalHeight : 0; - // let cumBottomSum = chartConfig.paddings.bottom; - // let cumLeftSum = showLegend ? legendStyle.verticalWidth : 0; - // let cumRightSum = chartConfig.paddings.right; + axisDimensions.forEach((axisDim, id) => { const axisSpec = axisSpecs.get(id); @@ -661,7 +638,7 @@ export function isVertical(position: Position) { } export function isHorizontal(position: Position) { - return !isVertical(position); + return position === Position.Top || position === Position.Bottom; } export function isLowerBound(domain: Partial): domain is LowerBoundedDomain { diff --git a/src/chart_types/xy_chart/utils/dimensions.ts b/src/chart_types/xy_chart/utils/dimensions.ts index 5ff1e0b82c..045a8e9725 100644 --- a/src/chart_types/xy_chart/utils/dimensions.ts +++ b/src/chart_types/xy_chart/utils/dimensions.ts @@ -19,11 +19,8 @@ export function computeChartDimensions( chartTheme: Theme, axisDimensions: Map, axisSpecs: Map, - showLegend: boolean, - legendPosition?: Position, ): Dimensions { const { chartMargins, chartPaddings } = chartTheme; - const legendStyle = chartTheme.legend; const { axisTitleStyle } = chartTheme.axes; const axisTitleHeight = axisTitleStyle.fontSize + axisTitleStyle.padding; @@ -72,37 +69,18 @@ export function computeChartDimensions( if (vRightAxisSpecWidth === 0) { hMargin += chartMargins.right; } - let legendTopMargin = 0; - let legendLeftMargin = 0; - if (showLegend) { - switch (legendPosition) { - case Position.Right: - hMargin += legendStyle.verticalWidth; - break; - case Position.Left: - hMargin += legendStyle.verticalWidth; - legendLeftMargin = legendStyle.verticalWidth; - break; - case Position.Top: - vMargin += legendStyle.horizontalHeight; - legendTopMargin = legendStyle.horizontalHeight; - break; - case Position.Bottom: - vMargin += legendStyle.horizontalHeight; - break; - } - } + let top = 0; let left = 0; if (hTopAxisSpecHeight === 0) { - top = chartMargins.top + chartPaddings.top + legendTopMargin; + top = chartMargins.top + chartPaddings.top; } else { - top = hTopAxisSpecHeight + chartPaddings.top + legendTopMargin; + top = hTopAxisSpecHeight + chartPaddings.top; } if (vLeftAxisSpecWidth === 0) { - left = chartMargins.left + chartPaddings.left + legendLeftMargin; + left = chartMargins.left + chartPaddings.left; } else { - left = vLeftAxisSpecWidth + chartPaddings.left + legendLeftMargin; + left = vLeftAxisSpecWidth + chartPaddings.left; } return { top, diff --git a/src/components/_container.scss b/src/components/_container.scss index a1dd5b6068..eb1576b225 100644 --- a/src/components/_container.scss +++ b/src/components/_container.scss @@ -3,9 +3,8 @@ */ .echContainer { + flex: 1; position: relative; - width: 100%; - height: 100%; &:hover { .echLegend__toggle { @@ -14,6 +13,15 @@ } } -.echChart--isBrushEnabled { - cursor: crosshair; +.echChart { + display: flex; + height: 100%; + + &--isBrushEnabled { + cursor: crosshair; + } + + &--column { + flex-direction: column; + } } diff --git a/src/components/_global.scss b/src/components/_global.scss new file mode 100644 index 0000000000..8099c94c55 --- /dev/null +++ b/src/components/_global.scss @@ -0,0 +1,3 @@ +.invisible { + visibility: hidden; +} diff --git a/src/components/_index.scss b/src/components/_index.scss index 857235a188..de8ee6711d 100644 --- a/src/components/_index.scss +++ b/src/components/_index.scss @@ -1,3 +1,4 @@ +@import 'global'; @import 'container'; @import 'annotation'; @import 'crosshair'; diff --git a/src/components/chart.tsx b/src/components/chart.tsx index f5c548c586..e33fa26632 100644 --- a/src/components/chart.tsx +++ b/src/components/chart.tsx @@ -1,6 +1,7 @@ +import React, { CSSProperties } from 'react'; import classNames from 'classnames'; import { Provider } from 'mobx-react'; -import React, { CSSProperties, Fragment } from 'react'; + import { SpecsParser } from '../specs/specs_parser'; import { ChartStore } from '../chart_types/xy_chart/store/chart_state'; import { htmlIdGenerator } from '../utils/commons'; @@ -10,8 +11,9 @@ import { Crosshair } from './crosshair'; import { Highlighter } from './highlighter'; import { Legend } from './legend/legend'; import { ReactiveChart as ReactChart } from './react_canvas/reactive_chart'; -// import { ReactiveChart as SVGChart } from './svg/reactive_chart'; import { Tooltips } from './tooltips'; +import { isHorizontal } from '../chart_types/xy_chart/utils/axis_utils'; +import { Position } from '../chart_types/xy_chart/utils/specs'; interface ChartProps { /** The type of rendered @@ -22,7 +24,11 @@ interface ChartProps { className?: string; } -export class Chart extends React.Component { +interface ChartState { + legendPosition: Position; +} + +export class Chart extends React.Component { static defaultProps: ChartProps = { renderer: 'canvas', }; @@ -31,26 +37,43 @@ export class Chart extends React.Component { constructor(props: any) { super(props); this.chartSpecStore = new ChartStore(); + this.state = { + legendPosition: this.chartSpecStore.legendPosition.get(), + }; + // value is set to chart_store in settings so need to watch the value + this.chartSpecStore.legendPosition.observe(({ newValue: legendPosition }) => { + this.setState({ + legendPosition, + }); + }); this.legendId = htmlIdGenerator()('legend'); } - render() { - const { renderer, size, className } = this.props; - let containerStyle: CSSProperties; + + static getContainerStyle = (size: any): CSSProperties => { if (size) { - containerStyle = { + return { position: 'relative', width: size[0], height: size[1], }; - } else { - containerStyle = {}; } - const chartClass = classNames('echContainer', className); + return {}; + }; + + render() { + const { renderer, size, className } = this.props; + const containerStyle = Chart.getContainerStyle(size); + const Horizontal = isHorizontal(this.state.legendPosition); + const chartClassNames = classNames('echChart', className, { + 'echChart--column': Horizontal, + }); + return ( - +
+ {this.props.children} -
+
{// TODO reenable when SVG rendered is aligned with canvas one @@ -58,10 +81,9 @@ export class Chart extends React.Component { {renderer === 'canvas' && } -
- +
); } diff --git a/src/components/legend/_legend.scss b/src/components/legend/_legend.scss index e1cc7c1ec0..a60420f2f4 100644 --- a/src/components/legend/_legend.scss +++ b/src/components/legend/_legend.scss @@ -1,11 +1,5 @@ // Legend -.echLegend { - // Margin supplied in JS to match chart margins - position: absolute !important; // Override shadow - overflow-y: hidden; -} - .echLegend--debug { background: red; } @@ -13,8 +7,7 @@ .echLegend--top, .echLegend--bottom { left: $euiSizeL; - right: 0; - height: $echLegendMaxHeight; + max-height: $echLegendMaxHeight; .echLegendList { flex-direction: row; @@ -23,22 +16,22 @@ .echLegendItem { margin-right: $euiSizeL; - width: $echLegendMaxWidth; + max-width: $echLegendMaxWidth; } } - -.echLegend--top { - top: 0; +.echLegend--top, +.echLegend--left { + order: 0; } -.echLegend--bottom { - bottom: 0; + +.echLegend--bottom, +.echLegend--right { + order: 1; } .echLegend--left, .echLegend--right { - top: 0; - bottom: 0; - width: $echLegendMaxWidth; + max-width: $echLegendMaxWidth; .echLegendList { flex-direction: column; @@ -49,13 +42,6 @@ } } -.echLegend--left { - left: 0; -} -.echLegend--right { - right: 0; -} - .echLegendListContainer { @include euiYScrollWithShadows; width: 100%; diff --git a/src/components/legend/_legend_item.scss b/src/components/legend/_legend_item.scss index cd3c4e31a7..a929e1dd2b 100644 --- a/src/components/legend/_legend_item.scss +++ b/src/components/legend/_legend_item.scss @@ -1,7 +1,6 @@ .echLegendItem { color: $euiTextColor; height: $echLegendItemHeight; - width: 100%; display: flex; flex-wrap: nowrap; justify-content: space-between; @@ -30,8 +29,8 @@ .echLegendItem__title { @include euiFontSizeXS; @include euiTextTruncate; - margin-right: $euiSizeXS; flex: 1; + &:hover { cursor: pointer; } @@ -50,7 +49,9 @@ .echLegendItem__displayValue { @include euiFontSizeXS; text-align: right; + margin-left: $euiSizeM; font-feature-settings: 'tnum'; + min-width: $euiSizeL * 2; &--hidden { display: none; diff --git a/src/components/legend/legend.tsx b/src/components/legend/legend.tsx index 18cdec197e..0b58e76259 100644 --- a/src/components/legend/legend.tsx +++ b/src/components/legend/legend.tsx @@ -4,34 +4,47 @@ import React from 'react'; import { isVertical } from '../../chart_types/xy_chart/utils/axis_utils'; import { LegendItem as SeriesLegendItem } from '../../chart_types/xy_chart/legend/legend'; import { ChartStore } from '../../chart_types/xy_chart/store/chart_state'; +import { Position } from '../../chart_types/xy_chart/utils/specs'; import { LegendItem } from './legend_item'; +import { Theme } from '../../utils/themes/theme'; interface LegendProps { chartStore?: ChartStore; // FIX until we find a better way on ts mobx legendId: string; } +interface PaddingStyle { + paddingTop?: number | string; + paddingBottom?: number | string; + paddingLeft?: number | string; + paddingRight?: number | string; +} + class LegendComponent extends React.Component { static displayName = 'Legend'; render() { const { legendId } = this.props; - const { initialized, legendItems, legendPosition, showLegend, debug, chartTheme } = this.props.chartStore!; + const { + legendInitialized, + chartInitialized, + legendItems, + legendPosition, + showLegend, + debug, + chartTheme, + } = this.props.chartStore!; - if (!showLegend.get() || !initialized.get() || legendItems.size === 0 || legendPosition === undefined) { + if (!showLegend.get() || !legendInitialized.get() || legendItems.size === 0) { return null; } + const paddingStyle = this.getPaddingStyle(legendPosition.get(), chartTheme); const legendClasses = classNames('echLegend', `echLegend--${legendPosition}`, { 'echLegend--debug': debug, + invisible: !chartInitialized.get(), }); - let paddingStyle; - if (isVertical(legendPosition)) { - paddingStyle = { - paddingTop: chartTheme.chartMargins.top, - paddingBottom: chartTheme.chartMargins.bottom, - }; - } + return (
@@ -41,6 +54,20 @@ class LegendComponent extends React.Component { ); } + getPaddingStyle = (position: Position, { chartMargins }: Theme): PaddingStyle => { + if (isVertical(position)) { + return { + paddingTop: chartMargins.top, + paddingBottom: chartMargins.bottom, + }; + } + + return { + paddingLeft: chartMargins.left, + paddingRight: chartMargins.right, + }; + }; + onLegendItemMouseover = (legendItemKey: string) => () => { this.props.chartStore!.onLegendItemOver(legendItemKey); }; diff --git a/src/components/react_canvas/reactive_chart.tsx b/src/components/react_canvas/reactive_chart.tsx index 06fa004f4a..9753e04868 100644 --- a/src/components/react_canvas/reactive_chart.tsx +++ b/src/components/react_canvas/reactive_chart.tsx @@ -341,8 +341,8 @@ class Chart extends React.Component { } render() { - const { initialized } = this.props.chartStore!; - if (!initialized.get()) { + const { chartInitialized } = this.props.chartStore!; + if (!chartInitialized.get()) { return null; } @@ -360,13 +360,14 @@ class Chart extends React.Component { if (isChartEmpty) { const { verticalWidth, horizontalHeight } = chartTheme.legend; + const legendPositionValue = legendPosition.get(); const paddingStyle = - legendPosition && isVertical(legendPosition) - ? legendPosition === Position.Right + legendPositionValue && isVertical(legendPositionValue) + ? legendPositionValue === Position.Right ? { paddingLeft: -verticalWidth } : { paddingLeft: verticalWidth } - : legendPosition === Position.Top + : legendPositionValue === Position.Top ? { paddingTop: horizontalHeight } : { paddingTop: -horizontalHeight }; diff --git a/src/specs/settings.tsx b/src/specs/settings.tsx index 50c15424f4..69f59ac03a 100644 --- a/src/specs/settings.tsx +++ b/src/specs/settings.tsx @@ -109,7 +109,10 @@ function updateChartStore(props: SettingSpecProps) { } chartStore.setShowLegend(showLegend); - chartStore.legendPosition = legendPosition; + + if (legendPosition) { + chartStore.legendPosition.set(legendPosition); + } chartStore.showLegendDisplayValue.set(showLegendDisplayValue); chartStore.customXDomain = xDomain; diff --git a/src/specs/specs_parser.tsx b/src/specs/specs_parser.tsx index b7412cdddb..9b30d6f4eb 100644 --- a/src/specs/specs_parser.tsx +++ b/src/specs/specs_parser.tsx @@ -11,7 +11,6 @@ export class SpecsSpecRootComponent extends PureComponent { props.chartStore!.specsInitialized.set(false); return null; } - state = {}; componentDidMount() { this.props.chartStore!.specsInitialized.set(true); this.props.chartStore!.computeChart(); @@ -21,7 +20,7 @@ export class SpecsSpecRootComponent extends PureComponent { this.props.chartStore!.computeChart(); } componentWillUnmount() { - this.props.chartStore!.initialized.set(false); + this.props.chartStore!.chartInitialized.set(false); } render() { return this.props.children || null; From 750206b6bd193a01302dcffd283eb7e7f452a35a Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Mon, 19 Aug 2019 08:43:04 -0500 Subject: [PATCH 3/9] refactor: measure vert legend width with static buffer --- .playground/playgroud.tsx | 30 ++++++- package.json | 1 + src/chart_types/xy_chart/store/chart_state.ts | 8 +- src/components/legend/_legend.scss | 7 +- src/components/legend/_legend_item.scss | 67 +++++++-------- src/components/legend/_variables.scss | 1 - src/components/legend/legend.tsx | 85 +++++++++++++++---- src/components/legend/legend_item.tsx | 33 +++++-- .../react_canvas/reactive_chart.tsx | 16 +--- src/utils/themes/dark_theme.ts | 1 + src/utils/themes/light_theme.ts | 1 + src/utils/themes/theme.ts | 16 ++++ yarn.lock | 9 +- 13 files changed, 187 insertions(+), 88 deletions(-) diff --git a/.playground/playgroud.tsx b/.playground/playgroud.tsx index d093bd93ad..9616d7fdbf 100644 --- a/.playground/playgroud.tsx +++ b/.playground/playgroud.tsx @@ -1,4 +1,5 @@ import React from 'react'; +import { loremIpsum } from 'lorem-ipsum'; import { Axis, @@ -16,9 +17,33 @@ import { KIBANA_METRICS } from '../src/utils/data_samples/test_dataset_kibana'; export class Playground extends React.Component { render() { - return <>{this.renderChart(Position.Top)}; + return <>{this.renderChart(Position.Bottom)}; } renderChart(legendPosition: Position) { + const renderMore = () => { + const random = Math.floor(Math.random() * 3) + 1; + const id = loremIpsum({ count: random, units: 'words' }); + return ( + + ); + }; const theme = mergeWithDefaultTheme({ lineSeriesStyle: { line: { @@ -103,6 +128,9 @@ export class Playground extends React.Component { }} yAccessors={[1]} /> + {Array(10) + .fill(null) + .map(renderMore)}
); diff --git a/package.json b/package.json index 84e09ee254..4e1ce7829a 100644 --- a/package.json +++ b/package.json @@ -94,6 +94,7 @@ "husky": "^1.3.1", "jest": "^24.1.0", "jest-environment-jsdom-fourteen": "^0.1.0", + "lorem-ipsum": "^2.0.3", "node-sass": "^4.11.0", "postcss-cli": "^6.1.3", "prettier": "1.16.4", diff --git a/src/chart_types/xy_chart/store/chart_state.ts b/src/chart_types/xy_chart/store/chart_state.ts index 28c8a1b61e..289987e11c 100644 --- a/src/chart_types/xy_chart/store/chart_state.ts +++ b/src/chart_types/xy_chart/store/chart_state.ts @@ -849,7 +849,7 @@ export class ChartStore { }); bboxCalculator.destroy(); - // // compute chart dimensions + // compute chart dimensions this.chartDimensions = computeChartDimensions( this.parentDimensions, this.chartTheme, @@ -873,8 +873,6 @@ export class ChartStore { this.enableHistogramMode.get(), ); - // tslint:disable-next-line:no-console - // console.log({ seriesGeometries }); this.geometries = seriesGeometries.geometries; this.xScale = seriesGeometries.scales.xScale; @@ -887,7 +885,7 @@ export class ChartStore { this.geometriesIndex = seriesGeometries.geometriesIndex; this.geometriesIndexKeys = [...this.geometriesIndex.keys()].sort(compareByValueAsc); - // // compute visible ticks and their positions + // compute visible ticks and their positions const axisTicksPositions = getAxisTicksPositions( this.chartDimensions, this.chartTheme, @@ -900,8 +898,6 @@ export class ChartStore { this.enableHistogramMode.get(), barsPadding, ); - // tslint:disable-next-line:no-console - // console.log({axisTicksPositions}); this.axesPositions = axisTicksPositions.axisPositions; this.axesTicks = axisTicksPositions.axisTicks; this.axesVisibleTicks = axisTicksPositions.axisVisibleTicks; diff --git a/src/components/legend/_legend.scss b/src/components/legend/_legend.scss index a60420f2f4..5ab3297ca7 100644 --- a/src/components/legend/_legend.scss +++ b/src/components/legend/_legend.scss @@ -1,5 +1,3 @@ -// Legend - .echLegend--debug { background: red; } @@ -7,7 +5,6 @@ .echLegend--top, .echLegend--bottom { left: $euiSizeL; - max-height: $echLegendMaxHeight; .echLegendList { flex-direction: row; @@ -16,7 +13,6 @@ .echLegendItem { margin-right: $euiSizeL; - max-width: $echLegendMaxWidth; } } .echLegend--top, @@ -31,8 +27,6 @@ .echLegend--left, .echLegend--right { - max-width: $echLegendMaxWidth; - .echLegendList { flex-direction: column; } @@ -46,4 +40,5 @@ @include euiYScrollWithShadows; width: 100%; overflow-y: auto; + overflow-x: hidden; } diff --git a/src/components/legend/_legend_item.scss b/src/components/legend/_legend_item.scss index a929e1dd2b..a1253ec699 100644 --- a/src/components/legend/_legend_item.scss +++ b/src/components/legend/_legend_item.scss @@ -12,52 +12,51 @@ text-decoration: underline; } } -} -.echLegendItem__color { - margin-right: $euiSizeXS; -} + &__color { + margin-right: $euiSizeXS; + } -.echLegendItem__visibility { - margin-right: $euiSizeXS; + &__visibility { + margin-right: $euiSizeXS; - &:hover { - cursor: pointer; + &:hover { + cursor: pointer; + } } -} -.echLegendItem__title { - @include euiFontSizeXS; - @include euiTextTruncate; - flex: 1; + &__title { + @include euiFontSizeXS; + @include euiTextTruncate; + flex: 1; - &:hover { - cursor: pointer; + &:hover { + cursor: pointer; + } } -} -.echLegendItem__title--selected { - text-decoration: underline; -} + &__title--selected { + text-decoration: underline; + } -.echLegendItem__title--hasClickListener { - &:hover { - cursor: pointer; + &__title--hasClickListener { + &:hover { + cursor: pointer; + } } -} -.echLegendItem__displayValue { - @include euiFontSizeXS; - text-align: right; - margin-left: $euiSizeM; - font-feature-settings: 'tnum'; - min-width: $euiSizeL * 2; + &__displayValue { + @include euiFontSizeXS; + text-align: right; + margin-left: $euiSizeM; + font-feature-settings: 'tnum'; - &--hidden { - display: none; + &--hidden { + display: none; + } } -} -.echLegendItem-isHidden { - color: $euiColorDarkShade; + &--hidden { + color: $euiColorDarkShade; + } } diff --git a/src/components/legend/_variables.scss b/src/components/legend/_variables.scss index 9eb2df5292..858c8ba72d 100644 --- a/src/components/legend/_variables.scss +++ b/src/components/legend/_variables.scss @@ -1,3 +1,2 @@ -$echLegendMaxWidth: 200px; $echLegendMaxHeight: $euiSizeXL * 2; $echLegendItemHeight: ($echLegendMaxHeight / 2) - 6px; diff --git a/src/components/legend/legend.tsx b/src/components/legend/legend.tsx index 0b58e76259..21f3cb3e3e 100644 --- a/src/components/legend/legend.tsx +++ b/src/components/legend/legend.tsx @@ -1,6 +1,6 @@ import classNames from 'classnames'; import { inject, observer } from 'mobx-react'; -import React from 'react'; +import React, { createRef } from 'react'; import { isVertical } from '../../chart_types/xy_chart/utils/axis_utils'; import { LegendItem as SeriesLegendItem } from '../../chart_types/xy_chart/legend/legend'; import { ChartStore } from '../../chart_types/xy_chart/store/chart_state'; @@ -13,16 +13,48 @@ interface LegendProps { legendId: string; } -interface PaddingStyle { - paddingTop?: number | string; - paddingBottom?: number | string; - paddingLeft?: number | string; - paddingRight?: number | string; +interface LegendState { + width?: number; } -class LegendComponent extends React.Component { +interface HorizontalLegendStyle { + paddingLeft: number | string; + paddingRight: number | string; + height: string; +} + +interface VerticalLegendStyle { + paddingTop: number | string; + paddingBottom: number | string; + maxWidth: string; + width?: string; +} + +class LegendComponent extends React.Component { static displayName = 'Legend'; + state = { + width: undefined, + }; + + private echLegend = createRef(); + + componentDidUpdate() { + const { chartInitialized, chartTheme, legendPosition } = this.props.chartStore!; + if ( + this.echLegend.current && + isVertical(legendPosition.get()) && + this.state.width === undefined && + !chartInitialized.get() + ) { + const buffer = chartTheme.legend.legendSpacingBuffer; + + this.setState({ + width: this.echLegend.current.offsetWidth + buffer, + }); + } + } + render() { const { legendId } = this.props; const { @@ -39,14 +71,16 @@ class LegendComponent extends React.Component { return null; } - const paddingStyle = this.getPaddingStyle(legendPosition.get(), chartTheme); + const style = { + ...this.getLegendStyle(legendPosition.get(), chartTheme), + }; const legendClasses = classNames('echLegend', `echLegend--${legendPosition}`, { 'echLegend--debug': debug, invisible: !chartInitialized.get(), }); return ( -
+
{[...legendItems.values()].map(this.renderLegendElement)}
@@ -54,17 +88,36 @@ class LegendComponent extends React.Component { ); } - getPaddingStyle = (position: Position, { chartMargins }: Theme): PaddingStyle => { + getLegendStyle = ( + position: Position, + { chartMargins, legend }: Theme, + ): HorizontalLegendStyle | VerticalLegendStyle => { + const { top: paddingTop, bottom: paddingBottom, left: paddingLeft, right: paddingRight } = chartMargins; + if (isVertical(position)) { + if (this.state.width !== undefined) { + const threshold = Math.min(this.state.width!, legend.verticalWidth); + const width = `${threshold}px`; + + return { + width, + maxWidth: width, + paddingTop, + paddingBottom, + }; + } + return { - paddingTop: chartMargins.top, - paddingBottom: chartMargins.bottom, + maxWidth: `${legend.verticalWidth}px`, + paddingTop, + paddingBottom, }; } return { - paddingLeft: chartMargins.left, - paddingRight: chartMargins.right, + paddingLeft, + paddingRight, + height: `${legend.horizontalHeight}px`, }; }; @@ -78,7 +131,8 @@ class LegendComponent extends React.Component { private renderLegendElement = (item: SeriesLegendItem) => { const { key, displayValue } = item; - const tooltipValues = this.props.chartStore!.legendItemTooltipValues.get(); + const { legendPosition, legendItemTooltipValues } = this.props.chartStore!; + const tooltipValues = legendItemTooltipValues.get(); let tooltipValue; if (tooltipValues && tooltipValues.get(key)) { @@ -92,6 +146,7 @@ class LegendComponent extends React.Component { {...item} key={key} legendItemKey={key} + legendPosition={legendPosition.get()} displayValue={newDisplayValue} onMouseEnter={this.onLegendItemMouseover(key)} onMouseLeave={this.onLegendItemMouseout} diff --git a/src/components/legend/legend_item.tsx b/src/components/legend/legend_item.tsx index d23ac9df9c..09c09aaafc 100644 --- a/src/components/legend/legend_item.tsx +++ b/src/components/legend/legend_item.tsx @@ -4,10 +4,13 @@ import React from 'react'; import { Icon } from '../icons/icon'; import { ChartStore } from '../../chart_types/xy_chart/store/chart_state'; +import { Position } from '../../chart_types/xy_chart/utils/specs'; +import { isHorizontal } from '../../chart_types/xy_chart/utils/axis_utils'; interface LegendItemProps { chartStore?: ChartStore; // FIX until we find a better way on ts mobx legendItemKey: string; + legendPosition: Position; color: string | undefined; label: string | undefined; isSeriesVisible?: boolean; @@ -44,22 +47,34 @@ class LegendItemComponent extends React.Component +
{this.renderColor(this.toggleColorPicker, color, isSeriesVisible)} {this.renderTitle(label, onTitleClick, hasTitleClickListener, isSelected, hasDisplayValue)} {this.renderDisplayValue(displayValue, showLegendDisplayValue, isSeriesVisible)} diff --git a/src/components/react_canvas/reactive_chart.tsx b/src/components/react_canvas/reactive_chart.tsx index 9753e04868..f3a3fc211a 100644 --- a/src/components/react_canvas/reactive_chart.tsx +++ b/src/components/react_canvas/reactive_chart.tsx @@ -354,26 +354,12 @@ class Chart extends React.Component { debug, setCursorPosition, isChartEmpty, - legendPosition, - chartTheme, } = this.props.chartStore!; if (isChartEmpty) { - const { verticalWidth, horizontalHeight } = chartTheme.legend; - const legendPositionValue = legendPosition.get(); - - const paddingStyle = - legendPositionValue && isVertical(legendPositionValue) - ? legendPositionValue === Position.Right - ? { paddingLeft: -verticalWidth } - : { paddingLeft: verticalWidth } - : legendPositionValue === Position.Top - ? { paddingTop: horizontalHeight } - : { paddingTop: -horizontalHeight }; - return (
-

No data to display

+

No data to display

); } diff --git a/src/utils/themes/dark_theme.ts b/src/utils/themes/dark_theme.ts index a88486918e..e206f97645 100644 --- a/src/utils/themes/dark_theme.ts +++ b/src/utils/themes/dark_theme.ts @@ -97,6 +97,7 @@ export const DARK_THEME: Theme = { legend: { verticalWidth: 200, horizontalHeight: 64, + legendSpacingBuffer: 40, }, crosshair: { band: { diff --git a/src/utils/themes/light_theme.ts b/src/utils/themes/light_theme.ts index 2f07bf1526..e8ef35a844 100644 --- a/src/utils/themes/light_theme.ts +++ b/src/utils/themes/light_theme.ts @@ -97,6 +97,7 @@ export const LIGHT_THEME: Theme = { legend: { verticalWidth: 200, horizontalHeight: 64, + legendSpacingBuffer: 40, }, crosshair: { band: { diff --git a/src/utils/themes/theme.ts b/src/utils/themes/theme.ts index dce8319318..8b588d8465 100644 --- a/src/utils/themes/theme.ts +++ b/src/utils/themes/theme.ts @@ -75,8 +75,24 @@ export interface ColorConfig { defaultVizColor: string; } export interface LegendStyle { + /** + * Max width used for left/right legend + * + * or + * + * Width of `LegendItem` for top/bottom legend + */ verticalWidth: number; + /** + * Max height used for top/bottom legend + */ horizontalHeight: number; + /** + * Added buffer between label and value. + * + * Smaller values render a more compact legend + */ + legendSpacingBuffer: number; } export interface Theme { /** diff --git a/yarn.lock b/yarn.lock index 0fd779fe4a..a91051f487 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4765,7 +4765,7 @@ commander@2.17.x: resolved "https://registry.yarnpkg.com/commander/-/commander-2.17.1.tgz#bd77ab7de6de94205ceacc72f1716d29f20a77bf" integrity sha512-wPMUt6FnH2yzG95SA6mzjQOEKUU3aLaDEmzs1ti+1E9h+CsrZghRlqEM/EJ4KscsQVG8uNN4uVreUeT8+drlgg== -commander@^2.19.0, commander@^2.20.0, commander@~2.20.0: +commander@^2.17.1, commander@^2.19.0, commander@^2.20.0, commander@~2.20.0: version "2.20.0" resolved "https://registry.yarnpkg.com/commander/-/commander-2.20.0.tgz#d58bb2b5c1ee8f87b0d340027e9e94e222c5a422" integrity sha512-7j2y+40w61zy6YC2iRNpUe/NwhNyoXrYpHMrSunaMG64nRnaf96zO/KMQR4OyN/UnE5KLyEBnKHd4aG3rskjpQ== @@ -9686,6 +9686,13 @@ loose-envify@^1.0.0, loose-envify@^1.1.0, loose-envify@^1.3.0, loose-envify@^1.3 dependencies: js-tokens "^3.0.0 || ^4.0.0" +lorem-ipsum@^2.0.3: + version "2.0.3" + resolved "https://registry.yarnpkg.com/lorem-ipsum/-/lorem-ipsum-2.0.3.tgz#9f1fa634780c9f58a349d4e091c3ba74f733164e" + integrity sha512-CX2r84DMWjW/DWiuzicTI9aRaJPAw2cvAGMJYZh/nx12OkTGqloj8y8FU0S8ZkKwOdqhfxEA6Ly8CW2P6Yxjwg== + dependencies: + commander "^2.17.1" + loud-rejection@^1.0.0: version "1.6.0" resolved "https://registry.yarnpkg.com/loud-rejection/-/loud-rejection-1.6.0.tgz#5b46f80147edee578870f086d04821cf998e551f" From ad6232c5ef283bff49b33d617d17d081e7a8a76a Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Mon, 19 Aug 2019 08:44:21 -0500 Subject: [PATCH 4/9] fix: legend toggle issue for top/bottom legends --- src/specs/specs_parser.tsx | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/specs/specs_parser.tsx b/src/specs/specs_parser.tsx index 9b30d6f4eb..aaa827e891 100644 --- a/src/specs/specs_parser.tsx +++ b/src/specs/specs_parser.tsx @@ -7,10 +7,6 @@ export interface SpecProps { } export class SpecsSpecRootComponent extends PureComponent { - static getDerivedStateFromProps(props: SpecProps) { - props.chartStore!.specsInitialized.set(false); - return null; - } componentDidMount() { this.props.chartStore!.specsInitialized.set(true); this.props.chartStore!.computeChart(); From 51041af268dd407c651705fb0e9ea499f336924b Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Mon, 19 Aug 2019 13:51:18 -0500 Subject: [PATCH 5/9] fix: merge errors --- src/chart_types/xy_chart/store/chart_state.ts | 2 - src/chart_types/xy_chart/utils/dimensions.ts | 37 +++---------------- 2 files changed, 5 insertions(+), 34 deletions(-) diff --git a/src/chart_types/xy_chart/store/chart_state.ts b/src/chart_types/xy_chart/store/chart_state.ts index 13310037f3..268378da3c 100644 --- a/src/chart_types/xy_chart/store/chart_state.ts +++ b/src/chart_types/xy_chart/store/chart_state.ts @@ -922,8 +922,6 @@ export class ChartStore { this.chartTheme, this.axesTicksDimensions, this.axesSpecs, - this.showLegend.get(), - this.legendPosition.get(), ); this.chartDimensions = computedChartDims.chartDimensions; diff --git a/src/chart_types/xy_chart/utils/dimensions.ts b/src/chart_types/xy_chart/utils/dimensions.ts index 7bab78a5f9..0d8d10d613 100644 --- a/src/chart_types/xy_chart/utils/dimensions.ts +++ b/src/chart_types/xy_chart/utils/dimensions.ts @@ -19,13 +19,11 @@ export function computeChartDimensions( chartTheme: Theme, axisDimensions: Map, axisSpecs: Map, - showLegend: boolean, - legendPosition?: Position, ): { chartDimensions: Dimensions; leftMargin: number; } { - const { chartMargins, chartPaddings, legend } = chartTheme; + const { chartMargins, chartPaddings } = chartTheme; const { axisTitleStyle } = chartTheme.axes; const axisTitleHeight = axisTitleStyle.fontSize + axisTitleStyle.padding; @@ -74,41 +72,16 @@ export function computeChartDimensions( const chartWidth = parentDimensions.width - chartLeftAxisMaxWidth - chartRightAxisMaxWidth; const chartHeight = parentDimensions.height - chartTopAxisMaxHeight - chartBottomAxisMaxHeight; - let vMargin = 0; - let hMargin = 0; - - // add space for legend - let legendTopMargin = 0; - let legendLeftMargin = 0; - if (showLegend && legendPosition) { - switch (legendPosition) { - case Position.Right: - hMargin += legend.verticalWidth; - break; - case Position.Left: - hMargin += legend.verticalWidth; - legendLeftMargin = legend.verticalWidth; - break; - case Position.Top: - vMargin += legend.horizontalHeight; - legendTopMargin = legend.horizontalHeight; - break; - case Position.Bottom: - vMargin += legend.horizontalHeight; - break; - } - } - - let top = chartTopAxisMaxHeight + chartPaddings.top + legendTopMargin; - let left = chartLeftAxisMaxWidth + chartPaddings.left + legendLeftMargin; + let top = chartTopAxisMaxHeight + chartPaddings.top; + let left = chartLeftAxisMaxWidth + chartPaddings.left; return { leftMargin: chartLeftAxisMaxWidth - vLeftAxisSpecWidth, chartDimensions: { top, left, - width: chartWidth - hMargin - chartPaddings.left - chartPaddings.right, - height: chartHeight - vMargin - chartPaddings.top - chartPaddings.bottom, + width: chartWidth - chartPaddings.left - chartPaddings.right, + height: chartHeight - chartPaddings.top - chartPaddings.bottom, }, }; } From 981cf6b3b16fb1176eb270997643ba31ab675aba Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Tue, 20 Aug 2019 09:27:10 -0500 Subject: [PATCH 6/9] test: fix broken tests --- .../xy_chart/store/chart_state.test.ts | 4 ++-- .../xy_chart/utils/axis_utils.test.ts | 23 ------------------- .../xy_chart/utils/dimensions.test.ts | 16 ++++++------- .../react_canvas/reactive_chart.tsx | 4 ++-- src/specs/settings.test.tsx | 6 ++--- src/specs/specs_parser.test.tsx | 4 ++-- 6 files changed, 17 insertions(+), 40 deletions(-) diff --git a/src/chart_types/xy_chart/store/chart_state.test.ts b/src/chart_types/xy_chart/store/chart_state.test.ts index 4b78381eee..3ecd8bdc0c 100644 --- a/src/chart_types/xy_chart/store/chart_state.test.ts +++ b/src/chart_types/xy_chart/store/chart_state.test.ts @@ -501,7 +501,7 @@ describe('Chart Store', () => { }; localStore.computeChart(); - expect(localStore.initialized.get()).toBe(false); + expect(localStore.chartInitialized.get()).toBe(false); }); test('only computes chart if series specs exist', () => { @@ -516,7 +516,7 @@ describe('Chart Store', () => { localStore.seriesSpecs = new Map(); localStore.computeChart(); - expect(localStore.initialized.get()).toBe(false); + expect(localStore.chartInitialized.get()).toBe(false); }); test('can set the color for a series', () => { diff --git a/src/chart_types/xy_chart/utils/axis_utils.test.ts b/src/chart_types/xy_chart/utils/axis_utils.test.ts index 913f7102a5..55dfd19334 100644 --- a/src/chart_types/xy_chart/utils/axis_utils.test.ts +++ b/src/chart_types/xy_chart/utils/axis_utils.test.ts @@ -33,11 +33,6 @@ import { import { CanvasTextBBoxCalculator } from '../../../utils/bbox/canvas_text_bbox_calculator'; import { SvgTextBBoxCalculator } from '../../../utils/bbox/svg_text_bbox_calculator'; -// const chartScalesConfig: ScalesConfig = { -// ordinal: { -// padding: 0, -// }, -// }; describe('Axis computational utils', () => { const mockedRect = { x: 0, @@ -774,7 +769,6 @@ describe('Axis computational utils', () => { test('should compute axis ticks positions with title', () => { const chartRotation = 0; - const showLegend = false; // validate assumptions for test expect(verticalAxisSpec.id).toEqual(verticalAxisSpecWTitle.id); @@ -792,7 +786,6 @@ describe('Axis computational utils', () => { }, LIGHT_THEME, chartRotation, - showLegend, axisSpecs, axisDims, xDomain, @@ -819,7 +812,6 @@ describe('Axis computational utils', () => { }, LIGHT_THEME, chartRotation, - showLegend, axisSpecs, axisDims, xDomain, @@ -982,9 +974,6 @@ describe('Axis computational utils', () => { test('should not compute axis ticks positions if missaligned specs', () => { const chartRotation = 0; - const showLegend = true; - const leftLegendPosition = Position.Left; - const axisSpecs = new Map(); axisSpecs.set(verticalAxisSpec.id, verticalAxisSpec); @@ -998,14 +987,12 @@ describe('Axis computational utils', () => { }, LIGHT_THEME, chartRotation, - showLegend, axisSpecs, axisDims, xDomain, [yDomain], 1, false, - leftLegendPosition, ); expect(axisTicksPosition.axisPositions.size).toBe(0); expect(axisTicksPosition.axisTicks.size).toBe(0); @@ -1015,10 +1002,6 @@ describe('Axis computational utils', () => { test('should compute axis ticks positions', () => { const chartRotation = 0; - const showLegend = true; - const leftLegendPosition = Position.Left; - const topLegendPosition = Position.Top; - const axisSpecs = new Map(); axisSpecs.set(verticalAxisSpec.id, verticalAxisSpec); @@ -1032,14 +1015,12 @@ describe('Axis computational utils', () => { }, LIGHT_THEME, chartRotation, - showLegend, axisSpecs, axisDims, xDomain, [yDomain], 1, false, - leftLegendPosition, ); const expectedVerticalAxisGridLines = [ @@ -1065,14 +1046,12 @@ describe('Axis computational utils', () => { }, LIGHT_THEME, chartRotation, - showLegend, axisSpecs, axisDims, xDomain, [yDomain], 1, false, - topLegendPosition, ); const expectedPositionWithTopLegend = { @@ -1095,14 +1074,12 @@ describe('Axis computational utils', () => { }, LIGHT_THEME, chartRotation, - showLegend, invalidSpecs, axisDims, xDomain, [yDomain], 1, false, - leftLegendPosition, ); }; diff --git a/src/chart_types/xy_chart/utils/dimensions.test.ts b/src/chart_types/xy_chart/utils/dimensions.test.ts index 785c2c5f8c..f3618c06e6 100644 --- a/src/chart_types/xy_chart/utils/dimensions.test.ts +++ b/src/chart_types/xy_chart/utils/dimensions.test.ts @@ -50,8 +50,8 @@ describe('Computed chart dimensions', () => { const legend: LegendStyle = { verticalWidth: 10, horizontalHeight: 10, + legendSpacingBuffer: 10, }; - const showLegend = false; const defaultTheme = LIGHT_THEME; const chartTheme = { ...defaultTheme, @@ -67,7 +67,7 @@ describe('Computed chart dimensions', () => { test('should be equal to parent dimension with no axis minus margins', () => { const axisDims = new Map(); const axisSpecs = new Map(); - const { chartDimensions } = computeChartDimensions(parentDim, chartTheme, axisDims, axisSpecs, showLegend); + const { chartDimensions } = computeChartDimensions(parentDim, chartTheme, axisDims, axisSpecs); expect(chartDimensions.left + chartDimensions.width).toBeLessThanOrEqual(parentDim.width); expect(chartDimensions.top + chartDimensions.height).toBeLessThanOrEqual(parentDim.height); expect(chartDimensions).toMatchSnapshot(); @@ -79,7 +79,7 @@ describe('Computed chart dimensions', () => { const axisSpecs = new Map(); axisDims.set(getAxisId('axis_1'), axis1Dims); axisSpecs.set(getAxisId('axis_1'), axisLeftSpec); - const { chartDimensions } = computeChartDimensions(parentDim, chartTheme, axisDims, axisSpecs, showLegend); + const { chartDimensions } = computeChartDimensions(parentDim, chartTheme, axisDims, axisSpecs); expect(chartDimensions.left + chartDimensions.width).toBeLessThanOrEqual(parentDim.width); expect(chartDimensions.top + chartDimensions.height).toBeLessThanOrEqual(parentDim.height); expect(chartDimensions).toMatchSnapshot(); @@ -91,7 +91,7 @@ describe('Computed chart dimensions', () => { const axisSpecs = new Map(); axisDims.set(getAxisId('axis_1'), axis1Dims); axisSpecs.set(getAxisId('axis_1'), { ...axisLeftSpec, position: Position.Right }); - const { chartDimensions } = computeChartDimensions(parentDim, chartTheme, axisDims, axisSpecs, showLegend); + const { chartDimensions } = computeChartDimensions(parentDim, chartTheme, axisDims, axisSpecs); expect(chartDimensions.left + chartDimensions.width).toBeLessThanOrEqual(parentDim.width); expect(chartDimensions.top + chartDimensions.height).toBeLessThanOrEqual(parentDim.height); expect(chartDimensions).toMatchSnapshot(); @@ -106,7 +106,7 @@ describe('Computed chart dimensions', () => { ...axisLeftSpec, position: Position.Top, }); - const { chartDimensions } = computeChartDimensions(parentDim, chartTheme, axisDims, axisSpecs, showLegend); + const { chartDimensions } = computeChartDimensions(parentDim, chartTheme, axisDims, axisSpecs); expect(chartDimensions.left + chartDimensions.width).toBeLessThanOrEqual(parentDim.width); expect(chartDimensions.top + chartDimensions.height).toBeLessThanOrEqual(parentDim.height); expect(chartDimensions).toMatchSnapshot(); @@ -121,7 +121,7 @@ describe('Computed chart dimensions', () => { ...axisLeftSpec, position: Position.Bottom, }); - const { chartDimensions } = computeChartDimensions(parentDim, chartTheme, axisDims, axisSpecs, showLegend); + const { chartDimensions } = computeChartDimensions(parentDim, chartTheme, axisDims, axisSpecs); expect(chartDimensions.left + chartDimensions.width).toBeLessThanOrEqual(parentDim.width); expect(chartDimensions.top + chartDimensions.height).toBeLessThanOrEqual(parentDim.height); expect(chartDimensions).toMatchSnapshot(); @@ -134,7 +134,7 @@ describe('Computed chart dimensions', () => { ...axisLeftSpec, position: Position.Bottom, }); - const chartDimensions = computeChartDimensions(parentDim, chartTheme, axisDims, axisSpecs, showLegend); + const chartDimensions = computeChartDimensions(parentDim, chartTheme, axisDims, axisSpecs); const expectedDims = { chartDimensions: { @@ -156,7 +156,7 @@ describe('Computed chart dimensions', () => { hide: true, position: Position.Bottom, }); - const hiddenAxisChartDimensions = computeChartDimensions(parentDim, chartTheme, axisDims, axisSpecs, showLegend); + const hiddenAxisChartDimensions = computeChartDimensions(parentDim, chartTheme, axisDims, axisSpecs); expect(hiddenAxisChartDimensions).toEqual(expectedDims); }); diff --git a/src/components/react_canvas/reactive_chart.tsx b/src/components/react_canvas/reactive_chart.tsx index f3a3fc211a..59329e5676 100644 --- a/src/components/react_canvas/reactive_chart.tsx +++ b/src/components/react_canvas/reactive_chart.tsx @@ -2,7 +2,7 @@ import classNames from 'classnames'; import { inject, observer } from 'mobx-react'; import React from 'react'; import { Layer, Rect, Stage } from 'react-konva'; -import { isLineAnnotation, isRectAnnotation, Position } from '../../chart_types/xy_chart/utils/specs'; +import { isLineAnnotation, isRectAnnotation } from '../../chart_types/xy_chart/utils/specs'; import { LineAnnotationStyle, RectAnnotationStyle } from '../../utils/themes/theme'; import { AnnotationId } from '../../utils/ids'; import { @@ -20,7 +20,7 @@ import { Grid } from './grid'; import { LineAnnotation } from './line_annotation'; import { LineGeometries } from './line_geometries'; import { RectAnnotation } from './rect_annotation'; -import { isVertical } from '../../chart_types/xy_chart/utils/axis_utils'; + interface ReactiveChartProps { chartStore?: ChartStore; // FIX until we find a better way on ts mobx } diff --git a/src/specs/settings.test.tsx b/src/specs/settings.test.tsx index f5d1b0525f..6844b86f55 100644 --- a/src/specs/settings.test.tsx +++ b/src/specs/settings.test.tsx @@ -47,7 +47,7 @@ describe('Settings spec component', () => { expect(chartStore.showLegend.get()).toEqual(true); expect(chartStore.tooltipType.get()).toEqual(TooltipType.None); expect(chartStore.tooltipSnap.get()).toEqual(false); - expect(chartStore.legendPosition).toBe(Position.Bottom); + expect(chartStore.legendPosition.get()).toBe(Position.Bottom); expect(chartStore.showLegendDisplayValue.get()).toEqual(false); expect(chartStore.debug).toBe(true); expect(chartStore.customXDomain).toEqual({ min: 0, max: 10 }); @@ -64,7 +64,7 @@ describe('Settings spec component', () => { expect(chartStore.tooltipType.get()).toEqual(DEFAULT_TOOLTIP_TYPE); expect(chartStore.tooltipSnap.get()).toEqual(DEFAULT_TOOLTIP_SNAP); expect(chartStore.showLegendDisplayValue.get()).toEqual(true); - expect(chartStore.legendPosition).toBeUndefined(); + expect(chartStore.legendPosition.get()).toBe(Position.Right); expect(chartStore.debug).toBe(false); expect(chartStore.customXDomain).toBeUndefined(); @@ -93,7 +93,7 @@ describe('Settings spec component', () => { expect(chartStore.showLegend.get()).toEqual(true); expect(chartStore.tooltipType.get()).toEqual(TooltipType.None); expect(chartStore.tooltipSnap.get()).toEqual(false); - expect(chartStore.legendPosition).toBe(Position.Bottom); + expect(chartStore.legendPosition.get()).toBe(Position.Bottom); expect(chartStore.showLegendDisplayValue.get()).toEqual(false); expect(chartStore.debug).toBe(true); expect(chartStore.customXDomain).toEqual({ min: 0, max: 10 }); diff --git a/src/specs/specs_parser.test.tsx b/src/specs/specs_parser.test.tsx index 1423438bbe..c68e8bdd1d 100644 --- a/src/specs/specs_parser.test.tsx +++ b/src/specs/specs_parser.test.tsx @@ -28,9 +28,9 @@ describe('Specs parser', () => { }); test('updates initialization state on unmount', () => { const chartStore = new ChartStore(); - chartStore.initialized.set(true); + chartStore.chartInitialized.set(true); const component = mount(); component.unmount(); - expect(chartStore.initialized.get()).toBe(false); + expect(chartStore.chartInitialized.get()).toBe(false); }); }); From a335522872bd0cc53a53875c8be34400f15cbc37 Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Tue, 20 Aug 2019 18:46:27 -0500 Subject: [PATCH 7/9] feat(legend): flex styles to grid Refactor css flex styles to use grid for more shinny UI! --- .playground/index.html | 3 +- .../xy_chart/utils/dimensions.test.ts | 2 +- src/components/_container.scss | 2 +- src/components/_tooltip.scss | 2 +- src/components/legend/_legend.scss | 66 ++++++++++--------- src/components/legend/_legend_item.scss | 25 ++++++- src/components/legend/_variables.scss | 5 +- src/components/legend/legend.tsx | 65 ++++++++++-------- src/components/legend/legend_item.tsx | 8 +-- src/utils/themes/dark_theme.ts | 2 +- src/utils/themes/light_theme.ts | 2 +- src/utils/themes/theme.ts | 2 +- stories/legend.tsx | 49 +++++++++++++- 13 files changed, 153 insertions(+), 80 deletions(-) diff --git a/.playground/index.html b/.playground/index.html index 653990094a..a8f3a27002 100644 --- a/.playground/index.html +++ b/.playground/index.html @@ -2,6 +2,7 @@ + Charts Playground Document @@ -23,7 +24,7 @@ .chart { position: relative; width: 100%; - height: 50%; + height: 100%; } diff --git a/src/chart_types/xy_chart/utils/dimensions.test.ts b/src/chart_types/xy_chart/utils/dimensions.test.ts index f3618c06e6..dbf7e330bb 100644 --- a/src/chart_types/xy_chart/utils/dimensions.test.ts +++ b/src/chart_types/xy_chart/utils/dimensions.test.ts @@ -50,7 +50,7 @@ describe('Computed chart dimensions', () => { const legend: LegendStyle = { verticalWidth: 10, horizontalHeight: 10, - legendSpacingBuffer: 10, + spacingBuffer: 10, }; const defaultTheme = LIGHT_THEME; const chartTheme = { diff --git a/src/components/_container.scss b/src/components/_container.scss index eb1576b225..bdcb537754 100644 --- a/src/components/_container.scss +++ b/src/components/_container.scss @@ -3,7 +3,7 @@ */ .echContainer { - flex: 1; + flex: 1 1 auto; position: relative; &:hover { diff --git a/src/components/_tooltip.scss b/src/components/_tooltip.scss index 02338a29cd..9200337d28 100644 --- a/src/components/_tooltip.scss +++ b/src/components/_tooltip.scss @@ -26,7 +26,7 @@ &__label { @include euiTextOverflowWrap; min-width: 1px; - flex: 1; + flex: 1 1 auto; } &__value { diff --git a/src/components/legend/_legend.scss b/src/components/legend/_legend.scss index 5ab3297ca7..fcb5d4671c 100644 --- a/src/components/legend/_legend.scss +++ b/src/components/legend/_legend.scss @@ -1,44 +1,46 @@ -.echLegend--debug { - background: red; -} +.echLegend { + &--top, + &--bottom { + .echLegendList { + display: grid; + grid-column-gap: $echLegendColumnGap; + grid-row-gap: $echLegendRowGap; + margin-top: $echLegendRowGap; + margin-bottom: $echLegendRowGap; -.echLegend--top, -.echLegend--bottom { - left: $euiSizeL; + @include internetExplorerOnly { + display: flex; + flex-direction: row; + flex-wrap: wrap; + } + } + } - .echLegendList { - flex-direction: row; - flex-wrap: wrap; + &--left, + &--right { + .echLegendList { + flex-direction: column; + } } - .echLegendItem { - margin-right: $euiSizeL; + &--top, + &--left { + order: 0; } -} -.echLegend--top, -.echLegend--left { - order: 0; -} -.echLegend--bottom, -.echLegend--right { - order: 1; -} + &--bottom, + &--right { + order: 1; + } -.echLegend--left, -.echLegend--right { - .echLegendList { - flex-direction: column; + &--debug { + background: red; } - .echLegendItem { + .echLegendListContainer { + @include euiYScrollWithShadows; width: 100%; + overflow-y: auto; + overflow-x: hidden; } } - -.echLegendListContainer { - @include euiYScrollWithShadows; - width: 100%; - overflow-y: auto; - overflow-x: hidden; -} diff --git a/src/components/legend/_legend_item.scss b/src/components/legend/_legend_item.scss index a1253ec699..e312e19eb3 100644 --- a/src/components/legend/_legend_item.scss +++ b/src/components/legend/_legend_item.scss @@ -1,11 +1,13 @@ +$legendItemVerticalPadding: $echLegendRowGap / 2; + .echLegendItem { color: $euiTextColor; - height: $echLegendItemHeight; display: flex; flex-wrap: nowrap; justify-content: space-between; user-select: none; align-items: center; + width: 100%; &:hover { .echLegendItem__title { @@ -28,7 +30,7 @@ &__title { @include euiFontSizeXS; @include euiTextTruncate; - flex: 1; + flex: 1 1 auto; &:hover { cursor: pointer; @@ -48,7 +50,7 @@ &__displayValue { @include euiFontSizeXS; text-align: right; - margin-left: $euiSizeM; + margin-left: $euiSizeXS; font-feature-settings: 'tnum'; &--hidden { @@ -56,6 +58,23 @@ } } + &--right, + &--left { + padding-top: $legendItemVerticalPadding; + padding-bottom: $legendItemVerticalPadding; + } + + @include internetExplorerOnly { + &--bottom, + &--top { + width: $echLegendMaxWidth; + margin-right: $euiSizeL; + } + + padding-top: $legendItemVerticalPadding; + padding-bottom: $legendItemVerticalPadding; + } + &--hidden { color: $euiColorDarkShade; } diff --git a/src/components/legend/_variables.scss b/src/components/legend/_variables.scss index 858c8ba72d..c8543808be 100644 --- a/src/components/legend/_variables.scss +++ b/src/components/legend/_variables.scss @@ -1,2 +1,3 @@ -$echLegendMaxHeight: $euiSizeXL * 2; -$echLegendItemHeight: ($echLegendMaxHeight / 2) - 6px; +$echLegendMaxWidth: 200px; +$echLegendRowGap: 8px; +$echLegendColumnGap: 24px; diff --git a/src/components/legend/legend.tsx b/src/components/legend/legend.tsx index 21f3cb3e3e..bfdb6e9c8b 100644 --- a/src/components/legend/legend.tsx +++ b/src/components/legend/legend.tsx @@ -1,7 +1,7 @@ import classNames from 'classnames'; import { inject, observer } from 'mobx-react'; import React, { createRef } from 'react'; -import { isVertical } from '../../chart_types/xy_chart/utils/axis_utils'; +import { isVertical, isHorizontal } from '../../chart_types/xy_chart/utils/axis_utils'; import { LegendItem as SeriesLegendItem } from '../../chart_types/xy_chart/legend/legend'; import { ChartStore } from '../../chart_types/xy_chart/store/chart_state'; import { Position } from '../../chart_types/xy_chart/utils/specs'; @@ -17,17 +17,18 @@ interface LegendState { width?: number; } -interface HorizontalLegendStyle { - paddingLeft: number | string; - paddingRight: number | string; - height: string; +interface LegendStyle { + maxHeight?: string; + maxWidth?: string; + width?: string; } -interface VerticalLegendStyle { - paddingTop: number | string; - paddingBottom: number | string; - maxWidth: string; - width?: string; +interface LegendListStyle { + paddingTop?: number | string; + paddingBottom?: number | string; + paddingLeft?: number | string; + paddingRight?: number | string; + gridTemplateColumns?: string; } class LegendComponent extends React.Component { @@ -47,7 +48,7 @@ class LegendComponent extends React.Component { this.state.width === undefined && !chartInitialized.get() ) { - const buffer = chartTheme.legend.legendSpacingBuffer; + const buffer = chartTheme.legend.spacingBuffer; this.setState({ width: this.echLegend.current.offsetWidth + buffer, @@ -66,34 +67,48 @@ class LegendComponent extends React.Component { debug, chartTheme, } = this.props.chartStore!; + const postion = legendPosition.get(); if (!showLegend.get() || !legendInitialized.get() || legendItems.size === 0) { return null; } - const style = { - ...this.getLegendStyle(legendPosition.get(), chartTheme), - }; - const legendClasses = classNames('echLegend', `echLegend--${legendPosition}`, { + const legendStyle = this.getLegendStyle(postion, chartTheme); + const legendListStyle = this.getLegendListStyle(postion, chartTheme); + const legendClasses = classNames('echLegend', `echLegend--${postion}`, { 'echLegend--debug': debug, invisible: !chartInitialized.get(), }); return ( -
+
-
{[...legendItems.values()].map(this.renderLegendElement)}
+
+ {[...legendItems.values()].map(this.renderLegendElement)} +
); } - getLegendStyle = ( - position: Position, - { chartMargins, legend }: Theme, - ): HorizontalLegendStyle | VerticalLegendStyle => { + getLegendListStyle = (position: Position, { chartMargins, legend }: Theme): LegendListStyle => { const { top: paddingTop, bottom: paddingBottom, left: paddingLeft, right: paddingRight } = chartMargins; + if (isHorizontal(position)) { + return { + paddingLeft, + paddingRight, + gridTemplateColumns: `repeat(auto-fill, minmax(${legend.verticalWidth}px, 1fr))`, + }; + } + + return { + paddingTop, + paddingBottom, + }; + }; + + getLegendStyle = (position: Position, { legend }: Theme): LegendStyle => { if (isVertical(position)) { if (this.state.width !== undefined) { const threshold = Math.min(this.state.width!, legend.verticalWidth); @@ -102,22 +117,16 @@ class LegendComponent extends React.Component { return { width, maxWidth: width, - paddingTop, - paddingBottom, }; } return { maxWidth: `${legend.verticalWidth}px`, - paddingTop, - paddingBottom, }; } return { - paddingLeft, - paddingRight, - height: `${legend.horizontalHeight}px`, + maxHeight: `${legend.horizontalHeight}px`, }; }; diff --git a/src/components/legend/legend_item.tsx b/src/components/legend/legend_item.tsx index 09c09aaafc..6dc2cb6c3b 100644 --- a/src/components/legend/legend_item.tsx +++ b/src/components/legend/legend_item.tsx @@ -5,7 +5,6 @@ import { Icon } from '../icons/icon'; import { ChartStore } from '../../chart_types/xy_chart/store/chart_state'; import { Position } from '../../chart_types/xy_chart/utils/specs'; -import { isHorizontal } from '../../chart_types/xy_chart/utils/axis_utils'; interface LegendItemProps { chartStore?: ChartStore; // FIX until we find a better way on ts mobx @@ -65,16 +64,13 @@ class LegendItemComponent extends React.Component +
{this.renderColor(this.toggleColorPicker, color, isSeriesVisible)} {this.renderTitle(label, onTitleClick, hasTitleClickListener, isSelected, hasDisplayValue)} {this.renderDisplayValue(displayValue, showLegendDisplayValue, isSeriesVisible)} diff --git a/src/utils/themes/dark_theme.ts b/src/utils/themes/dark_theme.ts index e206f97645..ea9a64f829 100644 --- a/src/utils/themes/dark_theme.ts +++ b/src/utils/themes/dark_theme.ts @@ -97,7 +97,7 @@ export const DARK_THEME: Theme = { legend: { verticalWidth: 200, horizontalHeight: 64, - legendSpacingBuffer: 40, + spacingBuffer: 40, }, crosshair: { band: { diff --git a/src/utils/themes/light_theme.ts b/src/utils/themes/light_theme.ts index e8ef35a844..01ef26cd8d 100644 --- a/src/utils/themes/light_theme.ts +++ b/src/utils/themes/light_theme.ts @@ -97,7 +97,7 @@ export const LIGHT_THEME: Theme = { legend: { verticalWidth: 200, horizontalHeight: 64, - legendSpacingBuffer: 40, + spacingBuffer: 40, }, crosshair: { band: { diff --git a/src/utils/themes/theme.ts b/src/utils/themes/theme.ts index 8b588d8465..cddfb4b416 100644 --- a/src/utils/themes/theme.ts +++ b/src/utils/themes/theme.ts @@ -92,7 +92,7 @@ export interface LegendStyle { * * Smaller values render a more compact legend */ - legendSpacingBuffer: number; + spacingBuffer: number; } export interface Theme { /** diff --git a/stories/legend.tsx b/stories/legend.tsx index 1886b671a6..7eea1fa8d9 100644 --- a/stories/legend.tsx +++ b/stories/legend.tsx @@ -1,4 +1,4 @@ -import { array, boolean, select } from '@storybook/addon-knobs'; +import { array, boolean, select, number } from '@storybook/addon-knobs'; import { storiesOf } from '@storybook/react'; import React from 'react'; import { @@ -13,6 +13,7 @@ import { Position, ScaleType, Settings, + PartialTheme, } from '../src/'; import * as TestDatasets from '../src/utils/data_samples/test_dataset'; import { TSVB_DATASET } from '../src/utils/data_samples/test_dataset_tsvb'; @@ -227,4 +228,48 @@ storiesOf('Legend', module) {seriesComponents} ); - }); + }) + .add( + 'legend spacingBuffer', + () => { + const theme: PartialTheme = { + legend: { + spacingBuffer: number('legend buffer value', 80), + }, + }; + + return ( + + + + Number(d).toFixed(2)} + /> + + + + + ); + }, + { + info: + 'For high variability in values it may be necessary to increase the `spacingBuffer` to account for larger numbers.', + }, + ); From ea01ba35e90b1807169847b99a52def918e062c5 Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Wed, 21 Aug 2019 09:49:42 -0500 Subject: [PATCH 8/9] fix: pr comments Remove legendId, fix safari max-height inheritance issue, remove uneeded legendList styles, revert empty state flex value. --- src/components/_container.scss | 2 +- src/components/chart.tsx | 5 +---- src/components/legend/_index.scss | 1 - src/components/legend/_legend_list.scss | 3 --- src/components/legend/legend.tsx | 8 +++----- 5 files changed, 5 insertions(+), 14 deletions(-) delete mode 100644 src/components/legend/_legend_list.scss diff --git a/src/components/_container.scss b/src/components/_container.scss index bdcb537754..eb1576b225 100644 --- a/src/components/_container.scss +++ b/src/components/_container.scss @@ -3,7 +3,7 @@ */ .echContainer { - flex: 1 1 auto; + flex: 1; position: relative; &:hover { diff --git a/src/components/chart.tsx b/src/components/chart.tsx index ddbc0a1da2..8d58628155 100644 --- a/src/components/chart.tsx +++ b/src/components/chart.tsx @@ -4,7 +4,6 @@ import { Provider } from 'mobx-react'; import { SpecsParser } from '../specs/specs_parser'; import { ChartStore } from '../chart_types/xy_chart/store/chart_state'; -import { htmlIdGenerator } from '../utils/commons'; import { AnnotationTooltip } from './annotation_tooltips'; import { ChartResizer } from './chart_resizer'; import { Crosshair } from './crosshair'; @@ -35,7 +34,6 @@ export class Chart extends React.Component { renderer: 'canvas', }; private chartSpecStore: ChartStore; - private legendId: string; constructor(props: any) { super(props); this.chartSpecStore = new ChartStore(); @@ -48,7 +46,6 @@ export class Chart extends React.Component { legendPosition, }); }); - this.legendId = htmlIdGenerator()('legend'); } static getContainerStyle = (size: any): CSSProperties => { @@ -91,7 +88,7 @@ export class Chart extends React.Component { return (
- + {this.props.children}
diff --git a/src/components/legend/_index.scss b/src/components/legend/_index.scss index 48c6d3a719..6b6f3997ff 100644 --- a/src/components/legend/_index.scss +++ b/src/components/legend/_index.scss @@ -1,4 +1,3 @@ @import 'variables'; @import 'legend'; -@import 'legend_list'; @import 'legend_item'; diff --git a/src/components/legend/_legend_list.scss b/src/components/legend/_legend_list.scss deleted file mode 100644 index 41d95552fe..0000000000 --- a/src/components/legend/_legend_list.scss +++ /dev/null @@ -1,3 +0,0 @@ -.echLegendList { - display: flex; -} diff --git a/src/components/legend/legend.tsx b/src/components/legend/legend.tsx index bfdb6e9c8b..751c580ca6 100644 --- a/src/components/legend/legend.tsx +++ b/src/components/legend/legend.tsx @@ -10,7 +10,6 @@ import { Theme } from '../../utils/themes/theme'; interface LegendProps { chartStore?: ChartStore; // FIX until we find a better way on ts mobx - legendId: string; } interface LegendState { @@ -57,7 +56,6 @@ class LegendComponent extends React.Component { } render() { - const { legendId } = this.props; const { legendInitialized, chartInitialized, @@ -73,7 +71,7 @@ class LegendComponent extends React.Component { return null; } - const legendStyle = this.getLegendStyle(postion, chartTheme); + const legendContainerStyle = this.getLegendStyle(postion, chartTheme); const legendListStyle = this.getLegendListStyle(postion, chartTheme); const legendClasses = classNames('echLegend', `echLegend--${postion}`, { 'echLegend--debug': debug, @@ -81,8 +79,8 @@ class LegendComponent extends React.Component { }); return ( -
-
+
+
{[...legendItems.values()].map(this.renderLegendElement)}
From 8b9c663b1e0684c9eba945e0e330f3f6eed7c009 Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Wed, 21 Aug 2019 10:25:53 -0500 Subject: [PATCH 9/9] chore: remove unused styles --- src/components/_container.scss | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/components/_container.scss b/src/components/_container.scss index eb1576b225..ad0bad3076 100644 --- a/src/components/_container.scss +++ b/src/components/_container.scss @@ -5,12 +5,6 @@ .echContainer { flex: 1; position: relative; - - &:hover { - .echLegend__toggle { - opacity: 1; - } - } } .echChart {