Ensure Try-Dart example sources are analyzed and tested #2124
Conversation
chalin
added
fix.examples
googlebot
added
the
cla: yes
chalin
force-pushed
the
chalin-homepage-src-1115
branch
from
0f3be81
to
0f7aee6
Compare
|
I'd really prefer to not introduce a second page with DartPad on it. That would leave us with three places: 1) landing page, 2) try-dart page, 3) dartpad.dev. My idea was to add some more white space around the dartpad embed on the landing page so it seems more focused. Checkout this demo: Note that it currently is broken in the mobile/narrow view, I left a TODO, should be easy to fix (?) for someone who unlike me actually knows html & css. Mock implementation: mit-mit@77310ed |
chalin
force-pushed
the
chalin-homepage-src-1115
branch
from
0f7aee6
to
7d17c1e
Compare
I've removed the trial /try-dart page.
I've created #2127 to track this and offer a place for discussion in case @galeyang et all might want to offer some advice. Staging server refreshed with most recent changes. |
|
Btw, the current approach always loads the try-dart example sources (because the sources are embedded in the homepage). If you believe that that is too inefficient for mobile (where we don't show the embedded DartPad), then I can move the try-dart example sources to another page that is only conditionally fetched. Of course there are tradeoffs to having the sources on another page, since that other page might fail to load -- not a situation that we need to deal with in the current approach. |
chalin
changed the title
|
|
||
| // Classes can extend other classes. | ||
| class DiamondSword extends Sword { | ||
| int damage = 50; // ignore: overridden_fields |
There was a problem hiding this comment.
Let's instead change the damage field to a getter to make this warning go away
| // Functions are objects. | ||
| int runTwice(int x, Function f) { | ||
| for (var i = 0; i < 2; i++) { | ||
| x = f(x); // ignore: invalid_assignment |
There was a problem hiding this comment.
would using a local variable make this warning go away?
There was a problem hiding this comment.
No, but giving f the type of int Function(int) works.
| return x; | ||
| } | ||
|
|
||
| // ignore: type_annotate_public_apis |
There was a problem hiding this comment.
let's make the return type void instead.
| return evenNumbers; | ||
| } | ||
|
|
||
| // ignore: type_annotate_public_apis |
There was a problem hiding this comment.
same here, let's make the return type void.
| 'Glass', | ||
| }; | ||
|
|
||
| main() { |
There was a problem hiding this comment.
does this need a void return type to pass analysis?
There was a problem hiding this comment.
If we want to drop the ignore directive in line 1, then yes.
| int timesFour(int x) => timesTwo(timesTwo(x)); | ||
|
|
||
| // Functions are objects. | ||
| int runTwice(int x, Function f) { |
There was a problem hiding this comment.
Why did this change from int Function(int) f?
There was a problem hiding this comment.
It currently is Function f on dart.dev. I changed it to int Function(int) f in the first commit to this PR, and then reverted the change so that we could separately discuss and address any required code changes in another PR.
|
@johnpryan - aside from the examples sources, any feedback on the changes to the other files -- including |
| ), | ||
| ); | ||
|
|
||
| String /*?*/ _getSrc(HtmlElement root, String id, [String fallback]) => |
There was a problem hiding this comment.
Oh, thanks for catching that. The function originally returned a nullable String, but it doesn't anymore. (I use /*?*/ to mark types as nullable, until we can use the ? syntax for real :)).
chalin
force-pushed
the
chalin-homepage-src-1115
branch
from
7d17c1e
to
9a8ca6a
Compare
Per @mit-mit's request. Will be addressed in a followup PR.
chalin
force-pushed
the
chalin-homepage-src-1115
branch
from
9a8ca6a
to
375d38c
Compare
|
Travis is green, merging. Followup changes will be tracked via #2126. |
Staged, see:
Try Dart page: https://dart-dev-staging-0.firebaseapp.com/try-dartcc @mit-mit