[DevTools] Start/stop of timeline recording should set UI controls disabled/enabled respectively.

[DevTools] Start/stop of timeline recording should set UI controls disabled/enabled respectively.

Once timeline recording starts, the state of "Capture ..." checkboxes has no effect on the recording behavior.
These controls should be disabled upon the start of recording and reset back upon completion of the recording.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=177490

[vivekg, 2014-07-02 07:00:43]

PTAL, thank you!

[loislo, 2014-07-03 06:24:34]

On 2014/07/02 07:00:43, vivekg_ wrote:
> PTAL, thank you!

I'd also gray the label elements because there is almost no difference between
empty checkbox and disabled empty checkbox.

[aandrey, 2014-07-03 08:00:49]

https://codereview.chromium.org/361203002/diff/1/Source/devtools/front_end/ti...
File Source/devtools/front_end/timeline/TimelinePanel.js (right):

https://codereview.chromium.org/361203002/diff/1/Source/devtools/front_end/ti...
Source/devtools/front_end/timeline/TimelinePanel.js:333:
this._captureStacksCheckbox = document.createElement("input");
I'd create an array this._uiControlsCheckboxes, push all input elements into it
and later iterate over the array to disable/enable all elements.

[vivekg, 2014-07-03 08:10:33]

On 2014/07/03 08:00:49, aandrey wrote:
>
https://codereview.chromium.org/361203002/diff/1/Source/devtools/front_end/ti...
> File Source/devtools/front_end/timeline/TimelinePanel.js (right):
> 
>
https://codereview.chromium.org/361203002/diff/1/Source/devtools/front_end/ti...
> Source/devtools/front_end/timeline/TimelinePanel.js:333:
> this._captureStacksCheckbox = document.createElement("input");
> I'd create an array this._uiControlsCheckboxes, push all input elements into
it
> and later iterate over the array to disable/enable all elements.

Thank you for the suggestion. I was exactly doing the same thing along with the
labels by the time you were commenting. :) 

PTAL at the new patch set.

[aandrey, 2014-07-03 08:25:37]

https://codereview.chromium.org/361203002/diff/20001/Source/devtools/front_en...
File Source/devtools/front_end/timeline/TimelinePanel.js (right):

https://codereview.chromium.org/361203002/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/timeline/TimelinePanel.js:378:
topPaneSidebarElement.appendChild(captureLayersAndPicturesLabel);
now let's extract common code to a method, like:
this._createSettingCheckbox(WebInspector.UIString("Capture pictures"),
this._captureLayersAndPicturesSetting, WebInspector.UIString("Capture graphics
layer positions and painted pictures"));

[vivekg, 2014-07-03 10:36:22]

Thank you. Uploaded new patch PTAL.

https://codereview.chromium.org/361203002/diff/20001/Source/devtools/front_en...
File Source/devtools/front_end/timeline/TimelinePanel.js (right):

https://codereview.chromium.org/361203002/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/timeline/TimelinePanel.js:378:
topPaneSidebarElement.appendChild(captureLayersAndPicturesLabel);
On 2014/07/03 08:25:37, aandrey wrote:
> now let's extract common code to a method, like:
> this._createSettingCheckbox(WebInspector.UIString("Capture pictures"),
> this._captureLayersAndPicturesSetting, WebInspector.UIString("Capture graphics
> layer positions and painted pictures"));

Done.

[aandrey, 2014-07-03 11:24:33]

looks good to me.

@loislo, can you do a final look?

https://codereview.chromium.org/361203002/diff/60001/Source/devtools/front_en...
File Source/devtools/front_end/timeline/TimelinePanel.js (right):

https://codereview.chromium.org/361203002/diff/60001/Source/devtools/front_en...
Source/devtools/front_end/timeline/TimelinePanel.js:331: * @param {string=}
tooltip
{string=} -> {string}

https://codereview.chromium.org/361203002/diff/60001/Source/devtools/front_en...
Source/devtools/front_end/timeline/TimelinePanel.js:688:
this._recordingOptionUIControls.forEach(function(uiControl) {
we generally don't inline function callbacks. i.e.:

/**
 * @param {{label: !Element, checkbox: !Element}} uiControl
 */
function handler(uiControl)
{
   //...
}
this._recordingOptionUIControls.forEach(handler);

https://codereview.chromium.org/361203002/diff/60001/Source/devtools/front_en...
Source/devtools/front_end/timeline/TimelinePanel.js:690:
uiControl.label.classList.toggle("disabled-label", disabled);
nit: we already use "dimmed" class.

https://codereview.chromium.org/361203002/diff/60001/Source/devtools/front_en...
File Source/devtools/front_end/timelinePanel.css (right):

https://codereview.chromium.org/361203002/diff/60001/Source/devtools/front_en...
Source/devtools/front_end/timelinePanel.css:816: .disabled-label {
label.dimmed

[loislo, 2014-07-03 11:33:38]

https://codereview.chromium.org/361203002/diff/60001/Source/devtools/front_en...
File Source/devtools/front_end/timeline/TimelinePanel.js (right):

https://codereview.chromium.org/361203002/diff/60001/Source/devtools/front_en...
Source/devtools/front_end/timeline/TimelinePanel.js:339: checkboxElement =
document.createElement("input");
var checkboxElement

[vivekg, 2014-07-03 11:38:36]

On 2014/07/03 11:33:38, loislo wrote:
>
https://codereview.chromium.org/361203002/diff/60001/Source/devtools/front_en...
> File Source/devtools/front_end/timeline/TimelinePanel.js (right):
> 
>
https://codereview.chromium.org/361203002/diff/60001/Source/devtools/front_en...
> Source/devtools/front_end/timeline/TimelinePanel.js:339: checkboxElement =
> document.createElement("input");
> var checkboxElement

Done. Thank you!

[loislo, 2014-07-03 13:00:48]

lgtm with nits

https://codereview.chromium.org/361203002/diff/80001/Source/devtools/front_en...
File Source/devtools/front_end/timeline/TimelinePanel.js (right):

https://codereview.chromium.org/361203002/diff/80001/Source/devtools/front_en...
Source/devtools/front_end/timeline/TimelinePanel.js:712:
this._disableUIControls(true);
Please change the name to positive logic: this._setUIControlsEnabled

[aandrey, 2014-07-03 13:07:04]

lgtm

[vivekg, 2014-07-03 18:35:21]

On 2014/07/03 13:00:48, loislo wrote:
> lgtm with nits
> 
>
https://codereview.chromium.org/361203002/diff/80001/Source/devtools/front_en...
> File Source/devtools/front_end/timeline/TimelinePanel.js (right):
> 
>
https://codereview.chromium.org/361203002/diff/80001/Source/devtools/front_en...
> Source/devtools/front_end/timeline/TimelinePanel.js:712:
> this._disableUIControls(true);
> Please change the name to positive logic: this._setUIControlsEnabled

Done. Thank you!

[vivekg, 2014-07-03 18:35:26]

The CQ bit was checked by vivekg@chromium.org

[commit-bot: I haz the power, 2014-07-03 18:36:27]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/vivek.vg@samsung.com/361203002/90001

[commit-bot: I haz the power, 2014-07-03 19:38:56]

Change committed as 177490