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

Easy React-based input bindings #22

Merged
merged 63 commits into from Apr 9, 2019
Merged

Easy React-based input bindings #22

merged 63 commits into from Apr 9, 2019

Conversation

@alandipert
Copy link
Collaborator

@alandipert alandipert commented Feb 22, 2019

input binding support

This adds a new scaffolding function, scaffoldReactShinyInput, for scaffolding new inputs based on React. It also adds:

  1. A new JavaScript function, reactR.reactShinyInput, for registering React components as Shiny inputs
  2. A new R function, createReactShinyInput, for constructing inputs with custom configuration and a default value on the R side.
  3. An additional tutorial vignette that demonstrates wrapping react-color with a custom input

In addition to the new scaffold function, existing scaffold code was extensively reworked. Scaffold functionality common between the widget and input scaffold functions were factored into helper functions.

@alandipert alandipert changed the title Input binding Easy React-based input bindings Feb 22, 2019
@alandipert alandipert requested a review from timelyportfolio Feb 28, 2019
@alandipert
Copy link
Collaborator Author

@alandipert alandipert commented Feb 28, 2019

@timelyportfolio I think this is in good enough shape for you to play around with now. Thanks in advance for any feedback!

I hope to rope in @jcheng5 for a code review this week too.

I'll collect feedback from you both and then let you know when things are in a merge-worthy state.

@alandipert alandipert marked this pull request as ready for review Mar 15, 2019
@timelyportfolio
Copy link
Collaborator

@timelyportfolio timelyportfolio commented Mar 26, 2019

@alandipert I got a chance to walk through the vignette from scratch, and everything worked nicely. I will try to walk through with a completely new and different component now.

I think we should add app.R' 'package.json' 'webpack.config.js' 'yarn.lock' to .Rbuildignore, so that the package is as near CRAN-worthy as possible and we adhere to those standards. If you agree, I'm happy to submit pull or feel free to change.

R/scaffold_input.R Outdated Show resolved Hide resolved
@alandipert
Copy link
Collaborator Author

@alandipert alandipert commented Mar 27, 2019

@timelyportfolio thanks as always for the close review. I'm happy to make these changes. I got sidetracked this week helping with a Shiny release but I should be able to work on this tomorrow.

I'll also have a very small change I'll push soon that restricts the name argument accepted by the scaffolding function, so as to prevent the user from inputting a name that would cause a JavaScript or R error later on.

@alandipert alandipert force-pushed the input-binding branch from d7e8674 to 74514c8 Mar 28, 2019
@alandipert
Copy link
Collaborator Author

@alandipert alandipert commented Mar 28, 2019

@timelyportfolio OK, all changes are in 👍

@timelyportfolio timelyportfolio merged commit 449d3c3 into master Apr 9, 2019
0 of 2 checks passed
0 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/travis-ci/push The Travis CI build could not complete due to an error
Details
@timelyportfolio
Copy link
Collaborator

@timelyportfolio timelyportfolio commented Apr 9, 2019

thanks @alandipert !!!! merged!!!

@timelyportfolio timelyportfolio mentioned this pull request Apr 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.