vizdata-f21 / project-2-viz_biz

https://phillip.shinyapps.io/viz_biz/
0 stars 1 forks source link

Problem downloading Kandinsky plot #14

Closed li11ianc closed 3 years ago

li11ianc commented 3 years ago

@mine-cetinkaya-rundel

Here is a link to the downloadHandler for plotInput_kandinsky.

An error I've noticed appearing in my console when I run it reads "Saving 7 x 7 in image Warning: Error in plot_kandinsky: could not find function "plot_kandinsky" [No stack trace available]"

So I'm currently googling that to attempt to troubleshoot!

mine-cetinkaya-rundel commented 3 years ago

@li11ianc I think I identified the problem, though haven't solved it yet. I believe you shouldn't solve this with an eventReactive() inside an observeEvent(). Just the observeEvent() should be sufficient.

Also, if you wanted to simplify the app code to make debugging easier you can create a folder called R in the folder where your app lives (final-shiny) and inside this folder put an R script with all the custom functions you want loaded. It'll be loaded automatically when your app runs. People generally used the title utils.R for that file, but you can really call it whatever you like.

I'll come back to this a little later this evening if you haven't solved it yet by then, need to step away for a bit.

li11ianc commented 3 years ago

@mine-cetinkaya-rundel The eventReactive() was the solution I found to the issue with hitting the Apply Changes button 2+ times (where it only worked the first time). I can remove that and try something else if you think it's causing the issue!

Also, I moved the functions at the top of my app to utils.R in an R folder, but the ones for plotting that really clog up the server stopped working when I moved them -- I really have no idea why this happens. Thank you again so much for helping with this!

mine-cetinkaya-rundel commented 3 years ago

The second part especially is weird. I'm back at my computer so can take another look. Will you push your latest updates? And which file are you working on (the final-shiny or the one in the folder with your name)?

li11ianc commented 3 years ago

@mine-cetinkaya-rundel My latest updates are pushed! And I'm working in the final-shiny

li11ianc commented 3 years ago

@mine-cetinkaya-rundel I just realized somehow my functions were removed from the shiny on the most recent push. That was totally my error and I've fixed it and pushed it again just now. I'm really sorry!

mine-cetinkaya-rundel commented 3 years ago

Got it, no worries! I figured it was something like that, let me take another look.

mine-cetinkaya-rundel commented 3 years ago

I am getting a weird error when I run your app as is:

Problem with `mutate()` input `..1`.
ℹ `..1 = across(contains("x"), ~clip(.x + noise_x, xmin, xmax))`.
x object 'xmin' not found

Are xmin and xmax the ones defined at https://github.com/vizdata-f21/project-2-viz_biz/blob/c3623c1d50b6af92e46f2cdc94d28b60016ff72b/shiny/final-shiny/app.R#L65-L68?

li11ianc commented 3 years ago

Yes they are!

mine-cetinkaya-rundel commented 3 years ago

See #15, I've fixed a few things so far.

mine-cetinkaya-rundel commented 3 years ago

@li11ianc I know it's taking a while but I think I have a solution for the image involving a loading spinner screen so I'll put in one more PR shortly!

li11ianc commented 3 years ago

@mine-cetinkaya-rundel Thank you so much!! I appreciate you working so late on this.

li11ianc commented 3 years ago

The artwork loading time is also significantly faster when you change the color palette with the new option I added so it must be accessing all the specific hex codes in a list and resetting scale fill/scale color for each layer individually that's taking the most time

mine-cetinkaya-rundel commented 3 years ago

Oh that's good. I'll pull those changes and try adding this other approach for the observeEvent()/actionButton()

mine-cetinkaya-rundel commented 3 years ago

@li11ianc Hopefully #16 resolves the issue that started this whole thing 😄

li11ianc commented 3 years ago

@mine-cetinkaya-rundel It's so beautiful!! The loading bar is much more elegant, thank you thank you! Was it just getting rid of the observeEvent and adding ignoreNULL? (No need to respond actually but I'd love to ask after class tomorrow.) Thanks again.

mine-cetinkaya-rundel commented 3 years ago

Happy to chat tomorrow!

In short, it was three things:

  1. Use eventReactive() to create a reactive plot, with ignoreNULL to get it going upon launch
  2. Render the reactive plot and display it in the UI
  3. USe the same reactive plot to download

And I figured out why the functions ended up having to stay in app.R, happy to say more if you're curious. That is also fixable but wouldn't change the resulting look so I figured not required for now.

Glad you like it, well done on the app, to all of you!