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

Add element to Shiny input #41

Merged
merged 7 commits into from Jul 9, 2020
Merged

Add element to Shiny input #41

merged 7 commits into from Jul 9, 2020

Conversation

@timelyportfolio
Copy link
Collaborator

@timelyportfolio timelyportfolio commented Jun 29, 2020

Normally in React this would be not be considered best practice but in the case of Shiny we know that element is stable across a session.

Also, many of the npm dependencies are out-of-date, so potentially at the risk of confusing this with that, went ahead and upgraded all npm dependencies. JS tests all passed.

https://github.com/react-R/reactvega/blob/master/srcjs/reactvega.jsx is an example usage of new functionality that allows us to add the el id to our Shiny input value for the purpose of referencing in Shiny with observe/observeEvent. I don't think there is any other way to accomplish otherwise.

@timelyportfolio
Copy link
Collaborator Author

@timelyportfolio timelyportfolio commented Jun 29, 2020

  • document change
  • add NEWS.md item
  • minor increment version
@timelyportfolio
Copy link
Collaborator Author

@timelyportfolio timelyportfolio commented Jun 29, 2020

@glin I have tested with reactable but would you be able to confirm that there is nothing breaking in these changes? Thanks so much.

The el change does not apply to htmlwidgets so hopefully nothing there, but there is a small chance that npm updates might affect.

@glin
Copy link

@glin glin commented Jul 6, 2020

@timelyportfolio Confirmed, all looks good here!

@timelyportfolio timelyportfolio merged commit a5d9869 into master Jul 9, 2020
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
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.