Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert examples to single file apps #1685

Merged
merged 9 commits into from Jul 11, 2017
Merged

Convert examples to single file apps #1685

merged 9 commits into from Jul 11, 2017

Conversation

mine-cetinkaya-rundel
Copy link
Contributor

@mine-cetinkaya-rundel mine-cetinkaya-rundel commented Apr 28, 2017

  • Convert all example apps to single file app.R file
  • Make relevant updates to READMEs to match up with single file app structure
  • Add color to plots (RStudio blue)
  • Add extensive comments to app.R files and use consistent formatting of comments across examples
  • Example specific edits:
    • In 04_mpg: Show outliers by default, as opposed to hide, since this is more routine, use pch = 19 for outlier so it's more easily visible
    • In 06_tabsets and 08_html: Don't name random data vector data (name d instead)
    • In 08_html: Add headers to clarify what each output is and show only head of data to simplify app
    • In 09_upload: Use req() to check for NULL entry, update comments to reflect single file upload, show only head of uploaded file by default, with an option to view all if requested
    • In 10_download: row.names = FALSE in write.csv()
  • Added entry to NEWS.md briefly summarizing changes.

- Make relevant updates to Readmes to match up with app.R structure
- Add color to plots (RStudio blue)
- In 04_mpg example: Show outliers by default, as opposed to hide, since this is more routine
- In 06_tabsets and 08_html examples: Don't name random data vector "data"
- Add extensive comments to app.R files and use consistent formatting of comments across examples
- In 09_upload example: Use req() to check for NULL entry
@bborgesr
Copy link
Contributor

bborgesr commented May 1, 2017

@mine-cetinkaya-rundel, I skimmed through this and it looks great; I trust that you were able to run each example with no problems, right?

@wch, @jcheng5, anything holding this up or can we merge?

@mine-cetinkaya-rundel
Copy link
Contributor Author

Thanks, and yes I was able to run each one.

@mine-cetinkaya-rundel
Copy link
Contributor Author

Echoing @bborgesr's question from earlier: @wch, @jcheng5, anything holding this up?

I have a few updates to tutorial/articles that depend on the final code that goes in the package, so I wanted to make sure I have this bit reviewed first.

@jcheng5
Copy link
Member

jcheng5 commented Jun 2, 2017

I'm tempted to replace all the shinyApp(ui = ui, server = server) with shinyApp(ui, server)--what do you think? Other than that, LGTM (I only skimmed)

@mine-cetinkaya-rundel
Copy link
Contributor Author

@jcheng5 One could use arbitrary names for the ui and server functions, and then assign those to the ui and server arguments in the shinyApp function, right? I mean, this might not be best practice, but it seems doable. If so, it makes sense to leave them as is to me, to make the distinction between argument name and input.

@mine-cetinkaya-rundel
Copy link
Contributor Author

Alternatively leave as is for first couple examples, then shorten?

@mine-cetinkaya-rundel
Copy link
Contributor Author

Implemented @jcheng5's suggestion for examples 3-11 except for custom HTML example

@wch wch merged commit d7391b1 into rstudio:master Jul 11, 2017
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.

None yet

5 participants