Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#2431 closed New feature (fixed)

JavaScript Charts

Reported by: EZdesign Owned by:
Priority: critical Milestone: 1.5 - Piwik 1.5
Component: UI - UX (AngularJS, twig, less) Keywords:
Cc: Sensitive: no

Description

Provide charts (pie, bar and evolution) rendered by JavaScript rather than Flash.

Suggested library: jqplot (http://www.jqplot.com)

  • Open source and free
  • Updated frequently
  • Highly customizable

Open questions

  • Do we add JS charts in addition to or instead of Flash?

Attachments (2)

jqplot-src.zip (1.5 MB) - added by EZdesign 3 years ago.
diff.zip (5.0 KB) - added by EZdesign 3 years ago.

Download all attachments as: .zip

Change History (37)

comment:1 Changed 3 years ago by EZdesign (BeezyT)

Here's a first proof of concept implementation.

In your config.ini, add

[General]
use_js_charts = 1

Apply the diff and place the contents of jqplot-src.zip in libs/jqplot/src/

Changed 3 years ago by EZdesign (BeezyT)

Changed 3 years ago by EZdesign (BeezyT)

comment:2 Changed 3 years ago by EZdesign (BeezyT)

Sorry for adding the diff as a zip, trac wouldn't accept it otherwise

comment:3 Changed 3 years ago by vipsoft (robocoder)

No worries. It's the spam filter at work.

Looking at the patch:

  • libs/jqplot.{css, js} - should probably be in plugins/CoreHome/templates/
  • libs/jqplot/src/* - could be moved up one level to libs/jqplot/

Otherwise I don't see a blocker for commiting to trunk.

comment:4 Changed 3 years ago by matt (mattab)

quick review

  • great work, long due change to JS graphs that will make a lot of users happy!!
  • please remove examples/, docs/ directory (and all other files not required by Piwik to keep size minimal and other risks associated with unused files)
  • also remove jquery etc. as they are already packaged in piwik
  • I vote for removing OFC from trunk as long as all features are supported. Better keep code simpler. Also, 1.5 will have probably a long RC cycle to iron out all new features, so we will have many testers and will find most bugs if there are any.
  • have you tried if ExampleUI plugin still work fine (it demonstrates how to use graphs in a plugin, to plot external data in a plugin) ? It would be great if the API was still working the same way. There is for example a graph that plots several lines which isn't used in Piwik core, but is used in other plugins (SiteSearch for example I think ;)

comment:5 Changed 3 years ago by vipsoft (robocoder)

  • Milestone set to 1.5 - Piwik 1.5

Oh, I see "Export as Image" isn't implemented yet.

comment:6 Changed 3 years ago by matt (mattab)

Currently, if you click on a sparkline, it will refresh the Evolution graph at the top of the page with the metric clicked on. Is this also working with JS charts?

I'm thinking that it will be easier also to add a list of metrics on the right of the graph, to select which metric to plot (#1820) once we use JS and not flash :)

comment:7 Changed 3 years ago by EZdesign (BeezyT)

Thanks for pointing that out, matt.

I have thought of that and in the current version on my local machine (which I will commit soon), clicking the sparklines works. It would be easy to implement a jquery event that can be triggered on the container and receives a data url which will be plotted. That could be used in a select element...

comment:8 Changed 3 years ago by matt (mattab)

Cool! Instead of a SELECT, I was thinking more of a showing the list of all metrics a bit like google adsense graphs: http://2.bp.blogspot.com/_YbURk67VlGk/TUdM5dkPLVI/AAAAAAAABWU/sDd80kcjJWA/s1600/%25233Graphs_EN.PNG

We could use a Radio button instead of checkbox to keep things simple (click on checkbox would refresh the line)

comment:9 Changed 3 years ago by EZdesign (BeezyT)

(In [4724]) refs #2431 Basic JavaScript Chart support using jqplot

comment:10 Changed 3 years ago by EZdesign (BeezyT)

(In [4725]) refs #2431 removed loading JS on the fly

comment:11 Changed 3 years ago by EZdesign (BeezyT)

(In [4734]) refs #2431 LEGALNOTICE, Export as Image translated, removed some unneeded JS

comment:12 Changed 3 years ago by EZdesign (BeezyT)

(In [4789]) refs #2431 new legend renderers: nicer for line chart, available for bar chart, inside pie chart. also fixes image export problems.

comment:13 Changed 3 years ago by EZdesign (BeezyT)

(In [4790]) refs #2431 show loading feedback when clicking on sparklines

comment:14 Changed 3 years ago by EZdesign (BeezyT)

(In [4800]) refs #2431 memory leak fixes (temporarily removed jqplot due to problems with svn)

comment:15 Changed 3 years ago by EZdesign (BeezyT)

(In [4801]) refs #2431 added jqplot again

comment:16 Changed 3 years ago by matt (mattab)

(In [4802]) Refs #2431 - Alerting "please upgrade your browser" when click on export as image not supported

comment:17 Changed 3 years ago by matt (mattab)

OK great improvements... I looked at our earlier discussions and all is now implemented and looking great!! :)
I think the last UI "regression" is:

  • When the evolution chart is clickable (when period != range currently), it would be usable to have the cursor be of type pointer to show that it's clickable
  • Pie chart 100% appears as a square filling all the graph

I still have a bit of feedback on leaks (sorry!):

  • Much less memory leaks, but still quite present. After usink piwik for 5 minutes, loading around 10 reports, I see 100M memory leak in FF
  • Tooltips are leaking, if you hover on evolution chart the memory grows and is not freed. In firefox, it leaks at about 1M per second when constantly hovering on evolution chart. The same happens for Pie charts and bar graphs.
  • In IE, if you hover from left to right on evolution chart, the browser becomes quite slow and uses 100% of proc, so I assume the memory and maybe events binding are not reset for tooltips? NOTE: I'm using IE8 which maybe is using excanvas (but I think it might be related to previous point about leaking tooltips anyway)
  • Maybe some of the memory logic is browser dependant... Chrome appears to be using a LOT less memory and leaking nearly none (appart from tooltips)?

and I think the last task is...

  • Remove OFC library from the trunk, remove all OFC custom PHP code and use only jqplot as graph library.

comment:18 Changed 3 years ago by matt (mattab)

  • Priority changed from normal to critical

comment:19 Changed 3 years ago by EZdesign (BeezyT)

About the tooltip leaks: I did lots of tests and came to the conlusion, that there are no major leaks we can fix anymore. The problem is much worse if Firebug is enable, so make sure it's not.

Here's some more info:

  • Firefox and Safari handle the charts the same way. Safari does not leak at all.
  • Other graph libs have the same problem in Firefox.
  • Firefox uses the most memory out of all browsers. It doesn't handle canvases well. This will definately be improved in future versions of Firefox.
  • By clicking around a lot you can up the mem usage about 50 to 100MB compared to when you first open the dashboard. Some of that memory will be freed after some time. That is a lot, but I don't see how we can influence it. Btw, in Safari it's 15MB.


Before the leak fixes, you could up the mem usage as much as you wanted, now it at least has reasonable upper bounds.

About IE: excanvas uses a lot of CPU, but I can't influence that either. It is very memory efficient though, because the canvases can be properly destroyed. If it's slow in IE8, it might show people how crappy that browser is and maybe they update to IE9, which works perfectly fine (and has a way better canvas imlementation than Firefox).

Over the course of the jqplot integration I realized that the canvas technology is still at it's very beginning. Maybe we are a few months ahead of our time, but I think it's a good thing to propagate new technology and maybe help users see how outdated their browsers are. There are modern browsers, why not use them?

If there is a good reason to continue using old browsers, everything works fine, it might just be a little slow (which Flash was as well).

There are some more things on my to do list:

  • Export as image has to disabled for IE7 & 8, becuase excanvas does not support that. Later, we can replace that with the ImageGraph library.
  • There is a little bug on the overview pages with sparklines: If you open an empty report, you can't switch anymore.
  • Excanvas should be included conditionally. Is there a way to do that? (Conditional comments don't work when merging JS)

comment:20 Changed 3 years ago by vipsoft (robocoder)

re tooltip/hover leak. Read some posts that suggest using a timer to blur focus of the canvas element periodically (eg every 200 ms) to trigger Firefox's garbage collector.

comment:21 Changed 3 years ago by vipsoft (robocoder)

re: conditional include, see plugins/CoreHome/templates/header.tpl

comment:22 Changed 3 years ago by EZdesign (BeezyT)

re tooltip/hover leak: Can you share the posts?

I doubt the problem are still the canvases. They are properly reused (which is what my fix is all about).

For testing I disabled the tooltip and the color changes. The memory still grows. I think, it's just the enourmouse amounts of mousemove events...

comment:23 Changed 3 years ago by EZdesign (BeezyT)

To illustrate how crappy Firefoxes memory management is: Try refreshing the page a couple of times...

comment:24 Changed 3 years ago by EZdesign (BeezyT)

(In [4807]) refs #2431 excanvas is only loaded in old internet explorers. also minified excanvas.

comment:25 Changed 3 years ago by EZdesign (BeezyT)

(In [4808]) refs #2431 removed unneeded jqplot files. reduced jqplot css size. cursor pointer for clickable evolution charts. small sparkline bugfix.

comment:26 Changed 3 years ago by EZdesign (BeezyT)

The pie chart bug has been reported to Chris. He will fix it.

Apart from that, everything should finally be done now!

As I said, there still are memory problems in Firefox which I think we won't be able to fix. Are we comfortable with removing OFC from trunk at this stage or should we keep it until Firefox improves its canvas implementation (or we realize that it is our fault after all)?

comment:27 Changed 3 years ago by matt (mattab)

Export as image has to disabled for IE7 & 8, becuase excanvas does not support that. Later, we can replace that with the ImageGraph library.

We could, but really low priority. IE7/8 can die ;)

There is a little bug on the overview pages with sparklines: If you open an empty report, you can't switch anymore.

OK not a problem

Are we comfortable with removing OFC from trunk at this stage or should we keep it until Firefox improves its canvas implementation (or we realize that it is our fault after all)?

Yes I think now, considering how well this is behaving in Chrome and Safari, the issue really is with firefox.

Please, you can remove all OFC code js/tpl/swf/php code from the trunk :)

wow this is so nice that you did push the feature up to completion to this quality standards!

comment:28 Changed 3 years ago by EZdesign (BeezyT)

  • Doing our best to speed up the "death" of IE7 & 8 sounds good for me ;-)
  • The bug concerning sparklines in overview pages has been fixed in [4808]
  • I'll start removing OFC. yay!

comment:29 Changed 3 years ago by EZdesign (BeezyT)

(In [4813]) refs #2431 ofc removed. perfomance improvements. request size optimizations. general refactorings.

comment:30 Changed 3 years ago by matt (mattab)

(In [4814]) Refs #2431 removing "flash export" on Widgets Screen, which feels good!

comment:31 Changed 3 years ago by matt (mattab)

Ticket now fixed, just waiting for jqplot to fix the 100% pie bug but on the piwik, 100% complete and working.

Timo, amazing work from start to finish of this project! Kuddos...

comment:32 Changed 3 years ago by matt (mattab)

  • Resolution set to fixed
  • Status changed from new to closed

comment:33 Changed 3 years ago by matt (mattab)

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:34 Changed 3 years ago by matt (mattab)

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [4882]) Fixes #2431

comment:35 Changed 3 years ago by EZdesign (BeezyT)

(In [4898]) Refs #2431 jqplot update

(sorry, forgot Refs in my original commit message)

Note: See TracTickets for help on using tickets.