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

Charts #1636

Open
natanfernandes opened this issue Jun 6, 2024 · 8 comments
Open

Charts #1636

natanfernandes opened this issue Jun 6, 2024 · 8 comments
Assignees
Labels
dev Indicates that the issue or pull request involves engineering considerations proposal Proposals for enhancements to the software

Comments

@natanfernandes
Copy link

natanfernandes commented Jun 6, 2024

Problem

Some of our admin applications currently utilize the same chart library (Recharts), but each application has implemented it differently. This fragmentation in implementation can affect our development process, user experience, and overall system efficiency (RFC). By addressing these problems, we can achieve a more cohesive and maintainable charting solution across all applications.

Solution

We've evaluated popular chart libraries to identify the one that best meets our needs in terms of features, performance, and ease of use, all of these points are in a RFC. At the final, we've decided that Echarts is the one that match our needs.

The main solution, consists in make a wrapper for Echarts lib, adding base styles for most used charts (Bar, Line for ex), in order to make a ready to use chart without any setup or customization, following the patterns and foundations of the chart library Figma made by our Design team, this would allow us to focus on mastering one tool, increasing efficiency and reducing learning curves.

Explain

  • Theme: we have a theme, supported by echarts, where we can define some default style for every chart type
  • Variants: In the themes, we cant define some styles on the chart serie, so @vtex/shoreline-charts have the the variants, custom object styles for different chart types, defining reusable a and customizing serie for that
  • Customizing: Even with the theme and variants, maybe the lib cant fit your need, but you can edit the option for the chart following the props of the echarts

At the final, what we're actually doing is:

  • Define some default styles in theme used by echarts and setting up in the component, by this, each chart of type bar will have this style unless the implementer override this
theme/themes.ts

defaultTheme: {...
  bar: {
    itemStyle: {
      borderRadius: [4, 4, 0, 0],
    },
  },
}

chart.tsx
<ReactECharts
  theme={defaultTheme}
/>
  • Accept the options (same as the original echarts props), so the implementer can edit and override
  • Create variants and default series styles, because some of styles are only changed in the series object (eg: horizontal bar chart):
theme/defaultStyles.ts

horizontal: {
  xAxis: {
    type: 'value',
  },
  yAxis: {
    type: 'category',
  },
  series: {
    type: 'bar',
    itemStyle: {
      borderRadius: [0, 4, 4, 0],
    },
  },
},
  • Merge the options object with our style object, attaching the default styles of our DS and the styles used by the implementer
utils/chart.ts

export const buildDefaultSerie = (
  serie: SeriesOption | SeriesOption[],
  defaultStyle: EChartsOption
): SeriesOption => {
  const seriesClone = cloneDeep(serie)
  const defaultStylesClone = cloneDeep(defaultStyle.series)
  const serieMerged = merge(defaultStylesClone, seriesClone) as SeriesOption

  return serieMerged
}

export const formatSeries = (
  series: SeriesOption | SeriesOption[] | undefined,
  defaultStyle: EChartsOption
) => {
  if (!series) return
  if (Array.isArray(series)) {
    return series.map((serie) => buildDefaultSerie(serie, defaultStyle))
  }

  return buildDefaultSerie(series, defaultStyle)
}

export const getChartOptions = (
  options: EChartsOption,
  type: ChartConfig['type'],
  variant: ChartConfig['variant']
): EChartsOption | undefined => {
  if (!options) return
  const { series, ...rest } = options
  const chartStyleType = CHART_STYLES[type]

  const defaultStyle = variant
    ? chartStyleType[variant]
    : chartStyleType.default

  const { series: defaultSeries, ...defaultRest } = defaultStyle

  const formattedSeries = formatSeries(series, defaultStyle)

  const mergedOptions = merge(defaultRest, rest)

  return { ...mergedOptions, series: formattedSeries }
}


chart.tsx

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


<ReactECharts
  theme={defaultTheme}
  option={chartOptions}
/>

Usage examples

At the bottom level, we'll have a base chart component, that wraps the base setup and functionalities of Echarts for any chart, which is already open.

And, we can use the base chart component to create the others and add their default styles:

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

With this basic component setup, we can create the base chart passing just the data for the xAxis and the data itself:
Captura de Tela 2024-06-11 às 16 20 11

But, the final user can decide to edit the chart on your own:

<Chart 
    option={{
      xAxis: {
          data: ['Mon', 'Tue', 'Wed', 'Thu', 'Fri', 'Sat', 'Sun'],
      },
      series: [{ data: [1, 2, 3, 4, 5, 6, 7] }, {data: [1, 2, 3, 4, 5, 6, 7], type: 'line'}],
   }} 
  chartConfig: { type: 'bar' , variant: 'default' },
  style={{
    height: 550
  }} 
/>

Captura de Tela 2024-06-11 às 16 32 53

Dependencies

As main dependency we have Echarts, which is the library that we're wrapping for our charts

References

RFC about charting library choose for Healthmonitor
Figma presentation

@natanfernandes natanfernandes added proposal Proposals for enhancements to the software status: pending labels Jun 6, 2024
@natanfernandes
Copy link
Author

TODO: explore a little bit more themes, setting the default values for each chart type in themes

@beatrizmilhomem beatrizmilhomem added status: discussing design Indicates that the issue or pull request involves design considerations dev Indicates that the issue or pull request involves engineering considerations and removed status: pending labels Jun 19, 2024
@beatrizmilhomem
Copy link
Contributor

Hello, @natanfernandes !
In a discussion with the maintainers, we've decided it would be better to block this issue for now. We understand that the initiative has advanced a lot and the design is in the final stages, but the approval from the maintainers is still pending on the design side. We are aligning with the designers so that can happen and everything is ready for you to move on. If you have any questions, please contact any of the maintainers!

@beatrizmilhomem beatrizmilhomem added blocked Blocked by some reason and removed status: discussing labels Jun 20, 2024
@beatrizmilhomem beatrizmilhomem linked a pull request Jun 24, 2024 that will close this issue
@beatrizmilhomem beatrizmilhomem moved this to Discussing in Shoreline Jul 17, 2024
@davicostalf
Copy link
Contributor

Related with #1700

@gabriellymoura
Copy link

Folks, have you any updates about this proposal? Here it's a RFC with a chart research. Is there any prediction on when we can implement it?

@beatrizmilhomem
Copy link
Contributor

@gabriellymoura The issue was blocked due to some topics that were still being defined on the design level.


@natanfernandes In a discussion with Bia, Luiz, and Ju, we've agreed that it is possible to go back to implementing charts. We had some questions about tokens usage and style, and that was solved.

  • Bia and Luiz will align with you on the next steps.
  • Releases can be divided into three parts, according to the type of charts, and they would include all assets related to a chart: Figma, code, and documentation. That would give a sense of progression and you could release faster what you've completed.

@beatrizmilhomem beatrizmilhomem removed the blocked Blocked by some reason label Aug 27, 2024
@natanfernandes
Copy link
Author

Hi @gabriellymoura we're back to implement this feature and hope to get back here soon with the first version!

@gabriellymoura
Copy link

Hi @gabriellymoura we're back to implement this feature and hope to get back here soon with the first version!

Great news! @natanfernandes do you have any doc (RFC, TDD) or action plan for these features? I ask this because I want to share with my team (Data analytics), which aims to develop charts on the shoreline too.

@beatrizmilhomem beatrizmilhomem moved this from Discussing to Active in Shoreline Oct 9, 2024
@beatrizmilhomem beatrizmilhomem removed the design Indicates that the issue or pull request involves design considerations label Oct 9, 2024
@natanfernandes
Copy link
Author

natanfernandes commented Oct 15, 2024

Hi @gabriellymoura we're back to implement this feature and hope to get back here soon with the first version!

Great news! @natanfernandes do you have any doc (RFC, TDD) or action plan for these features? I ask this because I want to share with my team (Data analytics), which aims to develop charts on the shoreline too.

At this moment we just have this issue as main documentation, i'll update with more details about nexts steps and external contributions, we're creating a roadmap of features and their respective documentation, so the contributors can easily collaborate following the chart patterns. Our first release (will be primary the bar chart) aims to go live in mid november.

@natanfernandes natanfernandes removed a link to a pull request Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev Indicates that the issue or pull request involves engineering considerations proposal Proposals for enhancements to the software
Projects
Status: Active
Development

No branches or pull requests

4 participants