Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(12)

Issue 2773583002: [DevTools] Introduce a sidebar with a drop-down

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 months, 4 weeks ago by eostroukhov
Modified:
4 months, 3 weeks ago
Reviewers:
caseq, dgozman
CC:
apavlov+blink_chromium.org, blink-reviews, caseq+blink_chromium.org, chromium-reviews, devtools-reviews_chromium.org, kozyatinskiy+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org, pfeldman
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[DevTools] Introduce a sidebar with a drop-down Introduces a frames dropdown and a new sidebar implementation that uses this drop-down. New sidebar is currently behind the experiment. BUG=698027

Patch Set 1 #

Patch Set 2 : [DevTools] Introduce a sidebar with a drop-down #

Total comments: 22

Patch Set 3 : [DevTools] Introduce a sidebar with a drop-down #

Total comments: 30

Patch Set 4 : [DevTools] Introduce a sidebar with a drop-down #

Total comments: 6

Patch Set 5 : [DevTools] Introduce a sidebar with a drop-down #

Total comments: 6

Patch Set 6 : [DevTools] Introduce a sidebar with a drop-down #

Patch Set 7 : [DevTools] Introduce a sidebar with a drop-down #

Total comments: 5
Unified diffs Side-by-side diffs Delta from patch set Stats (+439 lines, -41 lines) Patch
M third_party/WebKit/Source/devtools/BUILD.gn View 1 2 3 4 5 6 1 chunk +4 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/devtools/front_end/main/Main.js View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/devtools/front_end/quick_open/FilteredListWidget.js View 1 2 3 4 5 11 chunks +70 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/devtools/front_end/quick_open/filteredListWidget.css View 1 2 3 4 2 chunks +20 lines, -2 lines 0 comments Download
A third_party/WebKit/Source/devtools/front_end/resources/FrameMenu.js View 1 2 3 4 1 chunk +196 lines, -0 lines 2 comments Download
A third_party/WebKit/Source/devtools/front_end/resources/FrameMenuSidebar.js View 1 2 3 1 chunk +26 lines, -0 lines 1 comment Download
M third_party/WebKit/Source/devtools/front_end/resources/ResourcesPanel.js View 1 2 3 4 5 6 1 chunk +4 lines, -1 line 0 comments Download
A third_party/WebKit/Source/devtools/front_end/resources/frameMenu.css View 1 2 3 1 chunk +14 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/devtools/front_end/resources/frameMenuSidebar.css View 1 2 1 chunk +33 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/devtools/front_end/resources/module.json View 1 2 3 chunks +6 lines, -1 line 0 comments Download
M third_party/WebKit/Source/devtools/front_end/sdk/ResourceTreeModel.js View 1 2 3 4 5 6 2 chunks +65 lines, -32 lines 2 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 41 (29 generated)
eostroukhov
Please take a look. This is initial commit for this new functionality, I will be ...
6 months, 3 weeks ago (2017-03-23 23:35:12 UTC) #6
dgozman
https://codereview.chromium.org/2773583002/diff/20001/third_party/WebKit/Source/devtools/front_end/main/Main.js File third_party/WebKit/Source/devtools/front_end/main/Main.js (right): https://codereview.chromium.org/2773583002/diff/20001/third_party/WebKit/Source/devtools/front_end/main/Main.js#newcode102 third_party/WebKit/Source/devtools/front_end/main/Main.js:102: Runtime.experiments.register('framesSelectorAppPanel', 'Show frames selector on the Application panel', true); ...
6 months, 3 weeks ago (2017-03-24 17:06:55 UTC) #7
dgozman
Please include a screenshot either here or in the bug.
6 months, 3 weeks ago (2017-03-24 17:07:38 UTC) #8
eostroukhov
Redid the UX, moved to filtered list. Please take another look. https://codereview.chromium.org/2773583002/diff/20001/third_party/WebKit/Source/devtools/front_end/main/Main.js File third_party/WebKit/Source/devtools/front_end/main/Main.js (right): ...
6 months, 2 weeks ago (2017-04-04 23:47:17 UTC) #11
dgozman
https://codereview.chromium.org/2773583002/diff/40001/third_party/WebKit/Source/devtools/front_end/quick_open/FilteredListWidget.js File third_party/WebKit/Source/devtools/front_end/quick_open/FilteredListWidget.js (right): https://codereview.chromium.org/2773583002/diff/40001/third_party/WebKit/Source/devtools/front_end/quick_open/FilteredListWidget.js#newcode195 third_party/WebKit/Source/devtools/front_end/quick_open/FilteredListWidget.js:195: var element = event.deepElementFromPoint(); Implement this similarly to click? ...
6 months, 2 weeks ago (2017-04-05 20:52:56 UTC) #14
eostroukhov
Thank you for your review. A new version was uploaded, please take another look. https://codereview.chromium.org/2773583002/diff/40001/third_party/WebKit/Source/devtools/front_end/quick_open/FilteredListWidget.js ...
5 months, 2 weeks ago (2017-05-03 00:33:47 UTC) #16
dgozman
+Joel for FilteredListWidget, who is also working on similar thing for console. https://codereview.chromium.org/2773583002/diff/60001/third_party/WebKit/Source/devtools/front_end/quick_open/FilteredListWidget.js File third_party/WebKit/Source/devtools/front_end/quick_open/FilteredListWidget.js ...
5 months, 2 weeks ago (2017-05-03 17:32:58 UTC) #21
eostroukhov
I've uploaded a new version. Please take another look. https://codereview.chromium.org/2773583002/diff/60001/third_party/WebKit/Source/devtools/front_end/quick_open/FilteredListWidget.js File third_party/WebKit/Source/devtools/front_end/quick_open/FilteredListWidget.js (right): https://codereview.chromium.org/2773583002/diff/60001/third_party/WebKit/Source/devtools/front_end/quick_open/FilteredListWidget.js#newcode310 third_party/WebKit/Source/devtools/front_end/quick_open/FilteredListWidget.js:310: ...
5 months, 2 weeks ago (2017-05-03 22:01:38 UTC) #24
einbinder
https://codereview.chromium.org/2773583002/diff/80001/third_party/WebKit/Source/devtools/front_end/quick_open/FilteredListWidget.js File third_party/WebKit/Source/devtools/front_end/quick_open/FilteredListWidget.js (right): https://codereview.chromium.org/2773583002/diff/80001/third_party/WebKit/Source/devtools/front_end/quick_open/FilteredListWidget.js#newcode43 third_party/WebKit/Source/devtools/front_end/quick_open/FilteredListWidget.js:43: this._itemElementsContainer.addEventListener('mousemove', event => this._updateHover(event)); I think if you use ...
5 months, 2 weeks ago (2017-05-04 07:05:10 UTC) #27
eostroukhov
Thank you for the review. It is ready for another pass now :) https://codereview.chromium.org/2773583002/diff/80001/third_party/WebKit/Source/devtools/front_end/quick_open/FilteredListWidget.js File ...
5 months, 2 weeks ago (2017-05-04 17:55:52 UTC) #30
eostroukhov
This CL is again up for consideration. Please take a look.
4 months, 3 weeks ago (2017-05-24 21:57:18 UTC) #37
caseq
4 months, 3 weeks ago (2017-05-26 22:20:19 UTC) #41
https://codereview.chromium.org/2773583002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/resources/FrameMenu.js
(right):

https://codereview.chromium.org/2773583002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/resources/FrameMenu.js:66: var list
= new QuickOpen.FilteredListWidget(provider);
I think this should use a different control, preferably be consistent with
console execution context selector.

https://codereview.chromium.org/2773583002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/resources/FrameMenu.js:97:
Resources.FrameMenu._ListItem = class {
nit: convert to typedef?

https://codereview.chromium.org/2773583002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/resources/FrameMenuSidebar.js
(right):

https://codereview.chromium.org/2773583002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/resources/FrameMenuSidebar.js:7:
super();
Make it shadow, i.e. super(true)?

https://codereview.chromium.org/2773583002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/sdk/ResourceTreeModel.js
(right):

https://codereview.chromium.org/2773583002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/sdk/ResourceTreeModel.js:103: *
@param {?SDK.ResourceTreeFrame} a
Should it be nullable?

https://codereview.chromium.org/2773583002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/sdk/ResourceTreeModel.js:108: var
framesA = a ? SDK.ResourceTreeModel.framePath(a) : [];
How about this:

var framePathA = SDK.ResourceTreeModel.framePath(a).map(_ => _.id).join('\1');
var framePathB = SDK.ResourceTreeModel.framePath(b).map(_ => _.id).join('\1');

return framePathA.localeCompare(framePathB)
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 81bcdb8aa