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

Convert charts to D3 #4568

Closed
mariusandra opened this issue Jun 2, 2021 · 8 comments
Closed

Convert charts to D3 #4568

mariusandra opened this issue Jun 2, 2021 · 8 comments
Labels
enhancement New feature or request feature/lifecycle Feature Tag: Lifecycle feature/paths Feature Tag: Paths feature/trends Feature Tag: Trends stale

Comments

@mariusandra
Copy link
Collaborator

mariusandra commented Jun 2, 2021

Is your feature request related to a problem?

Our charts are a mess, both in features (hard to add new functionality like gradients) and code (mostly old untyped non-React JS via 3+ different libraries for charts, funnels, piecharts, paths, etc).

Describe the solution you'd like

Instead of wasting too much time cramming square pegs into round holes, let's fix the problem at the source. Having worked with dozens of charting libraries over many many years, both in React and not, the only real solution is to use D3 directly. Everything else is a compromise, including D3 wrappers for React.

Describe alternatives you've considered

Copy/pasting linecharts from stack overflow when we need a new one :).

Additional context

@samwinslow has expressed interest in working on this.

I'd have no problem approving (or pushing for approval for) any training material (such as this) if it helps save days off development time.

Perhaps pie charts are a good target to start from.

Thank you for your feature request – we love each and every one!

@mariusandra mariusandra added the enhancement New feature or request label Jun 2, 2021
This was referenced Jun 3, 2021
@samwinslow samwinslow added core-experience P1 Urgent, non-breaking (no crash but low usability) labels Jun 4, 2021
@mariusandra
Copy link
Collaborator Author

Here's the most approachable D3 tutorial I've found lately: https://dev.to/codesphere/creating-data-visualizations-with-d3-and-reactjs-10ei

I'm unfortunately not intimately familiar with D3 myself, so I don't have context on how to build this interface the best way, without everything turning into some spaghetti code :). I guess the best way forward is to just do something with one of our worst graphs (funnels or piecharts?) and take it from there.

The line charts are actually looking pretty good after the latest experiments, so they should take a backseat.

Regarding how to wire these things together, I'm not sure. Since a chart in the end is a fully visual object (e.g. part of the view/template), it makes sense to render it in a react component, probably via useEffect as shown in the link above. That said, depending on what we need the chart to do, it might benefit from being controlled by some actions/reducers/selectors 🤷 . However it's probably best to just start with... anything :).

@marcushyett-ph
Copy link
Contributor

+1 to "let's fix the problem at the source", this will pay off a lot in the long term - and probably even the short term.

@paolodamico paolodamico removed the P1 Urgent, non-breaking (no crash but low usability) label Jul 27, 2021
@paolodamico paolodamico added feature/trends Feature Tag: Trends and removed team-core-experience labels Feb 22, 2022
@paolodamico
Copy link
Contributor

@mariusandra is this still a thing?

@clarkus
Copy link
Contributor

clarkus commented Feb 23, 2022

I would be in favor of moving to D3. I've worked with that framework quite a bit and know that it would work well with the visualizations we're supporting today.

@mariusandra
Copy link
Collaborator Author

mariusandra commented Feb 24, 2022

@paolodamico kind of. This can remain open though.

Some newer charts (e.g. funnel time-to-convert) are already in D3. Some other new charts (e.g. the funnel boxes) are made with plain HTML elements. Some older charts still use the chartjs line chart, and it will make sense to move them to D3 at some point, but I'd say we update them when we decide to improve them somehow (e.g. adding area charts), not pre-emptively to just change the tech.

@clarkus
Copy link
Contributor

clarkus commented Feb 24, 2022

We're going to be updating trends soon. It has the most chart visualization options so it would cover a lot of surface area if we choose to include D3 in scope there.

@posthog-bot
Copy link
Contributor

This issue hasn't seen activity in two years! If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in two weeks.

@posthog-bot
Copy link
Contributor

This issue was closed due to lack of activity. Feel free to reopen if it's still relevant.

@posthog-bot posthog-bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature/lifecycle Feature Tag: Lifecycle feature/paths Feature Tag: Paths feature/trends Feature Tag: Trends stale
Projects
None yet
Development

No branches or pull requests

6 participants