hrbrmstr / metricsgraphics

:chart_with_upwards_trend: htmlwidget interface to the MetricsGraphics.js D3 chart library
http://hrbrmstr.github.io/metricsgraphics/
Other
132 stars 35 forks source link

Allow to draw grid charts when multiple series of data are given #13

Closed yutannihilation closed 9 years ago

yutannihilation commented 9 years ago

You know, MetricsGraphics.js has a nice feature, linked charts. I found this may be useful to implement ggplot2's facet_grid-like chart.

An example is here: http://rpubs.com/yutannihilation/metricsgraphics_grid

pssguy commented 9 years ago

Nice addition

hrbrmstr commented 9 years ago

very nice! let me poke at it a bit, but i should be able to accept the PR w/o changes (given that this is a very new package and subject to change).

hrbrmstr commented 9 years ago

Additional note, the current implementation I have does support linked charts - http://rpubs.com/hrbrmstr/52765 - but i don't think this changes that (folks may want separately linked charts for some reason)

timelyportfolio commented 9 years ago

Just installed and tested the fancy new grid functionality. Love the concept, but doesn't quite work for me. If you look closely, the dot follower does not track the line in both RStudio Viewer and Chrome. I'll research further.

image

timelyportfolio commented 9 years ago

Might be a namespace-type issue. Consider this other flaw.

library(metricsgraphics)
library(htmltools)

set.seed(1492)
 stocks <- data.frame(
   time = as.Date('2009-01-01') + 0:9,
   X = rnorm(10, 0, 1),
   Y = rnorm(10, 0, 2),
   Z = rnorm(10, 0, 4))

 stocks %>%
   mjs_plot(x=time, y=X) %>%
   mjs_line() %>%
   mjs_add_line(Y) %>%
   mjs_add_line(Z) %>%
   mjs_axis_x(xax_format="date") %>%
   mjs_add_legend(legend=c("X", "Y", "Z")) %>%
   mjs_grid() -> s1

stocks %>%
  mjs_plot(x=time, y=X) %>%
  mjs_line() %>%
  mjs_add_line(Y) %>%
  mjs_add_line(Z) %>%
  mjs_axis_x(xax_format="date") %>%
  mjs_add_legend(legend=c("X", "Y", "Z")) %>%
  mjs_grid() -> s2

tagList(s1,s2) %>% html_print
hrbrmstr commented 9 years ago

that's definitely an interesting issue (but, also kinda weird since there should be unique div's for each).

Also, I need to check with the metricsgraphics docs, but I don't think they ever expected to have any more than 2 charts linked at the same time. (I just realized that)

On Mon, Jan 12, 2015 at 10:55 AM, timelyportfolio notifications@github.com wrote:

library(metricsgraphics) library(htmltools) set.seed(1492) stocks <- data.frame( time = as.Date('2009-01-01') + 0:9, X = rnorm(10, 0, 1), Y = rnorm(10, 0, 2), Z = rnorm(10, 0, 4)) stocks %>% mjs_plot(x=time, y=X) %>% mjs_line() %>% mjs_add_line(Y) %>% mjs_add_line(Z) %>% mjs_axis_x(xax_format="date") %>% mjs_add_legend(legend=c("X", "Y", "Z")) %>% mjs_grid() -> s1 stocks %>% mjs_plot(x=time, y=X) %>% mjs_line() %>% mjs_add_line(Y) %>% mjs_add_line(Z) %>% mjs_axis_x(xax_format="date") %>% mjs_add_legend(legend=c("X", "Y", "Z")) %>% mjs_grid() -> s2 tagList(s1,s2) %>% html_print

timelyportfolio commented 9 years ago

Issue for the tagList error is id without namespace can only be used once in a page, so at line 189 duplication is not guaranteed but certainly likely with other metricsgraphics or other items on the page.

image

I'm sure you have noticed that htmlwidgets assigns a semi-random id to avoid conflict. This could be a strategy, or using class and then setting target to el.id + ' .' + y_accessor might be a strategy.

timelyportfolio commented 9 years ago

Even with just X and Y drawn I get the tooltip dot in the wrong place.

image

I am thinking that the Y scale is getting applied to X to determine tooltip vertical position.

timelyportfolio commented 9 years ago

As this gets resolved, I would suggest isolating in a branch and reverting master back.

hrbrmstr commented 9 years ago

aye. agreed. just did it.

On Mon, Jan 12, 2015 at 11:15 AM, timelyportfolio notifications@github.com wrote:

As this gets resolved, I would suggest isolating in a branch and reverting master back.

— Reply to this email directly or view it on GitHub https://github.com/hrbrmstr/metricsgraphics/pull/13#issuecomment-69594981 .

yutannihilation commented 9 years ago

Thank you for the discussion, and sorry that I didn't notice this bug... I will resolve this and send PR again.

yutannihilation commented 9 years ago

I got it!

The problem was that I reused mg_params for each chart, so that the side-effects stormed the other charts drawn before. (That's why only the chart drawn last worked fine.) Sorry for my poor Javascript skill...

The fix commit: https://github.com/yutannihilation/metricsgraphics/commit/a2266ed8a37d9c6d26e1b72f3c5ad421b115c326 Proof: http://rpubs.com/yutannihilation/53408

While in official linked links two charts (thanks @hrbrmstr for confirming in mozilla/metrics-graphics#309), it seems working with more than two charts. But I'm not sure if we can stably use this feature.

yutannihilation commented 9 years ago

What I'm thinking now is whether I should use $.extend() or not, since MetricsGraphics.js is leaving from jquery (https://github.com/mozilla/metrics-graphics/pull/267).