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 'New Shiny App' dialog #528

Merged
merged 34 commits into from Nov 18, 2015
Merged

add 'New Shiny App' dialog #528

merged 34 commits into from Nov 18, 2015

Conversation

kevinushey
Copy link
Contributor

Not quite ready for merge (still needs tweaks to UI + failure modes).


This PR adds a 'New Shiny Application' command to the 'Files' menu. Demo:

new-shiny-app

TODO

  • Icon for the menu item
  • How should we use the 'application name' element?
  • Consider a slightly less lightweight UI (?)
  • Failure modes (what to do if app.R, ui.R or server.R already exist (probably show an error dialog)?

# This is a single-file Shiny web application.
# You can find out more about building applications with Shiny here:
#
# http://www.rstudio.com/shiny/
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this now be http://shiny.rstudio.com/?

else if (appType == kShinyAppTypeMultiFile)
{
// ui.R
copyTemplateFile("ui.R", appDir.complete("ui.R"), &result);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's possible that we create ui.R, but fail to create server.R (or vice versa) and end up with leftovers. We should should verify that the files we're attempting to create don't exist beforehand.

@@ -1054,6 +1057,91 @@ public void onNewRMarkdownDoc()
newRMarkdownV1Doc();
}

private void doNewRShinyApp(NewShinyWebApplication.Result result)
{
server_.createShinyApp(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should also verify that Shiny is installed. (now done using dependencyManager_)

@@ -74,6 +74,17 @@ Error validateProjectPath(const json::JsonRpcRequest& request,
return Success();
}

Error getWorkingDirectory(const json::JsonRpcRequest& request,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need this since WorkbenchContext already has a method to get the current working directory (it's kept in sync as the working dir changes): https://github.com/rstudio/rstudio/blob/master/src/gwt/src/org/rstudio/studio/client/workbench/WorkbenchContext.java#L116-L119

@jjallaire
Copy link
Member

@jjallaire: To overcome the fact that the dialog isn't immediately centered you need to create a static ensureStylesInjected method and call it from here: https://github.com/rstudio/rstudio/blob/master/src/gwt/src/org/rstudio/studio/client/RStudio.java#L208

@kevinushey: This was actually just an artefact of how I recorded the screencast (the bottom of the RStudio window was cut off; it does indeed open in the middle of the window)

@jjallaire
Copy link
Member

@jjallaire: Saw that you had a todo for a more lightweight UI. I think a full-on dialog is fully appropriate for this level of user action.

@kevinushey: I was actually thinking the current version might be too lightweight (the help link helps to balance it out a lot)

@jjallaire
Copy link
Member

The 'application name' should be the name of the directory created for the app. The "directory" is the parent directory for the application (so we may want to say "Create within directory" or something like that). To reinforce the fact that the directory is somewhat secondary you could do an alternate treatment like the one here:

screen shot 2015-11-17 at 6 39 14 am

Let's discuss this when you come in.

@jjallaire
Copy link
Member

We'll want to preserve the user's choice for parent directory and application type using persistent client state (I think this can be global persistent client state as opposed to temporary or per-project)

@jjallaire
Copy link
Member

After creating the application we'll want to open the source files within the editor. For ui.R and server.R we need to decide what order to open them in and which one to set focus to. I'd move we set focus to ui.R

@jjallaire
Copy link
Member

For the icon let's use our standard shiny icon (but I'm not sure we have a command sized version of that, I'll check with Paul).

@jjallaire
Copy link
Member

We might want to use a dialog layout that results in a bit larger dialog. Something like this:

screen shot 2015-11-17 at 6 57 50 am

I also think that we should add a left gutter widget in the dialog with a HelpLink (https://github.com/rstudio/rstudio/blob/master/src/gwt/src/org/rstudio/studio/client/common/HelpLink.java) control with caption "Shiny Web Applications" that links to shiny.rstudio.com

@kevinushey
Copy link
Contributor Author

The icon unfortunately looks a bit out of place (all the other icons use the 'file' image as a background while the new one doesn't):

screen shot 2015-11-17 at 2 14 40 pm

But then again, the dialog doesn't necessarily create just a single file, so I'm not sure if we really would want the file background...

@kevinushey
Copy link
Contributor Author

The current view:

screen shot 2015-11-17 at 2 32 00 pm

@kevinushey
Copy link
Contributor Author

Alternate format using radio buttons (beefs up the widget a little bit + reduces number of clicks to change app type)

screen shot 2015-11-17 at 3 19 53 pm

@kevinushey
Copy link
Contributor Author

Thinking a bit more, are we sure we want to cache the directory where we create the Shiny application? IMO it seems more likely that one would want it to be part of the current project (if within an RStudio project).

That is, it seems to me like this action is more likely to be used to add a Shiny application to an existing project, and so it makes sense to make the project directory the default.

Perhaps we can chat about this (and other outstanding issues) sometime tomorrow?

@jjallaire
Copy link
Member

Unfortunately the use of projects is not at all widespread so I don't think
that logic is as compelling as it is at first glance.

That said, we have an easy way to detect that a project is in use so
perhaps we do this:

  1. If the user is in a project then use the current project's root
    directory;
  2. If the user is not in a project then use the sticky value.

On Tue, Nov 17, 2015 at 6:54 PM, Kevin Ushey notifications@github.com
wrote:

Thinking a bit more, are we sure we want to cache the directory where we
create the Shiny application? IMO it seems more likely that one would want
it to be part of the current project (if within an RStudio project).

That is, it seems to me like this action is more likely to be used to add
a Shiny application to an existing project, and so it makes sense to make
the project directory the default.


Reply to this email directly or view it on GitHub
#528 (comment).

@kevinushey
Copy link
Contributor Author

Agree on both fronts -- I'll take a stab at implementing this.

jjallaire added a commit that referenced this pull request Nov 18, 2015
@jjallaire jjallaire merged commit d581382 into master Nov 18, 2015
@jjallaire jjallaire deleted the feature/create-new-shiny-app branch November 18, 2015 18:37
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

2 participants