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

Shiny improvements #10

Merged
merged 11 commits into from Jul 7, 2021

Conversation

chriscpritchard
Copy link
Collaborator

@chriscpritchard chriscpritchard commented Jun 8, 2021

This PR is for shiny improvements. Currently in a draft state as further improvements needed - this branch is based off the standard-functions branch, so #7 would ideally need to be merged before this.

Closes #6
Closes #9

Renamed read_PRISMAdata() to PRISMA_data(), deprecated read_PRISMAdata()

Renamed sr_flow_interactive() to PRISMA_interactive_(), moved to utils.R

prefixed utils functions with PRISMA_ (renamed insertJS_ to insert_js_)

Added ORCID and email to contributor page
@chriscpritchard
Copy link
Collaborator Author

Tested this @nealhaddaway @mcguinlu works fine with the changes Neal made on the master branch so should be good to go and this can be released / deployed as V0.0.2 as it includes #7. I haven't tested on shinyapps.io as I don't have an account, but according to their website they support devtools::install_github() so it should just pull the latest version from here!

@nealhaddaway
Copy link
Collaborator

Great - thanks, Chris! And should I merge both PRs, or do one at a time...?

@chriscpritchard
Copy link
Collaborator Author

chriscpritchard commented Jul 7, 2021 via email

@nealhaddaway nealhaddaway merged commit 2b105e9 into prisma-flowdiagram:master Jul 7, 2021
@nealhaddaway
Copy link
Collaborator

Great - I've done that now

@nealhaddaway
Copy link
Collaborator

Hmm - it seems to have lost my code for removing blank lines in the sources boxes... But it's not super important...

@chriscpritchard
Copy link
Collaborator Author

THat's really annoying, give me a few minutes I'll try and see where that happened (might have been in the rejiggering of functions and I missed it when merging).

@chriscpritchard
Copy link
Collaborator Author

Yup I've found the issue, I think it was only updated in the shiny code so I missed it...

@nealhaddaway
Copy link
Collaborator

Ah cool - what's the best thing to do?

@chriscpritchard
Copy link
Collaborator Author

I'll fix it check it works and make a PR

@nealhaddaway
Copy link
Collaborator

nealhaddaway commented Jul 7, 2021 via email

@chriscpritchard
Copy link
Collaborator Author

Done, PR #12 fixes this, sorry I missed it. I've taken your code from the shiny function and added it to the main one

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.

Shiny improvements Standardise function names
2 participants