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

fix(Charts - New): fix rules of hooks & consistent margin calculation #459

Merged
merged 37 commits into from
Apr 28, 2020
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
e8b795e
remove unused chart margin from radar chart
MarcusNotheis Apr 23, 2020
20bea21
fix useChartMargin for complex accessors
MarcusNotheis Apr 23, 2020
db5abdd
Add color and font family to container
MarcusNotheis Apr 23, 2020
9c2138d
remove useMemo from useLabelElements
MarcusNotheis Apr 23, 2020
d69be79
refactor: useDataLabel
MarcusNotheis Apr 23, 2020
b8357ad
Merge branch 'master' into fix/charts
kleinju1017 Apr 24, 2020
ce1b36b
Merge branch 'master' into fix/charts
kleinju1017 Apr 27, 2020
b8d8932
calculate marginLeft by measures
kleinju1017 Apr 27, 2020
8067c41
update to recharts-beta.5
MarcusNotheis Apr 27, 2020
1d85c13
Merge branch 'fix/charts' of https://github.com/SAP/ui5-webcomponents…
MarcusNotheis Apr 27, 2020
024f94d
fix label positioning for bars in composed charts
MarcusNotheis Apr 27, 2020
eb7c929
fix(hide data labels when there are too much data)
kleinju1017 Apr 27, 2020
bcbbaf6
fix (reference line in composed chart)
kleinju1017 Apr 27, 2020
22a748a
Combined Chart: Support Vertical Layout
MarcusNotheis Apr 27, 2020
c3c195e
Merge branch 'fix/charts' of https://github.com/SAP/ui5-webcomponents…
MarcusNotheis Apr 27, 2020
754c9f8
added maxBarSize
kleinju1017 Apr 27, 2020
d9055cc
Merge branch 'fix/charts' of https://github.com/SAP/ui5-webcomponents…
kleinju1017 Apr 27, 2020
da57c4b
Revert "Merge branch 'fix/charts' of https://github.com/SAP/ui5-webco…
kleinju1017 Apr 27, 2020
11cf080
Revert "added maxBarSize"
kleinju1017 Apr 27, 2020
984357c
Update index.tsx
kleinju1017 Apr 27, 2020
3ad5464
refactoring
MarcusNotheis Apr 27, 2020
b75d044
Merge branch 'fix/charts' of https://github.com/SAP/ui5-webcomponents…
MarcusNotheis Apr 27, 2020
41ff903
LabelColoring
MarcusNotheis Apr 27, 2020
91440dd
Update tests
MarcusNotheis Apr 27, 2020
a1b6644
simplify useChartMargin
MarcusNotheis Apr 28, 2020
ca7e3cc
fix build
MarcusNotheis Apr 28, 2020
c4eaba6
improve bundle size
MarcusNotheis Apr 28, 2020
7954d8c
adjust size of loading placeholders
MarcusNotheis Apr 28, 2020
fbdd4a7
fix(snapshot tests of charts)
kleinju1017 Apr 28, 2020
88d8c23
feat: add composed chart placeholder
MarcusNotheis Apr 28, 2020
ae6d0b3
Merge branch 'fix/charts' of https://github.com/SAP/ui5-webcomponents…
MarcusNotheis Apr 28, 2020
ec5558d
Update jest.config.js
MarcusNotheis Apr 28, 2020
dbc251d
exlude lib sub directories from coverage
MarcusNotheis Apr 28, 2020
b6b45fd
Improved padding of composed chart
kleinju1017 Apr 28, 2020
333e7d5
code clean up
kleinju1017 Apr 28, 2020
20e2095
Update index.tsx
MarcusNotheis Apr 28, 2020
5c49864
add more default padding
MarcusNotheis Apr 28, 2020
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
1 change: 1 addition & 0 deletions .storybook/preview-head.html
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<style>
body {
font-family: '72', Arial, Helvetica, sans-serif;
background-color: var(--sapBackgroundColor);
}

.pageWithPadding > section {
Expand Down
2 changes: 1 addition & 1 deletion packages/charts/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
"lodash.merge": "^4.6.2",
"react-chartjs-2": "^2.8.0",
"react-content-loader": "^5.0.4",
"recharts": "2.0.0-beta.1"
"recharts": "2.0.0-beta.5"
},
"devDependencies": {
"@types/chart.js": "^2.9.8"
Expand Down
2 changes: 1 addition & 1 deletion packages/charts/rollup.config.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const rollupConfigFactory = require('../../shared/rollup/configFactory');

const config = rollupConfigFactory('charts', ['@ui5/webcomponents', '@ui5/webcomponents-base']);
const config = rollupConfigFactory('charts', ['@ui5/webcomponents', '@ui5/webcomponents-base', 'lodash', 'classnames']);
module.exports = config;
30 changes: 10 additions & 20 deletions packages/charts/src/components/BarChart/BarChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,17 @@ import {
YAxis
} from 'recharts';
import { useChartMargin } from '../../hooks/useChartMargin';
import { useAxisLabel, useDataLabel, useSecondaryDimensionLabel } from '../../hooks/useLabelElements';
import { useSecondaryDimensionLabel } from '../../hooks/useLabelElements';
import { usePrepareDimensionsAndMeasures } from '../../hooks/usePrepareDimensionsAndMeasures';
import { useTooltipFormatter } from '../../hooks/useTooltipFormatter';
import { IChartDimension } from '../../interfaces/IChartDimension';
import { IChartMeasure } from '../../interfaces/IChartMeasure';
import { RechartBaseProps } from '../../interfaces/RechartBaseProps';
import { ChartDataLabel } from '@ui5/webcomponents-react-charts/lib/components/ChartDataLabel';
import { XAxisTicks } from '@ui5/webcomponents-react-charts/lib/components/XAxisTicks';
import { YAxisTicks } from '@ui5/webcomponents-react-charts/lib/components/YAxisTicks';

const formatYAxisTicks = (tick) => {
const formatYAxisTicks = (tick = '') => {
const splitTick = tick.split(' ');
return splitTick.length > 3
? `${splitTick.slice(0, 3).join(' ')}...`
Expand Down Expand Up @@ -164,16 +167,13 @@ const BarChart: FC<BarChartProps> = forwardRef((props: BarChartProps, ref: Ref<a

const marginChart = useChartMargin(
dataset,
(d) => d,
primaryDimensionAccessor,
dimensions,
chartConfig.margin,
true,
dimensions.length > 1,
chartConfig.zoomingTool
);

const XAxisLabel = useAxisLabel(primaryMeasure?.formatter);

return (
<ChartContainer
dataset={dataset}
Expand All @@ -185,7 +185,7 @@ const BarChart: FC<BarChartProps> = forwardRef((props: BarChartProps, ref: Ref<a
tooltip={tooltip}
slot={slot}
>
<BarChartLib margin={marginChart} layout={'vertical'} data={dataset} barGap={chartConfig.barGap}>
<BarChartLib margin={marginChart} layout="vertical" data={dataset} barGap={chartConfig.barGap}>
<CartesianGrid
vertical={chartConfig.gridVertical ?? false}
horizontal={chartConfig.gridHorizontal}
Expand All @@ -195,17 +195,15 @@ const BarChart: FC<BarChartProps> = forwardRef((props: BarChartProps, ref: Ref<a
<XAxis
interval={0}
type="number"
tick={XAxisLabel}
tick={<XAxisTicks config={primaryMeasure} />}
axisLine={chartConfig.xAxisVisible ?? true}
tickFormatter={primaryMeasure?.formatter}
/>
)}
{(chartConfig.yAxisVisible ?? true) &&
dimensions.map((dimension, index) => {
const YAxisLabel =
index > 0
? useSecondaryDimensionLabel(true, dimension.formatter)
: useAxisLabel(dimension.formatter, true);
index > 0 ? useSecondaryDimensionLabel(true, dimension.formatter) : <YAxisTicks config={dimension} />;
return (
<YAxis
interval={dimension?.interval ?? (isBigDataSet ? 'preserveStart' : 0)}
Expand All @@ -221,22 +219,14 @@ const BarChart: FC<BarChartProps> = forwardRef((props: BarChartProps, ref: Ref<a
);
})}
{measures.map((element, index) => {
const BarDataLabel = useDataLabel(
!element.hideDataLabel,
element.DataLabel,
element.formatter,
false,
true,
false
);
return (
<Bar
stackId={element.stackId}
fillOpacity={element.opacity}
key={element.accessor}
name={element.label ?? element.accessor}
strokeOpacity={element.opacity}
label={BarDataLabel}
label={<ChartDataLabel config={element} chartType="bar" position="insideRight" />}
type="monotone"
dataKey={element.accessor}
fill={element.color ?? `var(--sapChart_OrderedColor_${(index % 11) + 1})`}
Expand Down
16 changes: 15 additions & 1 deletion packages/charts/src/components/BarChart/BarRechart.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,21 @@ export const withReferenceLineStory = () => {
accessor: 'volume'
}
]}
style={{ width: '95%', height: '70vh' }}
style={{
width: '95%',
height: '70vh',
'--sapChart_OrderedColor_1': '#0f828f',
'--sapChart_OrderedColor_2': '#5ac2ce',
'--sapChart_OrderedColor_3': '#03734d',
'--sapChart_OrderedColor_4': '#66c2a3',
'--sapChart_OrderedColor_5': '#3c6372',
'--sapChart_OrderedColor_6': '#adbcc3',
'--sapChart_OrderedColor_7': '#144b7f',
'--sapChart_OrderedColor_8': '#698caf',
'--sapChart_OrderedColor_9': '#d62f2f',
'--sapChart_OrderedColor_10': '#f8a6a6',
'--sapChart_OrderedColor_11': '#921473'
}}
noLegend={false}
loading
chartConfig={{
Expand Down
6 changes: 4 additions & 2 deletions packages/charts/src/components/BarChart/Placeholder.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@ import ContentLoader from 'react-content-loader';
export const BarChartPlaceholder = () => {
return (
<ContentLoader
height={145}
width={165}
viewBox="0 0 165 145"
preserveAspectRatio="xMidYMid meet"
width="100%"
height="100%"
speed={2}
backgroundColor={ThemingParameters.sapContent_ImagePlaceholderBackground}
foregroundColor={ThemingParameters.sapContent_ImagePlaceholderForegroundColor}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

exports[`BarRechart Renders with data 1`] = `
<div
style="width: 100%; height: 60vh; position: relative;"
style="font-family: var(--sapFontFamily); width: 100%; height: 60vh; position: relative;"
>
<div
class="recharts-responsive-container"
Expand All @@ -16,7 +16,7 @@ exports[`BarRechart Renders with data 1`] = `

exports[`BarRechart loading placeholder 1`] = `
<div
style="width: 30%; height: 400px; position: relative;"
style="font-family: var(--sapFontFamily); width: 30%; height: 400px; position: relative;"
>
<svg
aria-labelledby="ARIA-LABELLED-BY"
Expand Down
9 changes: 2 additions & 7 deletions packages/charts/src/components/BarChart/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,8 @@ import { useLegend, useLegendItemClickHandler } from '../../internal/ChartLegend
import { getCssVariableValue } from '../../themes/Utils';
import { ChartBaseDefaultProps } from '../../util/ChartBaseDefaultProps';
import { useChartData } from '../../util/populateData';
import {
formatAxisCallback,
formatDataLabel,
formatTooltipLabel,
getTextWidth,
useMergedConfig
} from '../../util/Utils';
import { getTextWidth } from '../../util/Utils';
import { formatAxisCallback, formatDataLabel, formatTooltipLabel, useMergedConfig } from '../../util/utils_deprecated';
import { BarChartPlaceholder } from './Placeholder';

export interface BarChartPropTypes extends ChartBaseProps {}
Expand Down
28 changes: 11 additions & 17 deletions packages/charts/src/components/ColumnChart/ColumnChart.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { enrichEventWithDetails } from '@ui5/webcomponents-react-base/lib/Utils';
import { ThemingParameters } from '@ui5/webcomponents-react-base/lib/ThemingParameters';
import { useConsolidatedRef } from '@ui5/webcomponents-react-base/lib/useConsolidatedRef';
import { enrichEventWithDetails } from '@ui5/webcomponents-react-base/lib/Utils';
import { ColumnChartPlaceholder } from '@ui5/webcomponents-react-charts/lib/ColumnChartPlaceholder';
import { ChartContainer } from '@ui5/webcomponents-react-charts/lib/next/ChartContainer';
import { useLegendItemClick } from '@ui5/webcomponents-react-charts/lib/useLegendItemClick';
Expand All @@ -16,13 +16,16 @@ import {
XAxis,
YAxis
} from 'recharts';
import { useChartMargin } from '../../hooks/useChartMargin';
import { useSecondaryDimensionLabel } from '../../hooks/useLabelElements';
import { usePrepareDimensionsAndMeasures } from '../../hooks/usePrepareDimensionsAndMeasures';
import { useTooltipFormatter } from '../../hooks/useTooltipFormatter';
import { IChartDimension } from '../../interfaces/IChartDimension';
import { IChartMeasure } from '../../interfaces/IChartMeasure';
import { RechartBaseProps } from '../../interfaces/RechartBaseProps';
import { useDataLabel, useAxisLabel, useSecondaryDimensionLabel } from '../../hooks/useLabelElements';
import { useChartMargin } from '../../hooks/useChartMargin';
import { useTooltipFormatter } from '../../hooks/useTooltipFormatter';
import { ChartDataLabel } from '@ui5/webcomponents-react-charts/lib/components/ChartDataLabel';
import { XAxisTicks } from '@ui5/webcomponents-react-charts/lib/components/XAxisTicks';
import { YAxisTicks } from '@ui5/webcomponents-react-charts/lib/components/YAxisTicks';

interface MeasureConfig extends IChartMeasure {
/**
Expand Down Expand Up @@ -168,8 +171,7 @@ const ColumnChart: FC<ColumnChartProps> = forwardRef((props: ColumnChartProps, r

const marginChart = useChartMargin(
dataset,
(d) => d,
primaryDimensionAccessor,
measures,
chartConfig.margin,
false,
dimensions.length > 1,
Expand All @@ -195,14 +197,13 @@ const ColumnChart: FC<ColumnChartProps> = forwardRef((props: ColumnChartProps, r
/>
{(chartConfig.xAxisVisible ?? true) &&
dimensions.map((dimension, index) => {
const XAxisLabel = useAxisLabel(dimension.formatter);
return (
<XAxis
key={dimension.accessor}
dataKey={dimension.accessor}
xAxisId={index}
interval={dimension?.interval ?? (isBigDataSet ? 'preserveStart' : 0)}
tick={index === 0 ? XAxisLabel : SecondaryDimensionLabel}
tick={index === 0 ? <XAxisTicks config={dimension} /> : SecondaryDimensionLabel}
tickLine={index < 1}
axisLine={index < 1}
/>
Expand All @@ -212,8 +213,8 @@ const ColumnChart: FC<ColumnChartProps> = forwardRef((props: ColumnChartProps, r
axisLine={chartConfig.yAxisVisible ?? false}
tickLine={false}
yAxisId="left"
tickFormatter={primaryMeasure?.formatter}
interval={0}
tick={<YAxisTicks config={primaryMeasure} />}
/>
{chartConfig.secondYAxis && chartConfig.secondYAxis.dataKey && (
<YAxis
Expand All @@ -226,13 +227,6 @@ const ColumnChart: FC<ColumnChartProps> = forwardRef((props: ColumnChartProps, r
/>
)}
{measures.map((element, index) => {
const ColumnDataLabel = useDataLabel(
!element.hideDataLabel,
element.DataLabel,
element.formatter,
!!element.stackId,
false
);
return (
<Column
yAxisId={chartConfig?.secondYAxis?.dataKey === element.accessor ? 'right' : 'left'}
Expand All @@ -241,7 +235,7 @@ const ColumnChart: FC<ColumnChartProps> = forwardRef((props: ColumnChartProps, r
key={element.accessor}
name={element.label ?? element.accessor}
strokeOpacity={element.opacity}
label={ColumnDataLabel}
label={<ChartDataLabel config={element} chartType="column" position={'insideTop'} />}
type="monotone"
dataKey={element.accessor}
fill={element.color ?? `var(--sapChart_OrderedColor_${(index % 11) + 1})`}
Expand Down
6 changes: 4 additions & 2 deletions packages/charts/src/components/ColumnChart/Placeholder.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@ import ContentLoader from 'react-content-loader';
export const ColumnChartPlaceholder = (props) => {
return (
<ContentLoader
height={145}
width={165}
viewBox="0 0 165 145"
preserveAspectRatio="xMidYMid meet"
width="100%"
height="100%"
speed={2}
backgroundColor={ThemingParameters.sapContent_ImagePlaceholderBackground}
foregroundColor={ThemingParameters.sapContent_ImagePlaceholderForegroundColor}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

exports[`ColumnRechart Renders with data 1`] = `
<div
style="width: 100%; height: 60vh; position: relative;"
style="font-family: var(--sapFontFamily); width: 100%; height: 60vh; position: relative;"
>
<div
class="recharts-responsive-container"
Expand All @@ -16,7 +16,7 @@ exports[`ColumnRechart Renders with data 1`] = `

exports[`ColumnRechart loading placeholder 1`] = `
<div
style="width: 30%; height: 400px; position: relative;"
style="font-family: var(--sapFontFamily); width: 30%; height: 400px; position: relative;"
>
<svg
aria-labelledby="ARIA-LABELLED-BY"
Expand Down
10 changes: 2 additions & 8 deletions packages/charts/src/components/ColumnChart/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,8 @@ import { useLegend, useLegendItemClickHandler } from '../../internal/ChartLegend
import { getCssVariableValue } from '../../themes/Utils';
import { ChartBaseDefaultProps } from '../../util/ChartBaseDefaultProps';
import { useChartData } from '../../util/populateData';
import {
formatAxisCallback,
formatDataLabel,
formatTooltipLabel,
getTextHeight,
getTextWidth,
useMergedConfig
} from '../../util/Utils';
import { getTextHeight, getTextWidth } from '../../util/Utils';
import { formatAxisCallback, formatDataLabel, formatTooltipLabel, useMergedConfig } from '../../util/utils_deprecated';
import { ColumnChartPlaceholder } from './Placeholder';

export interface ColumnChartPropTypes extends ChartBaseProps {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { action } from '@storybook/addon-actions';
import { ComposedChart } from '@ui5/webcomponents-react-charts/lib/next/ComposedChart';
import React from 'react';
import { bigDataSet, complexDataSet, secondaryDimensionDataSet, simpleDataSet } from '../../resources/DemoProps';
import { boolean } from '@storybook/addon-knobs';
import { boolean, select } from '@storybook/addon-knobs';

export default {
title: 'Charts - Unstable / ComposedChart',
Expand All @@ -17,6 +17,7 @@ export const renderStory = () => {
onLegendClick={action('onLegendClick')}
dataset={complexDataSet}
style={{ height: '60vh' }}
layout={select('layout', ['horizontal', 'vertical'], 'horizontal')}
dimensions={[
{
accessor: 'name',
Expand All @@ -34,7 +35,7 @@ export const renderStory = () => {
accessor: 'users',
label: 'Users',
formatter: (val) => val.toLocaleString(),
type: 'line'
type: 'area'
},
{
accessor: 'volume',
Expand Down Expand Up @@ -162,6 +163,7 @@ export const withReferenceLineStory = () => {
onLegendClick={action('onLegendClick')}
dataset={bigDataSet}
dimensions={[{ accessor: 'name' }]}
layout={select('layout', ['horizontal', 'vertical'], 'horizontal')}
measures={[
{
accessor: 'users',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

exports[`ComposedChart Renders with data 1`] = `
<div
style="width: 100%; height: 60vh; position: relative;"
style="font-family: var(--sapFontFamily); width: 100%; height: 60vh; position: relative;"
>
<div
class="recharts-responsive-container"
Expand All @@ -16,7 +16,7 @@ exports[`ComposedChart Renders with data 1`] = `

exports[`ComposedChart loading placeholder 1`] = `
<div
style="width: 50%; height: 400px; position: relative;"
style="font-family: var(--sapFontFamily); width: 50%; height: 400px; position: relative;"
>
<svg
aria-labelledby="ARIA-LABELLED-BY"
Expand Down
Loading