Skip to content

Add chart export module#493

Merged
jwildfire merged 6 commits into
devfrom
chart-export
Apr 15, 2021
Merged

Add chart export module#493
jwildfire merged 6 commits into
devfrom
chart-export

Conversation

@bzkrouse

Copy link
Copy Markdown
Contributor

Re-implement chart export module #449.

Further discussion/to-dos:

  • Consider workflow for replicating charts with R code
    • start of some helpers are embedded but we should think about making this as user friendly as possible
  • Modify/extract helper functions to /R directory
  • Reuse helpers in chart creation modules


# Function to create chart
create_chart <- function(chart, params){
if (chart$type=="htmlwidget"){

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty sure we need to add the init function (if any) in to the workflow here. I'll see what I can do in the next few days.

@jwildfire

jwildfire commented Apr 14, 2021

Copy link
Copy Markdown
Contributor

Widgets aren't rendering for me in the reports - I suspect it's because the init functions aren't running as expected. Going to try setting it up so that params are defined using the exact same process as here.

@jwildfire

jwildfire commented Apr 15, 2021

Copy link
Copy Markdown
Contributor

Pushed a tweak that helps deal with the init, but there are still a few issues:

  • hep-explorer and shiftplot aren't working because they use x.ns (instead of the native el) from the widget settings to identify the div to draw the chart. Probably need to update the renderers, but I'm pretty sure this was a hack to deal with rendering issues in the app. Filed Don't use namespace when rendering widgets safetyCharts#49.
  • the shifttable with Kable is bombing both here and in the app. Leaning towards deleting. Filed tpylr shift table isn't rendering safetyCharts#50
  • ae-timeline bombs when aeexplorer is present, but renderers fine otherwise. Guessing it just wants to be drawn first. Errors in js, make it seem like the linked table isn't rendering properly ...

Everything is also a bit slow, so probably need some kind of load bar.

@jwildfire

Copy link
Copy Markdown
Contributor

Merging - thanks for setting this up @bzkrouse!

Filed #518, #519 and #520 in this repo and SafetyGraphics/safetyCharts#49 and SafetyGraphics/safetyCharts#50 in safetyCharts to deal with down the road.

@jwildfire jwildfire self-requested a review April 15, 2021 16:13

@jwildfire jwildfire left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@jwildfire jwildfire merged commit 63bee7e into dev Apr 15, 2021
@jwildfire jwildfire linked an issue Apr 19, 2021 that may be closed by this pull request
@jwildfire jwildfire deleted the chart-export branch April 22, 2021 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reimplement Chart Export in V2

2 participants