Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(112)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1321683003: DevTools: Introduce timeline recording info dialog (Closed)

Created:
5 years, 3 months ago by alph
Modified:
5 years, 3 months ago
Reviewers:
caseq, pfeldman
CC:
blink-reviews, caseq+blink_chromium.org, yurys+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, sergeyv+blink_chromium.org, kozyatinskiy+blink_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

DevTools: Introduce timeline recording info dialog The rationale is: 1. Give the user more detailed information on what's going on. 2. Makes it more clear that the rest of UI is disabled during recording. 3. Fixes the problem with unintentional click to stop timeline starts a new timeline recording. BUG=525750 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=201452

Patch Set 1 #

Patch Set 2 : a tweak #

Patch Set 3 : 1/10sec -> sec #

Total comments: 2

Patch Set 4 : Use shadow DOM #

Patch Set 5 : Extract css into a file #

Total comments: 18

Patch Set 6 : addressing comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+237 lines, -42 lines) Patch
M Source/devtools/front_end/timeline/TimelinePanel.js View 1 2 3 4 5 8 chunks +144 lines, -38 lines 0 comments Download
M Source/devtools/front_end/timeline/module.json View 1 2 3 4 1 chunk +2 lines, -1 line 0 comments Download
A Source/devtools/front_end/timeline/timelineStatusDialog.css View 1 2 3 4 1 chunk +69 lines, -0 lines 0 comments Download
M Source/devtools/front_end/ui/Dialog.js View 1 2 3 4 5 5 chunks +22 lines, -3 lines 0 comments Download

Messages

Total messages: 16 (6 generated)
alph
5 years, 3 months ago (2015-08-27 20:42:08 UTC) #2
pfeldman
lgtm https://codereview.chromium.org/1321683003/diff/40001/Source/devtools/front_end/timeline/TimelinePanel.js File Source/devtools/front_end/timeline/TimelinePanel.js (right): https://codereview.chromium.org/1321683003/diff/40001/Source/devtools/front_end/timeline/TimelinePanel.js#newcode1754 Source/devtools/front_end/timeline/TimelinePanel.js:1754: this._status = this.element.createChild("div", "status"); Please use timeline prefix ...
5 years, 3 months ago (2015-08-27 22:22:13 UTC) #3
alph
ptal https://codereview.chromium.org/1321683003/diff/40001/Source/devtools/front_end/timeline/TimelinePanel.js File Source/devtools/front_end/timeline/TimelinePanel.js (right): https://codereview.chromium.org/1321683003/diff/40001/Source/devtools/front_end/timeline/TimelinePanel.js#newcode1754 Source/devtools/front_end/timeline/TimelinePanel.js:1754: this._status = this.element.createChild("div", "status"); On 2015/08/27 22:22:13, pfeldman ...
5 years, 3 months ago (2015-08-28 18:34:43 UTC) #4
pfeldman
https://codereview.chromium.org/1321683003/diff/80001/Source/devtools/front_end/timeline/TimelinePanel.js File Source/devtools/front_end/timeline/TimelinePanel.js (right): https://codereview.chromium.org/1321683003/diff/80001/Source/devtools/front_end/timeline/TimelinePanel.js#newcode671 Source/devtools/front_end/timeline/TimelinePanel.js:671: WebInspector.Dialog.currentInstance().focus(); Lets move this into show?
5 years, 3 months ago (2015-08-28 23:24:52 UTC) #5
caseq
https://codereview.chromium.org/1321683003/diff/80001/Source/devtools/front_end/timeline/TimelinePanel.js File Source/devtools/front_end/timeline/TimelinePanel.js (right): https://codereview.chromium.org/1321683003/diff/80001/Source/devtools/front_end/timeline/TimelinePanel.js#newcode669 Source/devtools/front_end/timeline/TimelinePanel.js:669: this._statusDialog.addEventListener(WebInspector.TimelinePanel.StatusDialog.Events.Finish, this._stopRecording, this); Can we make it a callback, ...
5 years, 3 months ago (2015-08-28 23:36:55 UTC) #7
alph
ptal https://codereview.chromium.org/1321683003/diff/80001/Source/devtools/front_end/timeline/TimelinePanel.js File Source/devtools/front_end/timeline/TimelinePanel.js (right): https://codereview.chromium.org/1321683003/diff/80001/Source/devtools/front_end/timeline/TimelinePanel.js#newcode669 Source/devtools/front_end/timeline/TimelinePanel.js:669: this._statusDialog.addEventListener(WebInspector.TimelinePanel.StatusDialog.Events.Finish, this._stopRecording, this); On 2015/08/28 23:36:55, caseq wrote: ...
5 years, 3 months ago (2015-08-29 00:49:38 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1321683003/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1321683003/100001
5 years, 3 months ago (2015-08-29 01:36:00 UTC) #11
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_asan_rel_ng/builds/46566)
5 years, 3 months ago (2015-08-29 03:08:54 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1321683003/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1321683003/100001
5 years, 3 months ago (2015-08-29 07:05:13 UTC) #15
commit-bot: I haz the power
5 years, 3 months ago (2015-08-29 08:36:54 UTC) #16
Message was sent while issue was closed. 
Committed patchset #6 (id:100001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=201452

Powered by Google App Engine
This is Rietveld 408576698