Converted Observatory isolate-nav-menu element

Converted Observatory isolate-nav-menu element

R=johnmccutchan@google.com, rmacnak@google.com

Committed: https://github.com/dart-lang/sdk/commit/3ccc1f337fa7b5fe7546069146b874af555c5f05

[cbernaschina, 2016-07-20 03:07:55]

cbernaschina@google.com changed reviewers:
+ johnmccutchan@google.com, rmacnak@google.com, turnidge@google.com

[Cutch, 2016-07-20 13:55:26]

https://codereview.chromium.org/2168463004/diff/20001/runtime/observatory/lib...
File runtime/observatory/lib/src/elements/nav/isolate_menu.dart (right):

https://codereview.chromium.org/2168463004/diff/20001/runtime/observatory/lib...
runtime/observatory/lib/src/elements/nav/isolate_menu.dart:32: _last = value;
this block of code for setting a bool (and marking as dirty / schedule
notification) is repeated a lot. Perhaps make a helper routine.

https://codereview.chromium.org/2168463004/diff/20001/runtime/observatory/lib...
runtime/observatory/lib/src/elements/nav/isolate_menu.dart:40:
Stream<M.IsolateUpdateEvent> updates, {bool last: false,
indentation is wrong here

https://codereview.chromium.org/2168463004/diff/20001/runtime/observatory/lib...
runtime/observatory/lib/src/elements/nav/isolate_menu.dart:79: var url_tail =
'?isolateId=${Uri.encodeComponent(isolate.id)}';
Consider using the Uri class to build your uris.

https://codereview.chromium.org/2168463004/diff/20001/runtime/observatory/lib...
runtime/observatory/lib/src/elements/nav/isolate_menu.dart:85: link:
'/debugger$url_tail'),
Use the Uri class instead of hand written concatenation.

[cbernaschina, 2016-07-20 20:01:42]

https://codereview.chromium.org/2168463004/diff/20001/runtime/observatory/lib...
File runtime/observatory/lib/src/elements/nav/isolate_menu.dart (right):

https://codereview.chromium.org/2168463004/diff/20001/runtime/observatory/lib...
runtime/observatory/lib/src/elements/nav/isolate_menu.dart:32: _last = value;
On 2016/07/20 13:55:25, Cutch wrote:
> this block of code for setting a bool (and marking as dirty / schedule
> notification) is repeated a lot. Perhaps make a helper routine.

Done.

https://codereview.chromium.org/2168463004/diff/20001/runtime/observatory/lib...
runtime/observatory/lib/src/elements/nav/isolate_menu.dart:40:
Stream<M.IsolateUpdateEvent> updates, {bool last: false,
On 2016/07/20 13:55:25, Cutch wrote:
> indentation is wrong here

Done.

https://codereview.chromium.org/2168463004/diff/20001/runtime/observatory/lib...
runtime/observatory/lib/src/elements/nav/isolate_menu.dart:79: var url_tail =
'?isolateId=${Uri.encodeComponent(isolate.id)}';
On 2016/07/20 13:55:25, Cutch wrote:
> Consider using the Uri class to build your uris.

Done.

https://codereview.chromium.org/2168463004/diff/20001/runtime/observatory/lib...
runtime/observatory/lib/src/elements/nav/isolate_menu.dart:85: link:
'/debugger$url_tail'),
On 2016/07/20 13:55:25, Cutch wrote:
> Use the Uri class instead of hand written concatenation. 

Done.

[Cutch, 2016-07-22 13:38:18]

LGTM after fixing the issues around links.

https://codereview.chromium.org/2168463004/diff/180001/runtime/observatory/li...
File runtime/observatory/lib/src/elements/nav/menu_item_wrapper.dart (right):

https://codereview.chromium.org/2168463004/diff/180001/runtime/observatory/li...
runtime/observatory/lib/src/elements/nav/menu_item_wrapper.dart:49: new
NavMenuItemElement(_anchor, link: '#$link',
why is the # character hard coded here?

https://codereview.chromium.org/2168463004/diff/180001/runtime/observatory/li...
File runtime/observatory/lib/src/elements/nav/menu_wrapper.dart (right):

https://codereview.chromium.org/2168463004/diff/180001/runtime/observatory/li...
runtime/observatory/lib/src/elements/nav/menu_wrapper.dart:56: new
NavMenuElement(_anchor, link: '#$_link', last: last,
same question

https://codereview.chromium.org/2168463004/diff/180001/runtime/observatory/li...
File runtime/observatory/lib/src/elements/nav/top_menu.dart (right):

https://codereview.chromium.org/2168463004/diff/180001/runtime/observatory/li...
runtime/observatory/lib/src/elements/nav/top_menu.dart:51: new
NavMenuElement('Observatory', link: Uris.vm(), last: last,
might want to make these into getters so you can just write:

link: Uris.vm,

[cbernaschina, 2016-07-22 17:26:43]

https://codereview.chromium.org/2168463004/diff/180001/runtime/observatory/li...
File runtime/observatory/lib/src/elements/nav/menu_item_wrapper.dart (right):

https://codereview.chromium.org/2168463004/diff/180001/runtime/observatory/li...
runtime/observatory/lib/src/elements/nav/menu_item_wrapper.dart:49: new
NavMenuItemElement(_anchor, link: '#$link',
On 2016/07/22 13:38:17, Cutch wrote:
> why is the # character hard coded here? 

After the introduction of the Uris helper we have a "backward compatibility"
issue.
Differently from the rest of the application the menu and menu-item are
generally initialized with link="/path" and not link="#/path".
Due to the fact that the Uris class adds the # on its own we cannot add it in
the real object. If we do not add it here though, all the templates that are
using the wrapper will miss the # in the links.
When the wrapper will be removed all the links will be created by Uris classes
and we will not need it anymore.

https://codereview.chromium.org/2168463004/diff/180001/runtime/observatory/li...
File runtime/observatory/lib/src/elements/nav/menu_wrapper.dart (right):

https://codereview.chromium.org/2168463004/diff/180001/runtime/observatory/li...
runtime/observatory/lib/src/elements/nav/menu_wrapper.dart:56: new
NavMenuElement(_anchor, link: '#$_link', last: last,
On 2016/07/22 13:38:17, Cutch wrote:
> same question

same answer

https://codereview.chromium.org/2168463004/diff/180001/runtime/observatory/li...
File runtime/observatory/lib/src/elements/nav/top_menu.dart (right):

https://codereview.chromium.org/2168463004/diff/180001/runtime/observatory/li...
runtime/observatory/lib/src/elements/nav/top_menu.dart:51: new
NavMenuElement('Observatory', link: Uris.vm(), last: last,
On 2016/07/22 13:38:17, Cutch wrote:
> might want to make these into getters so you can just write:
> 
> link: Uris.vm,

It was mainly for "uniformity" and as a "reserved for future use".
If in the future we would like to add an optional parameter to it we will have
to edit all the code that uses it.

[rmacnak, 2016-07-22 21:15:54]

lgtm

https://chromiumcodereview.appspot.com/2168463004/diff/180001/runtime/observa...
File runtime/observatory/lib/src/mocks/objects/isolate.dart (right):

https://chromiumcodereview.appspot.com/2168463004/diff/180001/runtime/observa...
runtime/observatory/lib/src/mocks/objects/isolate.dart:22: const
IsolateMock({this.id, this.number, this.name, this.startTime, this.runnable});
long line

[cbernaschina, 2016-07-22 21:18:23]

https://chromiumcodereview.appspot.com/2168463004/diff/180001/runtime/observa...
File runtime/observatory/lib/src/mocks/objects/isolate.dart (right):

https://chromiumcodereview.appspot.com/2168463004/diff/180001/runtime/observa...
runtime/observatory/lib/src/mocks/objects/isolate.dart:22: const
IsolateMock({this.id, this.number, this.name, this.startTime, this.runnable});
On 2016/07/22 21:15:53, rmacnak wrote:
> long line

Done.

[cbernaschina, 2016-07-22 21:27:42]

Description was changed from

==========
Converted Observatory isolate-nav-menu element
==========

to

==========
Converted Observatory isolate-nav-menu element

R=johnmccutchan@google.com, rmacnak@google.com

Committed:
https://github.com/dart-lang/sdk/commit/3ccc1f337fa7b5fe7546069146b874af555c5f05
==========

[cbernaschina, 2016-07-22 21:27:44]

Committed patchset #11 (id:200001) manually as
3ccc1f337fa7b5fe7546069146b874af555c5f05 (presubmit successful).