Make side panels auto-sized and resizeable.

Make side panels auto-sized and resizeable.

Currently, side panels hard-code their width, regardless of their contents.
This can waste space if side panels don't need all of their space,
or look unnecessarily cramped if side panels need more space than specified.

This CL automatically sets the side panel's width based on its contents when
it's created, and adds a drag handle to allow users to dynamically resize it,
so that side panels don't need to hard-code their width.

BUG=catapult:#2329

Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/849bc5d4ec90569bceaa29274fc80482947a94dd

[benjhayden, 2016-05-12 21:32:35]

benjhayden@chromium.org changed reviewers:
+ eakuefner@chromium.org, nednguyen@google.com

[benjhayden, 2016-05-12 21:32:36]

The auto-sizing doesn't work quite right yet, but the manual resizing works.

[benjhayden, 2016-05-12 21:40:13]

On 2016/05/12 at 21:32:36, benjhayden_chromium wrote:
> The auto-sizing doesn't work quite right yet, but the manual resizing works.

The auto-sizing works now, and the allowance for the vertical scrollbar is
corrected from 20 to 15.

[nednguyen, 2016-05-13 03:36:39]

nednguyen@google.com changed reviewers:
+ petrcermak@chromium.org

[nednguyen, 2016-05-13 03:36:40]

lgtm on the UI update, looks amazing!

Petr would know about the nitty gritty of tracing UI much better than me

https://codereview.chromium.org/1978673002/diff/20001/tracing/tracing/ui/side...
File tracing/tracing/ui/side_panel/metrics_side_panel.html (right):

https://codereview.chromium.org/1978673002/diff/20001/tracing/tracing/ui/side...
tracing/tracing/ui/side_panel/metrics_side_panel.html:103:
requestAnimationFrame(function() {
I thought the "tracing" way of doing this is tr.b.requestAnimationFrame ?

[petrcermak, 2016-05-13 09:23:17]

This is great!

However, the proposed implementation has a glitch: If you click on "Metrics"
multiple times, the metric side panel is opened multiple times (stacked
horizontally). I proposed a fix for this glitch in my inline comments ;-)

Apart from the above and Ned's comment about tr.b.requestAnimationFrame, LGTM!

Thanks,
Petr

https://codereview.chromium.org/1978673002/diff/20001/tracing/tracing/ui/side...
File tracing/tracing/ui/side_panel/metrics_side_panel.html (right):

https://codereview.chromium.org/1978673002/diff/20001/tracing/tracing/ui/side...
tracing/tracing/ui/side_panel/metrics_side_panel.html:103:
requestAnimationFrame(function() {
On 2016/05/13 03:36:40, nednguyen wrote:
> I thought the "tracing" way of doing this is tr.b.requestAnimationFrame ?

+1 (no need for bind then because tr.b.requestAnimationFrame has an opt_this
parameter)

https://codereview.chromium.org/1978673002/diff/20001/tracing/tracing/ui/side...
File tracing/tracing/ui/side_panel/side_panel_container.html (right):

https://codereview.chromium.org/1978673002/diff/20001/tracing/tracing/ui/side...
tracing/tracing/ui/side_panel/side_panel_container.html:155:
this.activePanelContainer_.removeChild(this.activePanel);
Why did you move this into the if-statement?

I'm pretty sure that this is causing the glitch I mentioned earlier. I modified
the code locally to:

  if (this.activePanelType) {
    this.getLabelElementForPanelType_(
        this.activePanelType).removeAttribute('selected');
  }

  if (this.activePanel)
    this.activePanelContainer_.removeChild(this.activePanel);

and I wasn't able to reproduce the glitch anymore.

https://codereview.chromium.org/1978673002/diff/20001/tracing/tracing/ui/valu...
File tracing/tracing/ui/value_set_view.html (right):

https://codereview.chromium.org/1978673002/diff/20001/tracing/tracing/ui/valu...
tracing/tracing/ui/value_set_view.html:108: requestAnimationFrame(function() {
ditto tr.b.requestAnimationFrame

[benjhayden, 2016-05-13 18:42:09]

I wasn't able to repro the glitch either with or without petr's fix, so I kept
the fix just in case there was a glitch.

Thanks for the review!

[benjhayden, 2016-05-13 18:45:01]

Description was changed from

==========
Make side panels resizeable

BUG=catapult:#2329
==========

to

==========
Make side panels resizeable

Currently, side panels hard-code their width, regardless of their contents.
This can waste space if side panels don't need all of their space,
or look unnecessarily cramped if side panels need more space than specified.

This CL automatically sets the side panel's width based on its contents when
it's created, and adds a drag handle to allow users to dynamically resize it,
so that side panels don't need to hard-code their width.

BUG=catapult:#2329
==========

[benjhayden, 2016-05-13 18:45:30]

Description was changed from

==========
Make side panels resizeable

Currently, side panels hard-code their width, regardless of their contents.
This can waste space if side panels don't need all of their space,
or look unnecessarily cramped if side panels need more space than specified.

This CL automatically sets the side panel's width based on its contents when
it's created, and adds a drag handle to allow users to dynamically resize it,
so that side panels don't need to hard-code their width.

BUG=catapult:#2329
==========

to

==========
Make side panels auto-sized and resizeable.

Currently, side panels hard-code their width, regardless of their contents.
This can waste space if side panels don't need all of their space,
or look unnecessarily cramped if side panels need more space than specified.

This CL automatically sets the side panel's width based on its contents when
it's created, and adds a drag handle to allow users to dynamically resize it,
so that side panels don't need to hard-code their width.

BUG=catapult:#2329
==========

[benjhayden, 2016-05-13 18:46:23]

The CQ bit was checked by benjhayden@chromium.org

[benjhayden, 2016-05-13 18:46:23]

The patchset sent to the CQ was uploaded after l-g-t-m from
nednguyen@google.com, petrcermak@chromium.org
Link to the patchset: https://codereview.chromium.org/1978673002/#ps60001
(title: "fix glitch?")

[commit-bot: I haz the power, 2016-05-13 18:46:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1978673002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1978673002/60001

[commit-bot: I haz the power, 2016-05-13 19:08:01]

Description was changed from

==========
Make side panels auto-sized and resizeable.

Currently, side panels hard-code their width, regardless of their contents.
This can waste space if side panels don't need all of their space,
or look unnecessarily cramped if side panels need more space than specified.

This CL automatically sets the side panel's width based on its contents when
it's created, and adds a drag handle to allow users to dynamically resize it,
so that side panels don't need to hard-code their width.

BUG=catapult:#2329
==========

to

==========
Make side panels auto-sized and resizeable.

Currently, side panels hard-code their width, regardless of their contents.
This can waste space if side panels don't need all of their space,
or look unnecessarily cramped if side panels need more space than specified.

This CL automatically sets the side panel's width based on its contents when
it's created, and adds a drag handle to allow users to dynamically resize it,
so that side panels don't need to hard-code their width.

BUG=catapult:#2329

Committed:
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...
==========

[commit-bot: I haz the power, 2016-05-13 19:08:02]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...