issue #1047 - Add additional settings to the project creation. #1232

branflake2267 · 2017-07-23T03:48:25Z

This works for IDEA. I'm looking at Webstorm next.

The default view for IDEA:

Todos:

Project type selection - Which should it be: radios, Combobox or wizard?
Help section - change the background color when the Dracula theme is selected.

branflake2267 · 2017-07-23T03:54:06Z

src/io/flutter/FlutterBundle.properties

+flutter.module.create.settings.radios.android.kotlin=Kotlin
+flutter.module.create.settings.radios.android.tip=Select an Android language.
+flutter.module.create.settings.radios.ios.label=iOS language:
+flutter.module.create.settings.radios.ios.object_c=Object C


branflake2267 · 2017-07-23T23:17:53Z

I've added the webstorm support. So now both IDEA and Webstorm have support for the additional creation fields.

The webstorm fields.

devoncarew · 2017-07-24T04:21:39Z

Wow, @branflake2267 , this is fantastic! Adding @pq as the reviewer since he's worked on the wizards most recently. A couple of drive by thoughts:

Android language: should say Java or Kotlin (currently iOS or Kotlin)
I'm of two minds about asking the Application / Project question on this page, or having a 2nd wizard (Flutter Application vs. Flutter Plugin). And - not really relevant to whether to use one wizard or two - users very new to Flutter will have difficulty knowing which type of flutter project they want.
the android / iOS language choice may do better as a combo box?
we might choose to elide the driver test question completely? Either always create a driver test, or never do (with some cli route to create a test in a separate step)

branflake2267 · 2017-07-24T04:27:54Z

lol, oops on the Android language choice. Spaced on that. Good catch. :) Thanks.

Combos work for me, gets the job done.
Fine with me on the driver test removal.

branflake2267 · 2017-07-24T04:30:28Z

src/io/flutter/FlutterBundle.properties

+flutter.module.create.settings.description.tip=Enter a project description which shows up in the pubsec.yaml.
+flutter.module.create.settings.description.default_text=A new Flutter project.
+flutter.module.create.settings.radios.android.label=Android language:
+flutter.module.create.settings.radios.android.ios=iOS


s/ios/java/

branflake2267 · 2017-07-24T04:36:04Z

The latest rendition, with Java. :)

branflake2267 · 2017-07-24T04:45:53Z

src/io/flutter/module/settings/FlutterCreateAddtionalSettingsFields.java

+    settingsStep.addSettingsField(FlutterBundle.message("flutter.module.create.settings.help.project_name.label"),
+                                  new JLabel(FlutterBundle.message("flutter.module.create.settings.help.project_name.description")));
+    settingsStep.addSettingsField(FlutterBundle.message("flutter.module.create.settings.help.org.label"),
+                                  new JLabel(FlutterBundle.message("flutter.module.create.settings.help.org.description")));


Add help for project type, to help distinguish application vs plugin?

Project type: Select 'application' to build a mobile application for end users.
_____________ Choose a 'plugin' if you're building native device feature hooks for a developer.

I love this. I wonder if we could do something to set this help apart a bit more. Maybe putting it in it's own group (sorry, that's an eclipse/SWT-ism... not sure what that's called in Swing). Or having a dividing line or styled text? Maybe we could add a hyperlink to get more info as well? (Oops: I guess that assumes there's a good page with flutter create docs but I don't see one. cc @LarkAscending in case I'm missing one.

Good idea. I think you could achieve the grouping effect by compositing a panel and that panel could look like it's in a group, or put a group in that panel. That could be nifty.

mit-mit · 2017-07-24T08:11:28Z

This is great @branflake2267 !

I think we need to split this over two pages in the wizard:

select project type (application, plugin, or package)
set properties (name, language, etc.)

The reason is that not all project types have the same properties. For example, a package does not contain any platform code, and thus is does not make sense to ask for android and ios languages for that one.

branflake2267 · 2017-07-24T15:24:39Z

Thanks. Ah, I didn't see package on the horizon. In that case I'm with you. I can see explaining the type generation options could be a benefit in specific wizard selecting the generation type. It would be easy to add another wizard and land on the properties for the particular generation type.

Another option I like to use for dimensional properties, on selection is to disable those that don't apply. If the package options were minor, the properties that don't apply could be changed or disabled. Would this be of interest?

Would you be the consensus for adding a wizard to my patch?

branflake2267 · 2017-07-24T15:41:17Z

WebStorm only has one step and IDEA has two steps in the project create wizards. It looks possible to add wizard steps to Webstorm, so I would suggest keeping the wizards consistent with IDEA. Then this would mean the flutter SDK selection would have to be extracted into it's own wizard. What are your thoughts on keeping Webstorms and IDEAs steps similar?

devoncarew · 2017-07-24T16:24:19Z

@branflake2267, we're not optimizing for the webstorm experience - having it be slightly different than IDEA is fine.

In terms of keeping this one page, perhaps a combo box for project type? That way we can default to Flutter Application, and make the common case simple. Options which don't apply - like platform code types for package project - could be disabled.

pq

A bunch of nits but substantially awesome! 🎉

pq · 2017-07-24T17:05:28Z

src/io/flutter/module/FlutterModuleBuilder.java

  }

+  @Nullable
+  @Override


nit: make final?

pq · 2017-07-24T17:07:11Z

src/io/flutter/module/settings/FlutterCreateAddtionalSettingsFields.java

+  private JTextField descriptionField;
+  private RadiosForm androidLanguageRadios;
+  private RadiosForm iosLanguageRadios;
+  private JCheckBox includeDriverTextField;


Can these all be made final?

pq · 2017-07-24T17:08:52Z

src/io/flutter/module/FlutterSmallIDEGeneratorPeer.java

  }

+  public FlutterCreateAdditionalSettings getAddtionalSettings() {
+    return settingsFields.getAddtionalSettings();


I'm guessing buildUI is sure to have been called before this is accessed and settingsFields is certain not to be null?

I'm moving the instantiation to the ctor.

pq · 2017-07-24T17:10:53Z

src/io/flutter/module/settings/RadiosForm.java

+
+    radio2.setText(label2);
+
+    ButtonGroup radioGroup = new ButtonGroup();


nit: make final?

pq · 2017-07-24T17:11:40Z

src/io/flutter/sdk/FlutterCreateAdditionalSettings.java

+  }
+
+  public List<String> getArgs() {
+    List<String> args = new ArrayList<String>();


nit: final?

Also, I think you can simplify to:

new ArrayList<>();

pq · 2017-07-24T17:22:34Z

src/io/flutter/sdk/FlutterSdk.java

+    // keep as the last argument
+    args.add(appDir.getName());
+
+    String[] vargs = args.stream().toArray(String[]::new);


And final here too...

pq · 2017-07-24T17:23:22Z

testSrc/unit/io/flutter/sdk/FlutterCreateAddtionalSettingsTest.java

+
+    List<String> args1 = addtionalSettings1.getArgs();
+    List<String> args2 = addtionalSettings2.getArgs();
+    List<String> args3 = addtionalSettings3.getArgs();


More opportunities to go final...

pq · 2017-07-24T17:24:51Z

testSrc/unit/io/flutter/sdk/FlutterCreateAddtionalSettingsTest.java

+    List<String> args2 = addtionalSettings2.getArgs();
+    List<String> args3 = addtionalSettings3.getArgs();
+
+    assertTrue(args1.size() == 1);


Not a big deal but failure would read a little better if this were:

assertEquals(1, args1.size());

pq · 2017-07-24T17:25:38Z

testSrc/unit/io/flutter/sdk/FlutterCreateAddtionalSettingsTest.java

+    assertEquals("--plugin", args1.get(0));
+
+    assertTrue(args2.size() == 0);
+    assertTrue(args3.size() == 0);


Similarly preferring assertEquals here?

pq · 2017-07-24T17:26:15Z

testSrc/unit/io/flutter/sdk/FlutterCreateAddtionalSettingsTest.java

+  }
+
+  @Test
+  public void descriptionPropertyTest() {


Comments in generatePluginPropertyTest apply here to and below. 👍

branflake2267 · 2017-07-24T17:28:30Z

I like the current config I've proposed in this patch the best with radio selection. My solution to the selection complexity would be to add notes to the help section and probably a link to get started if that were possible. And I like disabling vs adding wizards. Although a combo has the exact same effect I'd go after except, I might want to add some verbiage to a combo's list to explain the option in more detail. I just want more options, so I'll build however you like it.

branflake2267 · 2017-07-25T05:23:51Z

I've appended a commit with the latest nit fixes and added help section.

Todos:

Fix the background color for dracula. I'm not sure how to change it yet, ran out of time to look for docs. Do you have any notes on changing colors based on theme?
Figure out what the consensus is for project type selection, combo or radio? Or is the latest rendition worth trying?
Test creating projects based on my latest changes, possibly add mockito to start mocking wizards...

branflake2267 · 2017-07-25T05:27:57Z

Webstorm scrolls with help, hm...

branflake2267 · 2017-07-25T06:07:20Z

What do you think of having a link to the getting started guide? I've appended a commit for this.

branflake2267 · 2017-07-26T03:54:02Z

Latest rendition.

IntelliJ IDEA Dracula:

IntelliJ IDEA Default:

WebStorm:

@pq @devoncarew What do you think so far? Let me know if you want to try out the combos for project selection. I imagine some more tuning will need to be done on the helps. How do you want to proceed?

devoncarew · 2017-07-26T08:41:59Z

@branflake2267, the UI looks great! We may follow up with some tweaks and iterations after it lands (like trying out a combo for project selection to see how it feels).

I'll let @pq speak to the code in terms of a lgtm. We're more or less in feature freeze for M16 (releasing this week), so we'll likely wait to land for a day or two until after we push M16.

devoncarew · 2017-07-26T08:43:57Z

Let me know if you want to try out the combos for project selection

And - I would like to see how this looks - but it's not something we need to do for this PR. Happy to iterate a bit in follow-ups if that's your preference.

pq · 2017-07-26T13:14:39Z

Cool!

As @devoncarew said, let's wait until M16 goes out (#1153) and then land this and iterate. 🎉

pq · 2017-07-26T13:22:33Z

src/io/flutter/module/settings/SettingsHelpForm.form

+            <properties/>
+            <border type="none"/>
+            <children>
+              <component id="26e7" class="javax.swing.JTextPane" binding="gettingStartedUrl">


I notice the styling of this looks a little different in your screenshot. I wonder if using a LinkLabel (as we do for the SDK install link) would look more consistent?

branflake2267 · 2017-07-26T15:12:51Z

@devoncarew @pq Sounds good on iteration. I think a combo might be nice, I'll wait and see what you do.

@pq I didn't use a LinkLabel, that looks like a better choice. I used a JTextPane with html. I'll change that over tonight. Thanks.

pq · 2017-07-26T22:15:27Z

Great! No worries about the LinkLabel. If you don't get to it we can pick that up on another pass. The important bits are here and thanks a million for that!

…nal create settings.

branflake2267 · 2017-07-27T03:19:47Z

I rebased onto the head. That'll be it until Monday for me. I'll be out of pocket until then, just in case.

pq · 2017-07-27T14:11:57Z

Super, thanks! M16 should be out by then too. 🎉

pq · 2017-07-31T18:53:21Z

As per the plan, I'll land this now and we can iterate/fiddle.

Thanks!

branflake2267 · 2017-07-31T19:12:26Z

Sounds nifty :)

googlebot added the cla: yes label Jul 23, 2017

branflake2267 commented Jul 23, 2017

View reviewed changes

branflake2267 force-pushed the brandon-new-project-settings branch 3 times, most recently from 58a7981 to e603fa7 Compare July 23, 2017 05:03

branflake2267 force-pushed the brandon-new-project-settings branch from 5959f07 to e5e7329 Compare July 23, 2017 23:26

devoncarew requested a review from pq July 24, 2017 04:21

branflake2267 commented Jul 24, 2017

View reviewed changes

branflake2267 force-pushed the brandon-new-project-settings branch from e5e7329 to b454d30 Compare July 24, 2017 04:35

branflake2267 commented Jul 24, 2017

View reviewed changes

pq reviewed Jul 24, 2017

View reviewed changes

branflake2267 force-pushed the brandon-new-project-settings branch 3 times, most recently from abdf591 to e2d6f2e Compare July 26, 2017 03:51

pq reviewed Jul 26, 2017

View reviewed changes

branflake2267 force-pushed the brandon-new-project-settings branch from e2d6f2e to c3d2fa9 Compare July 27, 2017 03:01

branflake2267 added 4 commits July 26, 2017 20:14

issue flutter#1047 - Add additional settings to the project creation.

a2107de

issue flutter#1047 - add additional flutter create fields for webstorm.

ec3892f

issue flutter#1047 - Fixing nits and adding a help section to additio…

b5bcb28

…nal create settings.

issue flutter#1047 - Add a getting started link to the wizard.

5f793ef

branflake2267 force-pushed the brandon-new-project-settings branch from c3d2fa9 to 5f793ef Compare July 27, 2017 03:16

pq merged commit d77887a into flutter:master Jul 31, 2017

pq mentioned this pull request Jul 31, 2017

Migrate to a LinkLabel #1255

Merged

devoncarew mentioned this pull request Aug 3, 2017

iterate on the new project wizard #1262

Closed


		radio2.setText(label2);

		ButtonGroup radioGroup = new ButtonGroup();

issue #1047 - Add additional settings to the project creation. #1232

issue #1047 - Add additional settings to the project creation. #1232

Uh oh!

Conversation

branflake2267 commented Jul 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

branflake2267 commented Jul 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

devoncarew commented Jul 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

branflake2267 commented Jul 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

branflake2267 commented Jul 24, 2017

Uh oh!

branflake2267 Jul 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mit-mit commented Jul 24, 2017

Uh oh!

branflake2267 commented Jul 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

branflake2267 commented Jul 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

devoncarew commented Jul 24, 2017

Uh oh!

pq left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

branflake2267 commented Jul 24, 2017

Uh oh!

branflake2267 commented Jul 25, 2017

Uh oh!

branflake2267 commented Jul 25, 2017

Uh oh!

branflake2267 commented Jul 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

branflake2267 commented Jul 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

devoncarew commented Jul 26, 2017

branflake2267 commented Jul 23, 2017 •

edited

Loading

branflake2267 commented Jul 23, 2017 •

edited

Loading

devoncarew commented Jul 24, 2017 •

edited

Loading

branflake2267 commented Jul 24, 2017 •

edited

Loading

branflake2267 Jul 24, 2017 •

edited

Loading

branflake2267 commented Jul 24, 2017 •

edited

Loading

branflake2267 commented Jul 24, 2017 •

edited

Loading

branflake2267 commented Jul 25, 2017 •

edited

Loading

branflake2267 commented Jul 26, 2017 •

edited

Loading