-
Notifications
You must be signed in to change notification settings - Fork 330
Co-edit module created in Android Studio #4004
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
Conversation
So, after this PR: File > New > New Flutter Project will so:
File > New > New Module will show:
And the Flutter Module screen will show the additional four options? It sounds like most users will still go through the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some initial comments and questions. I'd like to figure out what percentage of people would be using this flow (vs. the New Flutter project flow), how alarming that initial gradle error will be, and what we can do to avoid the error in the first place.
@@ -48,5 +48,6 @@ | |||
<orderEntry type="library" name="com.android.tools:common" level="project" /> | |||
<orderEntry type="library" name="com.android.tools:repository" level="project" /> | |||
<orderEntry type="library" name="studio-analytics-proto" level="project" /> | |||
<orderEntry type="module" module-name="intellij.groovy.psi" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this used? I don't see any references in the code to a groovy package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I tried taking it out and got a compile error.
new FlutterProjectCreator(this).createModule(); | ||
if (projectType().get().isPresent() && projectType().get().get() == FlutterProjectType.IMPORT) { | ||
String location = projectLocation().get(); | ||
assert (!location.isEmpty()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to assert here? If the location is empty, that will throw an exception.
I don't see where the location
local variable is used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's been a long time since I wrote that and don't remember why I wanted that assert. I think it is last-chance error checking since without a location we can't do an import. But the wizard verification code shouldn't let the user get to this point without a valid location.
String location = projectLocation().get(); | ||
assert (!location.isEmpty()); | ||
if (isModule()) { | ||
// The FlutterProjectType.MODULE is used in two places. In FlutterProjectModel it is used to create a new top-level |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is handleFinished()
long running? Can or should anything here be done in a background task, w/ progress reported to the user?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it runs a while. Progress is reported via the console and the status bar at the bottom of the window. It is in here that the error is generated. I'm thinking it might be better to wrap a modal progress indicator around all this, but that's another issue.
After this PR the New Flutter Project wizard does not change. The New Module wizard does not change if the host project is Flutter. If the host project is Android without Flutter then the New Module wizard shows two items for Flutter (first screen shot). If the host project is Android that has an add-to-app Flutter module then the New Module wizard shows four items for Flutter (second screen shot).
I expect that to be true. This is only for those people who need to add Flutter to an existing Android app and want to have the IDE do it for them. IIRC there is a relatively low number of people interested today, but a recent survey indicated growing interest. If the tool makes it easy to experiment then I'd expect the number to grow, but still be significantly lower that pure-Flutter users. One idea for dealing with the Gradle error is to wrap the long-running code in a modal progress indicator. Doing that would prevent making any changes to the code while the analysis and conversion is completing. It might be possible to "clean up" the error message, too. I haven't found any way to prevent it, though. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One idea for dealing with the Gradle error is to wrap the long-running code in a modal progress indicator. Doing that would prevent making any changes to the code while the analysis and conversion is completing. It might be possible to "clean up" the error message, too. I haven't found any way to prevent it, though.
I'm not familiar enough with the workflow around the end of the dialog and the handleFinished()
call, but one idea is to delegate all the work in that call to an IntelliJ Task.Backgroundable
. It'll show progress to the user and run asynchronous. I don't think it would give us any additional tools in terms of capturing the error however.
There's no need to add another background task; that stuff is already running in the background. The user is free to edit or whatever while the Flutter build and Gradle sync are running. Thanks for the review! |
* Create add-to-app module directly in AS
Add the ability to create an add-to-app module directly to Android Studio.
Needs work: Gradle sync generates an error claiming that a module cannot be compiled. That error is clean up in a later pass, but for now I can't find a way to prevent it.
File > New > New Module...
looks like this for an Android app with no Flutter components, after scrolling to the bottom of the list:And, yes, there is a TODO to differentiate the icons.
Import Flutter Module
already exists and is how existing modules are linked into Android apps. The newFlutter Module
is for creating add-to-app modules.After a Flutter module has been added it has a few more options:
This is quite a lengthy process. In order to link the Flutter module into the Android app we first have to build it. Running
flutter build aar
takes about 5 minutes on my machine. In addition we have to run Gradle sync twice: once after the AAR is built to pick up the Flutter Android Gradle module, and again after that to fully link the module into the app to enable co-editing. The two sync's can run in just a few seconds, though, depending on project complexity.Switching the Project view to the Project Files option provides good visibility of both Flutter and Android sources.