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

Feature/charts package #1648

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
2a42d22
feat: setup with echarts for react component and tests
natanfernandes Jun 13, 2024
3bd8c56
fix: add default value to loading and lint
natanfernandes Jun 13, 2024
f0ed931
Merge branch 'main' of https://github.com/vtex/shoreline into feature…
natanfernandes Jun 13, 2024
30150af
fix: biomejs on vscode
natanfernandes Jun 13, 2024
414b773
fix: color and add props export
natanfernandes Jun 13, 2024
4546750
fix: canUseDom and array check
natanfernandes Jun 17, 2024
8b38b74
feat: add chartConfig props to validate variant and type
natanfernandes Jun 18, 2024
4218d1b
fix: wrong colors on the chart labels and lines
natanfernandes Jun 21, 2024
e417a65
Merge branch 'main' of https://github.com/vtex/shoreline into feature…
natanfernandes Oct 1, 2024
4ef950c
feat: use cloneDeep from lodash, bar default styles
natanfernandes Oct 1, 2024
a45b6bb
feat: add sunrise colors, story with controls and fix somes styles in…
natanfernandes Oct 23, 2024
87a20b7
Merge branch 'main' of https://github.com/vtex/shoreline into feature…
natanfernandes Oct 23, 2024
b00e7f0
fix: pnpm lock for chart lib
natanfernandes Oct 23, 2024
c3db54f
fix: types and spacing grid
natanfernandes Oct 23, 2024
6d6e0a9
fix: legends spacing
natanfernandes Oct 23, 2024
72344e0
feat: add tooltip on charts
natanfernandes Nov 27, 2024
1e6e2ba
fix: lock pnpm
natanfernandes Nov 27, 2024
95a80a6
chore: add chart docs
natanfernandes Nov 27, 2024
581abae
feat: export css chart bundle and import on docs
natanfernandes Nov 28, 2024
4abd5f8
chore: start to add doc to chart variants
natanfernandes Nov 28, 2024
1589a97
docs: improve doc props
natanfernandes Dec 10, 2024
e905a0f
Merge branch 'main' of https://github.com/vtex/shoreline into feature…
natanfernandes Dec 19, 2024
87ce98c
fix: pnpm lock
natanfernandes Dec 19, 2024
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
7 changes: 7 additions & 0 deletions packages/charts/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# Shoreline Charts

`shoreline-components` and `echarts` are peer dependencies of `shoreline-charts`

```sh
pnpm add @vtex/shoreline echarts @vtex/shoreline-charts
```
56 changes: 56 additions & 0 deletions packages/charts/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
{
"name": "@vtex/shoreline-charts",
"description": "Shoreline datavis library",
"version": "0.0.0",
"main": "./dist/index.js",
"module": "./dist/index.mjs",
"types": "./dist/index.d.ts",
"publishConfig": {
"access": "public",
"registry": "https://registry.npmjs.org"
},
"files": [
"dist"
],
"exports": {
".": {
"require": "./dist/index.js",
"import": "./dist/index.mjs",
"types": "./dist/index.d.ts"
}
},
"engines": {
"node": ">=16"
},
"scripts": {
"prebuild": "rm -rf dist",
"dev": "tsup --watch",
"build": "tsup"
},
"repository": {
"directory": "packages/charts",
"type": "git",
"url": "git+https://github.com/vtex/shoreline.git"
},
"bugs": {
"url": "https://github.com/vtex/shoreline/issues"
},
"peerDependencies": {
"@vtex/shoreline": "1.x",
"echarts": "5.x",
"react": "18.x",
"react-dom": "18.x"
},
"devDependencies": {
"@types/lodash": "^4.17.4",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"@types/lodash": "^4.17.4",

"@vtex/shoreline": "workspace:*",
"echarts": "5.5.0"
},
"dependencies": {
"@vtex/shoreline-utils": "workspace:*",
"echarts-for-react": "^3.0.2",
"lodash": "^4.17.21",
Copy link
Contributor

Choose a reason for hiding this comment

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

You're not using it, right?

Suggested change
"lodash": "^4.17.21",

Copy link
Author

Choose a reason for hiding this comment

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

Right, i'll remove it

"vitest": "^1.6.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

vistest is already on the global scope.

Suggested change
"vitest": "^1.6.0",

"vitest-canvas-mock": "^0.3.3"
}
}
103 changes: 103 additions & 0 deletions packages/charts/src/chart.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
import {
useRef,
useEffect,
useMemo,
forwardRef,
useImperativeHandle,
type ComponentPropsWithRef,
useCallback,
} from 'react'
import type { EChartsOption, SetOptionOpts } from 'echarts'
import ReactECharts from 'echarts-for-react'
import type * as echarts from 'echarts'

import { defaultTheme } from './theme/themes'
import type { ChartTypes, ChartVariants } from './types/chart'
import { getChartOptions } from './utils/chart'

/**
* Render a Shoreline Chart with echarts
* @see https://echarts.apache.org/en/index.html
*/
export const Chart = forwardRef<echarts.EChartsType | undefined, ChartProps>(
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this undefined?

Copy link
Author

@natanfernandes natanfernandes Jun 17, 2024

Choose a reason for hiding this comment

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

I was facing a type error with ref, on the useImperativeHandle and tried the undefined

Copy link
Contributor

Choose a reason for hiding this comment

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

What does this echarts.ECartsType return? Some HTMLDivElement?

Copy link
Author

Choose a reason for hiding this comment

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

It returns the Chart instance, the same as in chartRef.current.getEchartsInstance()

function Charts(props, ref) {
const {
option,
settings,
loading = false,
variant = 'default',
type,
style,
...otherProps
} = props

const chartRef = useRef<ReactECharts>(null)

useImperativeHandle(ref, () => {
if (chartRef.current) {
return chartRef.current.getEchartsInstance()
}
return undefined
})

const chartOptions: EChartsOption = useMemo(() => {
return getChartOptions(option, type, variant) || option
}, [option, type, variant])

const handleResize = useCallback(() => {
if (chartRef.current) {
chartRef.current.getEchartsInstance().resize()
}
}, [])

useEffect(() => {
window.addEventListener('resize', handleResize)
return () => {
window.removeEventListener('resize', handleResize)
}
}, [handleResize])
Copy link
Contributor

@matheusps matheusps Jun 17, 2024

Choose a reason for hiding this comment

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

You can add a check for dom. It is useful for SSR. You can use the canUseDom util from @vtex/shoreline utils

Suggested change
useEffect(() => {
window.addEventListener('resize', handleResize)
return () => {
window.removeEventListener('resize', handleResize)
}
}, [handleResize])
useEffect(() => {
if(!canUseDom) return
window.addEventListener('resize', handleResize)
return () => {
window.removeEventListener('resize', handleResize)
}
}, [handleResize])

Copy link
Author

Choose a reason for hiding this comment

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

Nice tip!


if (loading) return <div>loading...</div>

return (
<div data-sl-chart {...otherProps}>
<ReactECharts
ref={chartRef}
theme={defaultTheme}
option={chartOptions}
style={style}
opts={{
renderer: 'svg',
}}
/>
</div>
)
}
)

export interface ChartsOptions {
/**
* Echarts options
*/
option: EChartsOption
/**
* Echarts settings
*/
settings?: SetOptionOpts
/**
* Wether is loading
* @default false
*/
loading?: boolean
/**
* Chart type to be rendered
*/
type: ChartTypes
/**
* Pre-defined chart style for each type
* @default default
*/
variant?: ChartVariants
}

export type ChartProps = ChartsOptions & ComponentPropsWithRef<'div'>
2 changes: 2 additions & 0 deletions packages/charts/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export { Chart } from './chart'
export type { ChartProps } from './chart'
58 changes: 58 additions & 0 deletions packages/charts/src/stories/bar-charts.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import { Chart } from '../index'

export default {
title: 'Charts/bar',
}

export function Basic() {
return (
<Chart
option={{
xAxis: {
data: ['Mon', 'Tue', 'Wed', 'Thu', 'Fri', 'Sat', 'Sun'],
},
series: { data: [1, 2, 3, 4, 5, 6, 7] },
}}
type="bar"
style={{ height: 550 }}
/>
)
}

export function Horizontal() {
return (
<Chart
option={{
xAxis: {
data: ['Mon', 'Tue', 'Wed', 'Thu', 'Fri', 'Sat', 'Sun'],
},
series: [
{ data: [1, 2, 3, 4, 5, 6, 7] },
{ data: [1, 4, 2, 1, 4, 3, 5] },
],
}}
type="bar"
variant="horizontal"
style={{ height: 550 }}
/>
)
}

export function MultiType() {
return (
<Chart
option={{
xAxis: {
data: ['Mon', 'Tue', 'Wed', 'Thu', 'Fri', 'Sat', 'Sun'],
},
series: [
{ data: [1, 2, 3, 4, 5, 6, 7] },
{ data: [1, 4, 2, 1, 4, 3, 5], type: 'line' },
],
}}
type="bar"
variant="default"
style={{ height: 550 }}
/>
)
}
8 changes: 8 additions & 0 deletions packages/charts/src/tests/__fixtures__/chartData.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
export const BAR_CHART_DATA = {
xAxis: {
weekdays: ['Mon', 'Tue', 'Wed', 'Thu', 'Fri', 'Sat', 'Sun'],
},
series: {
dayNumbers: [1, 2, 3, 4, 5, 6, 7],
},
}
38 changes: 38 additions & 0 deletions packages/charts/src/tests/charts.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import {
describe,
expect,
test,
render,
waitFor,
screen,
} from '@vtex/shoreline-test-utils'
import { Chart } from '../chart'
import { BAR_CHART_DATA } from './__fixtures__/chartData'

describe('@vtex.shoreline-charts bar chart tests', () => {
test('renders the bar chart with correct data', async () => {
const { container } = render(
<Chart
option={{
xAxis: {
data: BAR_CHART_DATA.xAxis.weekdays,
},
series: { data: BAR_CHART_DATA.series.dayNumbers },
}}
type="bar"
style={{ width: '100%', height: '400px' }}
/>
)

const divChartContainer = container.querySelector('[data-sl-chart]')
await waitFor(() => expect(divChartContainer).toBeInTheDocument())

BAR_CHART_DATA.xAxis.weekdays.forEach((value) =>
waitFor(() => expect(screen.queryByText(value)).toBeInTheDocument())
)

BAR_CHART_DATA.series.dayNumbers.forEach((value) =>
waitFor(() => expect(screen.queryByText(value)).toBeInTheDocument())
)
})
})
20 changes: 20 additions & 0 deletions packages/charts/src/tests/vitest.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { vi } from 'vitest'
import 'vitest-canvas-mock'

vi.mock('echarts', async () => {
const echarts = await vi.importActual<typeof import('echarts')>('echarts')
return {
...echarts,
init: vi.fn(() => {
return {
setOption: vi.fn(),
resize: vi.fn(),
getOption: vi.fn(),
dispose: vi.fn(),
clear: vi.fn(),
on: vi.fn(),
off: vi.fn(),
}
}),
}
})
29 changes: 29 additions & 0 deletions packages/charts/src/theme/chartStyles.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
export const CHART_STYLES = {
bar: {
default: {
xAxis: {
type: 'category',
},
yAxis: {
type: 'value',
},
series: {
type: 'bar',
},
},
horizontal: {
xAxis: {
type: 'value',
},
yAxis: {
type: 'category',
},
series: {
type: 'bar',
itemStyle: {
borderRadius: [0, 4, 4, 0],
},
},
},
},
}
20 changes: 20 additions & 0 deletions packages/charts/src/theme/colors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
export const CATEGORICAL = {
primary: '#014592',
secondary: '#9C56F3',
tertiary: '#0D504D',
quaternary: '#CA226A',
quinary: '#F95D47',
senary: '#5C12B6',
septenary: '#08A822',
octonary: '#EF5997',
nonary: '#157BF4',
denary: '#B18D01',
undenary: '#013A5E',
duodenary: '#01A29B;',
ternary: '#B24D01',
fourteen: '#720000',
}
Copy link
Contributor

@matheusps matheusps Jun 17, 2024

Choose a reason for hiding this comment

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

It looks to me that this naming could be simpler. For example: 100, 200, 300, 400, 500... OR 1, 2, 3, 4, 5... You would write less and semantic actually gets lost after quaternary, for example. What do you think?

Copy link
Author

Choose a reason for hiding this comment

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

Maybe we can achieve a better way to name it, for now i'm was following the token names defined in Figma

Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about this naming, @beatrizmilhomem, @davicostalf ?


export const BASE = {
lineColor: '#ADADAD',
}
45 changes: 45 additions & 0 deletions packages/charts/src/theme/themes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { BASE, CATEGORICAL } from './colors'

export const defaultTheme = {
color: [
CATEGORICAL.primary,
CATEGORICAL.secondary,
CATEGORICAL.tertiary,
CATEGORICAL.quaternary,
CATEGORICAL.quinary,
CATEGORICAL.senary,
CATEGORICAL.septenary,
CATEGORICAL.octonary,
CATEGORICAL.nonary,
CATEGORICAL.denary,
CATEGORICAL.undenary,
CATEGORICAL.duodenary,
CATEGORICAL.ternary,
CATEGORICAL.fourteen,
],
categoryAxis: {
axisTick: {
show: false,
},
axisLine: {
show: true,
lineStyle: {
color: BASE.lineColor,
},
},
},
valueAxis: {
type: 'value',
axisLine: {
show: true,
lineStyle: {
color: BASE.lineColor,
},
},
},
bar: {
itemStyle: {
borderRadius: [4, 4, 0, 0],
},
},
}
Loading
Loading