Add DartPad to flutter.dev by legalcodes · Pull Request #2938 · flutter/website

legalcodes · 2019-08-27T01:10:21Z

Last staged 9/26/19:

https://jt-flutter-website-3.firebaseapp.com (Scroll all the way to the bottom)

sfshaza2

@legalcodes, I think this looks good, but has it been reviewed by @maguinis, @timsneath, and the UX folk?

I wonder if we should include a "Try Flutter" link higher up in the front page. I'm not sure how many ppl scroll down to the bottom. It would be easy to miss that DartPad instance. I also like injecting it into the Install page, but I think it needs to be higher up in the page. It would be easy to miss in the TOC, and it would be equally easy to select a platform and proceed to the next steps without noticing DartPad. I'm wondering if we should add a new step 1: Try Flutter, and then step 2: Install. OR, another suggestion is we replace the big blue "Get started" button at the top right of every page with "Try Flutter". And then the Try Flutter page leads to install.

Just some ideas.

maguinis · 2019-08-27T13:49:12Z

This has my approval - I sat next to Jon yesterday as we made these changes. I proposed adding it to the Install page since that is where our "Get Started" buttons all lead to and hence it is the path that devs wanting to try Flutter take. I found this to be a cleaner option than to add 'Try Flutter' since that could be confused with 'Get Started' (unless we changed 'Get Started' to 'Download'. One thing I did mention is that having DartPad on bottom of homepage and Install page could help us A/B test and if we feel more users are using the Homepage version then we can alter buttons to point to that one. Also, since the current time on the install page is very short, it will be a good measure to see how much longer on average users spend on that page when we implement DartPad there. Martin Aguinis | Developer Marketing | Flutter

…

On Tue, Aug 27, 2019 at 12:24 AM Shams Zakhour (ignore Sfshaza) < ***@***.***> wrote: ***@***.**** commented on this pull request. @legalcodes <https://github.com/legalcodes>, I think this looks good, but has it been reviewed by @maguinis <https://github.com/maguinis>, @timsneath <https://github.com/timsneath>, and the UX folk? I wonder if we should include a "Try Flutter" link higher up in the front page. I'm not sure how many ppl scroll down to the bottom. It would be easy to miss that DartPad instance. I also like injecting it into the Install page, but I think it needs to be higher up in the page. It would be easy to miss in the TOC, and it would be equally easy to select a platform and proceed to the next steps. I'm wondering if we should add a new step 1: Try Flutter, and then step 2: Install. OR, another suggestion is we replace the big blue "Get started" button at the top right of every page with "Try Flutter". And then the Try Flutter page and lead to install. Just some ideas. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2938?email_source=notifications&email_token=ACFSNXQWSRCZLCULN24F63LQGTJC7A5CNFSM4IPZTIQ2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCCYHOJQ#pullrequestreview-280000294>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACFSNXSMZB6SWX4RQMSDA7TQGTJC7ANCNFSM4IPZTIQQ> .

sfshaza2 · 2019-08-27T16:13:32Z

@maguinis, glad to hear. I was suggesting that we rename the "Get started" button to "Try Flutter", but we could also add a second button that says "Try Flutter" and rename "Get started" to "Install". Years ago @sethladd hammered out my preference for "Download" in favor of "Install" (you probably heard me kicking and screaming). Have we gone back to preferring "Download"?

When you say, A/B testing, how do you mean? Are you suggesting that we do actual A/B testing, or do you mean that it's kind of like an A/B test by having it on both pages?

I really think that "Try Flutter" should be a big blue button on every page. What do the UX folk say?

johnpryan

Looks good overall - thanks Jon! My main concern is around including samples that are affected by dart-lang/dart-pad#1223 initially. Maybe we can leave those out for now.

johnpryan · 2019-08-27T13:56:18Z

+    // Snippet('Strings', strings),
+    // Snippet('Collection literals', collectionLiterals),
+    // Snippet('Classes', classes),
+    // Snippet('Compute Pi', piMonteCarlo),


nit: commented out code

johnpryan · 2019-08-27T16:30:02Z

+$dash-dartpad-border: #293542;
+$dash-callout: #121a26;
+$dash-off-white: #f8f9fa;
+$dash-dartpad-border: #293542;


this variable is defined twice

johnpryan · 2019-08-27T16:30:34Z

@@ -1,3 +1,55 @@
+
+$dash-dartpad-border: #293542;


nitpick: Maybe this should be (0, 0, 0, 0.125); to match the card border?

johnpryan · 2019-08-27T16:34:57Z

+  var select = querySelector('#dartpad-select');
+
+  var snippets = [
+    Snippet('Counter', counter),


My only concern with this and the todo examples is that they require Chrome 77 to work well ( dart-lang/dart-pad#1223).

I agree, I think that just before or after the first DartPad instance, there should be a warning about the version of Chrome that you need.

Cool, as discussed we'll:

Wait until 77

add a warning about the Chrome version

johnpryan · 2019-08-27T16:35:37Z

+  Widget build(BuildContext context) {
+    return Container(
+        height: 50,
+        width: 400,


nitpick: the width doesn't need to be set

johnpryan · 2019-08-27T16:35:55Z

+              border: OutlineInputBorder(),
+              labelText: labelText,
+              hintText: hintText),
+        ));


nit: put a comma between these to and re-format

Nice catch:

return Container( height: 50, margin: const EdgeInsets.all(20.0), child: TextField( obscureText: false, decoration: InputDecoration( icon: Icon(icon), border: OutlineInputBorder(), labelText: labelText, hintText: hintText), ) );

(without adding a comma, right?)

adding a comma will tell the formatter to format it like this:

Widget build(BuildContext context) { return Container( height: 50, width: 400, margin: const EdgeInsets.all(20.0), child: TextField( obscureText: false, decoration: InputDecoration( icon: Icon(icon), border: OutlineInputBorder(), labelText: labelText, hintText: hintText), ), );

which I think is a best-practice for readability

Interesting. Thanks for the tip

johnpryan · 2019-08-27T16:36:33Z

+'''
+    .trim();
+
+var strings = '''


let's take out the samples that aren't being used - they can be added back in if needed.

redbrogdon · 2019-08-27T18:39:03Z

+import 'package:dartpad_picker/dartpad_picker.dart';
+
+void main() {
+  if (isMobile()) {


This needs to be extended to detect Safari as well due to https://bugs.webkit.org/show_bug.cgi?id=199866.

This is the check we're doing in dart-pad:

if (window.navigator.vendor.contains('Apple') && !window.navigator.userAgent.contains('CriOS') && !window.navigator.userAgent.contains('FxiOS')) { }

this also needs to be changed on site-www dart-lang/site-www#1854

johnpryan · 2019-08-27T18:39:37Z

+
+class SpinningSquare extends StatefulWidget {
+  @override
+  _SpinningSquareState createState() => new _SpinningSquareState();


nitpick: new isn't required anymore

johnpryan · 2019-08-27T18:39:46Z

+          width: 150.0,
+          height: 150.0,
+          color: const Color(0xFF00FF00),
+        ));


also need a comma here

sfshaza2 · 2019-08-27T20:06:34Z

Some other suggestions, after a in-person discussion with Kathy and Jon:

The "Get started" CTA takes you to a page called "Install". This is disconcerting. Let's add a page actually called "Get started". At the top of that page, have the "Try Flutter" Dartpad. Either on that page, or a second page, provide the "install Flutter" option with the platform pre-selected. (Do we want to call it Install or Download? Currently, the platform is NOT pre-selected on flutter.dev, but it is on dart.dev.)
Jon is going to look into how much work it would be to do real A/B testing. If it's not an onerous amount of work, then we can test whether one page for try Flutter/install is better or two pages is better. Kathy gave Jon the name of an external Google service that implements A/B testing and works on Firebase, as well as AppEngine.
Jon also plans to talk to the UX folk to see if they have a preference for how this is implemented.

galeyang · 2019-08-28T23:30:38Z

Talked to @legalcodes offline. Recommend to divide this into several PRs/issues

Landing page:
We'd suggest to go ahead --- putting DartPad at the bottom of flutter.dev and monitor the usage. We can track the funnel of (1) page visit, (2) to seeing it, (3) and try it. For the "seeing it part", we need to add extra tracking code.
Get started and installation parts can be in different PRs.

Detailed feedback about Dartpad on the landing page

Heading "Try Flutter in your browser": text color should be #4A4A4A. Same as other headings.
iframe border color should be: #cccccc. Same as other use cases on flutter.dev
It'd good if the sample code (at least the first one) is visually compelling. In the past UX study, new users may raise concerns if UIs shown on the landing page and showcase page are not very polished.
Maybe add a little comments to explain the code since most users are very new to Flutter/Dart when they see the code on the landing page.
"Want more practice? Try a codelab." maybe link to the layout codelab since other codelabs require users to install Flutter SDK first.

- Reframe the installation page as the very first step of get started (aka try/install) - Put the embedded Dartpad above the installation buttons and change the page title. - Update title in left nav

sfshaza2

LGTM!

maguinis · 2019-09-24T02:53:57Z

LGTM from Marketing. Let's ship!

johnpryan · 2019-09-24T15:50:53Z

+  var select = querySelector('#dartpad-select');
+
+  var snippets = [
+    Snippet('Spinning flutter', spinning_logo),


nit: "Flutter"?

sfshaza2 · 2019-09-26T19:30:37Z

I think the last I saw about this PR, Jon was suggesting that we back out changes to the Install page and only update the home page, by adding DP, with no pointers, to the bottom of the page.

It doesn't feel like there's much point to the PR if that's all it does. We would track how many people find the DP instance at the bottom of the page, and that's it.

Thoughts?

legalcodes · 2019-09-26T19:51:37Z

@sfshaza2 Thank you for your thoughts!

The original goal of this PR was to do just that -- add DartPad to the homepage as a standalone PR. The proposal to add DartPad to the install page came as a later addition, and we have had extensive discussion about the benefits of modifying the install page in a separate PR from this one @galeyang @redbrogdon please feel free to chime in!

Of the ~550 lines of code in this PR, only ~30 pertain to modifying src/docs/get-started/install/index.md so IMHO this is PR is valid on its own, without modifying the install page, since it (1) adds necessary dartpad_picker code to flutter/website and (2) adds DartPad to the homepage, but I am open to hearing other thoughts.

…padHomepage

sfshaza2 · 2019-09-27T15:54:07Z

LGTM! Ship it! (I'll let you merge it.)

chalin · 2019-10-15T19:19:41Z

 .flutter-plugins
 .packages
 .dart_tool
-# pubspec.lock


Hi @legalcodes @johnpryan et all. Why was it decided that we'd start committing the pubspec.lock files contained in the repo?

Hey all, I created a fresh issue to discuss this (#3107) rather than only having a comment in an old PR. (Sorry for the email notification noise.)

Oups, my bad, I wasn't looking at this edit carefully: you just deleted a commented out line! I'm the one who commented it out in #1237 last year. I didn't realize that it had been that long. I thought that it was a change that had been done in the last 6 months or so. Apologies!

FYI, I'll still go ahead a remove the pubspec.lock files.

chalin · 2019-10-15T20:12:19Z

    <i class="fas fa-exclamation-triangle"></i> **Warning:**
  end: </aside>

+custom:


We should probably drop this custom field/tag at some point.

legalcodes added 4 commits August 22, 2019 11:41

Added dartpad but with old examples from dart.dev

39f9584

Updated dartpad to flutter mode + light theme

1c5fcab

Refactor to counter example

54abe74

Added 3 working examples, added dartpad to install page

377b54e

legalcodes requested review from johnpryan, maguinis and sfshaza2 August 27, 2019 01:10

googlebot added the cla: yes Contributor has signed the Contributor License Agreement label Aug 27, 2019

legalcodes requested a review from redbrogdon August 27, 2019 01:12

legalcodes added the st.WIP Issue in progress label Aug 27, 2019

sfshaza2 reviewed Aug 27, 2019

View reviewed changes

johnpryan requested changes Aug 27, 2019

View reviewed changes

redbrogdon reviewed Aug 27, 2019

View reviewed changes

johnpryan reviewed Aug 27, 2019

View reviewed changes

johnpryan mentioned this pull request Aug 27, 2019

Add Safari detection for DartPad on front page dart-lang/site-www#1854

Closed

legalcodes added 3 commits September 13, 2019 13:04

Add an include in index

bcdca05

Add style and language updates to /docs/get-started/install:

7b0d751

- Reframe the installation page as the very first step of get started (aka try/install) - Put the embedded Dartpad above the installation buttons and change the page title. - Update title in left nav

Minor updates/corrections

92dad6c

legalcodes requested a review from johnpryan September 23, 2019 21:48

legalcodes changed the title ~~[WIP] Add DartPad to flutter.dev~~ Add DartPad to flutter.dev Sep 23, 2019

Fix broken link for chrome os

71994fe

legalcodes removed the st.WIP Issue in progress label Sep 23, 2019

legalcodes added 2 commits September 23, 2019 17:13

Add styling tweak to dartpad section on landing page

3b4980b

Use 'Install Flutter' on landing page

30105a3

legalcodes requested review from redbrogdon and sfshaza2 September 24, 2019 00:21

sfshaza2 approved these changes Sep 24, 2019

View reviewed changes

johnpryan approved these changes Sep 24, 2019

View reviewed changes

johnpryan reviewed Sep 24, 2019

View reviewed changes

legalcodes added 4 commits September 24, 2019 09:36

Fix typo

91cbf66

Rebuild dartpad_picker

11f0c9a

Remove warning about Chrome 77

57403b6

Left-align headings on install page

ecbeef7

legalcodes added 3 commits September 26, 2019 14:15

Remove changes to install page

75f3682

Remove changes to sidenav for install page

90685d8

Merge branch 'master' of https://github.com/flutter/website into dart…

52ac743

…padHomepage

legalcodes merged commit 06189f3 into flutter:master Sep 27, 2019

chalin reviewed Oct 15, 2019

View reviewed changes

Uh oh!

Conversation

legalcodes commented Aug 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sfshaza2 left a comment • 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!

maguinis commented Aug 27, 2019 via email

Uh oh!

sfshaza2 commented Aug 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

johnpryan 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!

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!

sfshaza2 commented Aug 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

galeyang commented Aug 28, 2019

Uh oh!

sfshaza2 left a comment

Choose a reason for hiding this comment

Uh oh!

maguinis commented Sep 24, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sfshaza2 commented Sep 26, 2019

Uh oh!

legalcodes commented Sep 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sfshaza2 commented Sep 27, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chalin Oct 15, 2019 • 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!

chalin Oct 16, 2019 • 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!

legalcodes commented Aug 27, 2019 •

edited

Loading

sfshaza2 left a comment •

edited

Loading

sfshaza2 commented Aug 27, 2019 •

edited

Loading

sfshaza2 commented Aug 27, 2019 •

edited

Loading

legalcodes commented Sep 26, 2019 •

edited

Loading

chalin Oct 15, 2019 •

edited

Loading

chalin Oct 16, 2019 •

edited

Loading