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

[Feedback] Future for data visualization #2577

Closed
matuzalemsteles opened this issue Oct 3, 2019 · 22 comments
Closed

[Feedback] Future for data visualization #2577

matuzalemsteles opened this issue Oct 3, 2019 · 22 comments

Comments

@matuzalemsteles
Copy link
Member

@matuzalemsteles matuzalemsteles commented Oct 3, 2019

Hey guys, this is an issue to start a thread about the future of data visualization in Clay (ClayCharts), we are wondering how you use Charts today, what your expectations were and what your pains were about the current Charts with Billboard.js.

An important point to note, dealing with charts is complex and requires a learning curve, we have many libraries out there that deliver different data visualization values, we want to help you build charts quickly according to Lexicon specifications and provide documentation so you can build more complex charts.

We're planning to make significant changes to the Charts, whether it's library shifting, improving APIs, writing new components... based on what we'll be seeing. These are some of the points we want to get with ClayCharts:

  • Performance
  • Flexibility
  • Consistency with Lexicon
  • Readable API (We will do our best to strike a balance between flexibility and readability)
  • Best Documentations - This includes best practices, use cases, samples, and possible tutorials.

So let your thoughts on this, especially talk about the problems you had with him, the more details the better. To help you clear your mind, start by talking about:

  • Pains with ClayCharts
  • Expectations
  • Use cases
  • Tell us what your team is planning for the future about charts.

Spend a little time to devote a few words of what you have in mind that can help us build something better and make better decisions about it, please share this issue with everyone who worked with ClayCharts or mark them on here.

@matuzalemsteles
Copy link
Member Author

@matuzalemsteles matuzalemsteles commented Oct 3, 2019

I'm tagging some people that I know worked with the charts. Please leave your thoughts: @interaminense @fcodias @FabioDiegoMastrorilli @oliveiraaraujo @phcp @kienD

@julien @jbalsas @bryceosterhaus please mark other people I may have forgotten and also leave your thoughts 🙂.

@jbalsas
Copy link
Contributor

@jbalsas jbalsas commented Oct 3, 2019

@FabioDiegoMastrorilli
Copy link
Member

@FabioDiegoMastrorilli FabioDiegoMastrorilli commented Oct 3, 2019

/cc @gcmznt

@interaminense
Copy link
Member

@interaminense interaminense commented Oct 3, 2019

@interaminense
Copy link
Member

@interaminense interaminense commented Oct 3, 2019

Do you guys think to change the chart lib? Are there other libs that I think are better than Billboard.js

Until today we face some issues with billboard.js that I can list here:

  • Performance issue when we insert a large amount of data in a line chart for example;
  • There is a bug when manipulating the axis values ​​(we don't allow the chart to control) that when updating the data, the axis isn't updated; So to solve the problem, we have to destroy the chart and build again, causing performance issues;
  • If we want to override the default chart tooltip, we have to convert the tooltip (component) to a string and moving to the chart;
  • We have an Analytics list that can display more than 10 charts on the screen and these 10 charts can have up to 90 points on each chart, and all these charts are customized using the billboard API, this is causing repaint and reflow on screen issues.

These are points that I could remember so far. I want to make it clear that all the design requirements that are passed for us to implement, we were able to implement using the API and taking questions using the Billboard.js documentation. The problem we face most is the performance issue. I can detail every topic I mentioned above if you need it.

My suggestion was to use some lib or create a new chart lib using React because that way we could solve these performance issues. Billboard.js is bad in that sense because it directly handles the DOM. I've been researching some libs that do this and I liked these:
https://github.com/recharts/recharts
https://uber.github.io/react-vis/

@diegonvs
Copy link
Member

@diegonvs diegonvs commented Oct 4, 2019

Thanks for the contribution @interaminense :)

Just for adding a little thing that I think you may miss: Some days ago, we were searching for a good library with a high customization power and we found Victory. This chart library is based on React for building good charts in a very flexible way.

This requires some study but I think We can create a theme based on Clay for this Chart library or deliver some high-levels based on this library.

@bryceosterhaus
Copy link
Member

@bryceosterhaus bryceosterhaus commented Oct 4, 2019

I spent some time going through Analytics use cases for charts and talking with @kienD. Overall, I think our currently implementation of Charts works for some generic use cases but provides a lot of headache and often can feel rigid with the billboard base layer. With an increasing demand of interactivity and visualization, I think it becomes evident that we might want a more low-level and composable approach to charts. If try to create that ourselves with Billboard, our overhead and complexity for maintaining will also grow significantly.

I spent some time looking through recharts and react-vis and I really like their approach to composing the chart together. I think we could leverage one of these existing libraries and instead of providing OOTB high-level charts, we could offer configuration and theming specific for the lexicon use case. This gives users flexibility to use the charts however they want, and it also gives us the ability to pre-define some of the look and feel to standardize with our designs.

An example of how a line-graph might look would be something like this where the ClayCharts* components would be wrapped recharts components with a lexicon specific design. (using recharts for example, not necessarily saying its the library to choose.)

import {LineChart, XAxis, YAxis, CartesianGrid} from 'recharts';
import {ClayChartsTooltip, ClayChartsLegend, ClayChartsLine} from '@clayui/recharts';

<LineChart data={data}>
	<CartesianGrid strokeDasharray="3 3" />
	<XAxis dataKey="name" />
	<YAxis />
	<ClayChartsTooltip />
	<ClayChartsLegend />
	<ClayChartsLine dataKey="pv" />
	<ClayChartsLine dataKey="uv" />
</LineChart>

IMO this sort of direction seems like both a win for clay users and clay maintainers since the focus is on the lexicon design, and not the interactivity of the chart itself. This also gives a lot of freedom for users to create unique charts that may not be exactly like the demos we currently have for clay charts.

@kienD
Copy link
Member

@kienD kienD commented Oct 4, 2019

I spent some time going through Analytics use cases for charts and talking with @kienD. Overall, I think our currently implementation of Charts works for some generic use cases but provides a lot of headache and often can feel rigid with the billboard base layer. With an increasing demand of interactivity and visualization, I think it becomes evident that we might want a more low-level and composable approach to charts. If try to create that ourselves with Billboard, our overhead and complexity for maintaining will also grow significantly.

I spent some time looking through recharts and react-vis and I really like their approach to composing the chart together. I think we could leverage one of these existing libraries and instead of providing OOTB high-level charts, we could offer configuration and theming specific for the lexicon use case. This gives users flexibility to use the charts however they want, and it also gives us the ability to pre-define some of the look and feel to standardize with our designs.

An example of how a line-graph might look would be something like this where the ClayCharts* components would be wrapped recharts components with a lexicon specific design. (using recharts for example, not necessarily saying its the library to choose.)

import {LineChart, XAxis, YAxis, CartesianGrid} from 'recharts';
import {ClayChartsTooltip, ClayChartsLegend, ClayChartsLine} from '@clayui/recharts';

<LineChart data={data}>
	<CartesianGrid strokeDasharray="3 3" />
	<XAxis dataKey="name" />
	<YAxis />
	<ClayChartsTooltip />
	<ClayChartsLegend />
	<ClayChartsLine dataKey="pv" />
	<ClayChartsLine dataKey="uv" />
</LineChart>

IMO this sort of direction seems like both a win for clay users and clay maintainers since the focus is on the lexicon design, and not the interactivity of the chart itself. This also gives a lot of freedom for users to create unique charts that may not be exactly like the demos we currently have for clay charts.

I think @bryceosterhaus summed up the results of our conversation very well. 🔥 💯

Customizing clay-charts to meet the needs of AC has been a painful process for us. I think the rigidness of billboardjs and the fact that it wasn't built to be used in react has a lot to do with the issues we have had developing with it.

If we use a composable charting library as a base for clay-charts and then themed it, we would be able to provide the flexibility that clay-charts consumers need while minimizing the excess work that the clay-charts maintainers would be doing.

clay-charts consumers: 👍
clay-charts maintainers: 👍

@erickakoyama
Copy link

@erickakoyama erickakoyama commented Oct 4, 2019

Just to echo what everyone else is mentioning, I think it would be great to move to some other base library that is built in React.

Coming from using a composable library recharts, it was difficult to work with Billboard because it doesn't seem to fit the SPA pattern. That was apparent with the dynamic chart transformations we've done where we need some part of the chart config like axes, type, or overall sizing to change gracefully.

It would also be great to have more flexibility, we had a situation where we tried to make an x-axis rotated histogram chart, and it was at best, ...hacky. This was in part because Billboard does not support histograms and also because it was tricky to get the chart to resize properly when adding/deleting histogram bars.

@julien
Copy link
Member

@julien julien commented Oct 7, 2019

I also agree with everyone that now that we're using React; we should probably use a library that integrates better with React, after all when we decided to choose Billboard.js we were using Metal.js+Soy and we also probably didn't know all the features we needed to implement. That said, it was a very good experience, because the team maintaining Billboard.js made it very easy to contribute to their codebase by being open minded and friendly. When a feature didn't exist in Billboard.js, it was mostly a matter of submitting the feature and discussing it with the developers, and if they agreed it was quickly available.

Concerning performance issues, this is something we should talk about, because maybe choosing another library isn't going to improve that: this is just a guess, maybe someone has already got data or a proof of concept that can demonstrate that changing the "chart" library does improve the performance (or that it actually doesn't).

Last time I checked d3.js does manipulate the DOM, so unless the React layer does something special about that, I doubt there will be a massive boost in terms of performance. Maybe in some cases for huge datasets it might be better to use a GPU (i.e. WebGL) focused library: some of them actually also integrate well with d3.js. I think it would be interesting to compare both the d3.s+React approach and the d3.js+WebGL approach when performance is the main issue.

@leeoniya
Copy link

@leeoniya leeoniya commented Oct 24, 2019

if you want performance, you'll want to go with a canvas-based lib, which i think will limit your styling/theming flexibility vs svg. i'm fairly certain that my time-series lib will not work for you, but you can compare some libs in its perf section.

https://github.com/leeoniya/uPlot

@bryceosterhaus
Copy link
Member

@bryceosterhaus bryceosterhaus commented Dec 9, 2019

Hey all, I went through and re-created some of the clay chart demos with recharts. Please let me know what you think and help give feedback on how you would like to see the "clay" side of things exposed.

PR: #2779
Storybook: https://deploy-preview-2779--storybook-clayui.netlify.com/?path=/story/components-clayrecharts--line

Currently I am thinking our best option for exposing would be creating a clay-recharts package that would expose custom components to be used with recharts as well as colors. This package would then have a peer-dependency on recharts. Meaning that in your projects you would both have dependencies on recharts and @clayui/recharts.

All feedback is appreciated.

@diegonvs
Copy link
Member

@diegonvs diegonvs commented Dec 10, 2019

Just for information, If we need to make comparisons about charts library, maybe this is a good source: https://stackshare.io/recharts

On stackshare we can check all pro and cons from users of these chart libraries :)

@julien
Copy link
Member

@julien julien commented Dec 11, 2019

@diegonvs I think we're going with recharts for now, at least that's what I understood.

@bryceosterhaus
Copy link
Member

@bryceosterhaus bryceosterhaus commented Dec 11, 2019

I'm going to spend some time playing with some other libraries today to see if there are any major tradeoffs. Although I did find recharts really great while using it. Typically I loathe creating any charts, but I found myself looking forward to each use case in recharts since it was so ergonomic.

@bryceosterhaus
Copy link
Member

@bryceosterhaus bryceosterhaus commented Dec 12, 2019

cc @drakonux could you give a bit of feedback on these chart demos? Do these clash with lexicon at all? There are some interactions that do not work out of the box like they did with billboard. Demos

@drakonux
Copy link
Member

@drakonux drakonux commented Dec 13, 2019

Hey @bryceosterhaus I admit that I'm a bit lost with the work done by Emiliano when he was working on charts. So if you don't mind I would like to wait him before commenting something (he is on PTO and he will be back next week)

@drakonux
Copy link
Member

@drakonux drakonux commented Dec 16, 2019

Hi everyone!

Well, I 've already talked to Emiliano and @julien . From the Lexicon point of view, Clay should have at least the same features, interactions and charts Clay v2 provided.

Besides, Clay v3 should pay attention to some of the charts that we made a special effort for v2 like:

  • Predictive Forecast (for Commerce)
  • Heat map (for AC)

And keep the accessibility with the highest standards to be used across the company (area patterns, lines and shapes, etc.).

That's what we think. We will provide you more support and documentation if it's needed, just request us.

About the future of Clay with data visualization, we should not work if we don't receive a team request. For instance, although we didn't publish it, we already have a visual definition for a Map chart visualization. But without a formal request, we won't publish it. It's up to you to request it to us (cc\ @jbalsas )

@bryceosterhaus
Copy link
Member

@bryceosterhaus bryceosterhaus commented Dec 17, 2019

Thanks for the feedback @drakonux!

Both the predictive forecast and heat map charts should be doable. Heat charts come by default with recharts and I need to work on forecasting a bit more.

The one area of OOTB(out of the box) features we may not be able to provide would be interactions such as clicking on a certain data set and bluring the other data sets(this was OOTB with billboard). This functionality is really easy to implement with recharts, but just isn't necessarily something we can provide OOTB. recharts interactions are pretty vanilla OOTB but I actually think that is helpful since users typically want to control that stuff themselves. Will this be a major issue?

Edit: recharts does not support heatmap. Billboard also doesnt support heatmap though, so I'm not sure where AC is getting that from.

@drakonux
Copy link
Member

@drakonux drakonux commented Dec 23, 2019

From our point of view, we would like to have those interactions however if the cost/time is high you can go with the basic interactions and invest time further in the future on the most complex ones cc\ @jbalsas

@bryceosterhaus
Copy link
Member

@bryceosterhaus bryceosterhaus commented Dec 23, 2019

Performance issue when we insert a large amount of data in a line chart for example;

Hey @interaminense, do you have an example of performance issues? I'd be curious to get the set of data you are working with and demoing it with recharts. Can you slack me an example of the data that is causing issues?

@jbalsas
Copy link
Contributor

@jbalsas jbalsas commented Jan 20, 2020

I think we can close this as work is ongoing and most feedback was gathered/accounted for.

@jbalsas jbalsas closed this Jan 20, 2020
@jbalsas jbalsas unpinned this issue Jan 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet