thebioengineer / camcorder

Record plots generated during an R session and replay as a gif!
https://thebioengineer.github.io/camcorder/
Other
201 stars 7 forks source link

gt table support #54

Open yjunechoe opened 1 year ago

yjunechoe commented 1 year ago

Hey all,

I got nerd-sniped on masstodon by a {camcorder} user who wanted to see support for {gt} tables. This draft PR attempts to add that, with some examples you can view in _snaps/gt.

Now that I have a working prototype, I wanted to gauge whether this feature would be of interest 😃 (and possible get some more hands on board for testing).

A list of things to consider:

Known issues:

LMK what you think!

gkaramanis commented 1 year ago

Hi June!

This is so great! Did some quick testing and it works flawlessly.

I'd love to have support for {gt} tables in {camcorder}, let's see what @thebioengineer and @z3tt think 😄

One thing I'm wondering about is the dimensions in camcorder. Setting the height and width in this case doesn't really work as when saving plots (and it probably shouldn't, anyway). I guess it's a webshot thing, I remember it being tricky with dimensions when I used it before. What I'm trying to say is that, if we add {gt} support, we should add a warning about the dimensions?

yjunechoe commented 1 year ago

Thanks @gkaramanis!

You're totally right about the different behavior of height/width for gt tables - I hadn't thought it through that far so good catch 😅

As you say it's tricky and not a great way out of this, but perhaps we could doing something like:

1) Expose the vwidth/vheight that {webshot2} uses for viewport width/height. This is roughly the amount of room in the html doc that the table gets when it's rendered. This controls stuff like whether a long column gets wrapped, but is agnostic to the size of the table when converted to image. This is faithful to how width/height is understood in {webshot2} 2) The existing width/height argument should be reserved for image dimensions. {webshot2} doesn't control the dimensions of the screenshot, but we could add a post-processing step re-sizing the table image with {magick}, to make table dims match plot dims.

So then record_ggplot() for gt tables would roughly look something like:

function(x) {
  gt_html <- gt::gtsave(x)
  gt_screenshot <- webshot2::webshot(gt_html, vwidth, vheight, ...)
  gt_image <- magick::image_resize(gt_screenshot, width, height, ...)
  ...
}

And in gg_record(), you can set both width/height and (if recording gt) vwidth/vheight:

gg_record(
  ...,
  # shared options
  width,
  height,
  ...
  # gt-specific options
  vwidth,
  vheight,
  ...
)

Whatever we decide, I definitely agree that setting dimensions for tables in {camcorder} could use a warning or note!

I'll also play around with handling gt dimensions a bit more and report back what I find

thebioengineer commented 1 year ago

Hey @yjunechoe - this is a great idea, and I would be happy to expand the scope of camcorder to include tables. In theory this could expand to other html-based views, but we can sort that out later.

I would prefer to not add a lot of new arguments to gg_record specific to supporting gt tables, but if we could sort out another way. The height and width expected by webshot2 in pixels, so we can always do unit conversion from what is passed in into viewer height. I think the usual assumption is 96 pixels per inch .