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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 186763002: DevTools: Small refactoring of InspectorView (Closed)

Created:
6 years, 9 months ago by sergeyv
Modified:
6 years, 9 months ago
Reviewers:
vsevik
CC:
blink-reviews, caseq+blink_chromium.org, loislo+blink_chromium.org, eustas+blink_chromium.org, alph+blink_chromium.org, yurys+blink_chromium.org, lushnikov+blink_chromium.org, vsevik+blink_chromium.org, pfeldman+blink_chromium.org, paulirish+reviews_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, aandrey+blink_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

DevTools: Small refactoring of InspectorView BUG= Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=168569

Patch Set 1 #

Total comments: 12

Patch Set 2 : Address vsevik's comments #

Patch Set 3 : Change enableStyleClass on toggle #

Total comments: 1

Patch Set 4 : Address vsevik's comments #2 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+44 lines, -88 lines) Patch
M Source/devtools/front_end/InspectorView.js View 1 2 3 2 chunks +44 lines, -88 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
sergeyv
6 years, 9 months ago (2014-03-04 17:03:49 UTC) #1
vsevik
https://codereview.chromium.org/186763002/diff/1/Source/devtools/front_end/InspectorView.js File Source/devtools/front_end/InspectorView.js (right): https://codereview.chromium.org/186763002/diff/1/Source/devtools/front_end/InspectorView.js#newcode344 Source/devtools/front_end/InspectorView.js:344: _handleHistoryMoveShortcut: function(event, move) I would inline event handling and ...
6 years, 9 months ago (2014-03-04 17:29:55 UTC) #2
sergeyv
https://codereview.chromium.org/186763002/diff/1/Source/devtools/front_end/InspectorView.js File Source/devtools/front_end/InspectorView.js (right): https://codereview.chromium.org/186763002/diff/1/Source/devtools/front_end/InspectorView.js#newcode344 Source/devtools/front_end/InspectorView.js:344: _handleHistoryMoveShortcut: function(event, move) On 2014/03/04 17:29:56, vsevik wrote: > ...
6 years, 9 months ago (2014-03-04 17:55:42 UTC) #3
vsevik
lgtm https://chromiumcodereview.appspot.com/186763002/diff/40001/Source/devtools/front_end/InspectorView.js File Source/devtools/front_end/InspectorView.js (right): https://chromiumcodereview.appspot.com/186763002/diff/40001/Source/devtools/front_end/InspectorView.js#newcode330 Source/devtools/front_end/InspectorView.js:330: var direction = this._openBracketIdentifiers[event.keyIdentifier] ? -1 : 0; ...
6 years, 9 months ago (2014-03-04 18:02:48 UTC) #4
sergeyv
The CQ bit was checked by sergeyv@chromium.org
6 years, 9 months ago (2014-03-05 09:34:02 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sergeyv@chromium.org/186763002/60001
6 years, 9 months ago (2014-03-05 09:34:18 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sergeyv@chromium.org/186763002/60001
6 years, 9 months ago (2014-03-05 20:57:41 UTC) #7
commit-bot: I haz the power
6 years, 9 months ago (2014-03-06 04:16:14 UTC) #8
Message was sent while issue was closed. 
Change committed as 168569

Powered by Google App Engine
This is Rietveld 408576698