Issue 185943004: DevTools: Do not save split view size on show mode changes.

Issue 185943004: DevTools: Do not save split view size on show mode changes. (Closed)

Created:
6 years, 9 months ago by vsevik

Modified:
6 years, 9 months ago

Reviewers:
dgozman, pfeldman

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:
svn://svn.chromium.org/blink/trunk

Visibility:
Public.

More Reviews

Description

DevTools: Do not save split view size on show mode changes. BUG=348887 R=dgozman@chromium.org, dgozman Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=168608 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=168775

Patch Set 1 #

Total comments: 2

Patch Set 2 : Rebaselined #

Patch Set 3 : Comments addressed, test fixed #

Patch Set 4 : Fixed an issue, introduced a split view test #

Total comments: 5

Patch Set 5 : Comments addressed #

Total comments: 1

Patch Set 6 : Removed unneeded wasShown handling #

Patch Set 7 : Rebaselined #

Patch Set 8 : Rebaselined #

Created: 6 years, 9 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+379 lines, -108 lines)			Patch
M	LayoutTests/http/tests/inspector/network/network-sidebar-width.html	View	1 2 3	1 chunk	+0 lines, -49 lines	0 comments	Download
D	LayoutTests/http/tests/inspector/network/network-sidebar-width-expected.txt	View	1 2 3	1 chunk	+0 lines, -7 lines	0 comments	Download
A	LayoutTests/inspector/split-view.html	View	1 2 3 4	1 chunk	+126 lines, -0 lines	0 comments	Download
A	LayoutTests/inspector/split-view-expected.txt	View	1 2 3 4	1 chunk	+195 lines, -0 lines	0 comments	Download
M	Source/devtools/front_end/InspectedPagePlaceholder.js	View	1 2 3 4	1 chunk	+1 line, -1 line	0 comments	Download
M	Source/devtools/front_end/SplitView.js	View	1 2 3 4 5	11 chunks	+57 lines, -51 lines	0 comments	Download

Messages

Total messages: 15 (0 generated)

Expand Messages | Collapse Messages

dgozman

lgtm https://codereview.chromium.org/185943004/diff/1/Source/devtools/front_end/SplitView.js File Source/devtools/front_end/SplitView.js (right): https://codereview.chromium.org/185943004/diff/1/Source/devtools/front_end/SplitView.js#newcode632 Source/devtools/front_end/SplitView.js:632: this._saveSizeToSetting(); We agreed this is not needed. https://codereview.chromium.org/185943004/diff/1/Source/devtools/front_end/SplitView.js#newcode763 ...

6 years, 9 months ago (2014-03-05 17:52:53 UTC) #2

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vsevik@chromium.org/185943004/40001

6 years, 9 months ago (2014-03-06 06:04:32 UTC) #4

pfeldman

A revert of this CL has been created in https://codereview.chromium.org/185063005/ by pfeldman@chromium.org. The reason for ...

6 years, 9 months ago (2014-03-06 11:50:48 UTC) #6

dgozman

I think this is getting too complicated with all saved, preferred, update, restore, etc. Can ...

6 years, 9 months ago (2014-03-06 17:53:35 UTC) #8

dgozman

lgtm https://codereview.chromium.org/185943004/diff/80001/Source/devtools/front_end/SplitView.js File Source/devtools/front_end/SplitView.js (right): https://codereview.chromium.org/185943004/diff/80001/Source/devtools/front_end/SplitView.js#newcode600 Source/devtools/front_end/SplitView.js:600: this._restoreSidebarSizeFromSettings(); Remove this.

6 years, 9 months ago (2014-03-07 11:32:52 UTC) #10

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vsevik@chromium.org/185943004/140001

6 years, 9 months ago (2014-03-08 08:39:59 UTC) #12

commit-bot: I haz the power

The CQ bit was unchecked by commit-bot@chromium.org

6 years, 9 months ago (2014-03-08 09:56:46 UTC) #13

commit-bot: I haz the power

Try jobs failed on following builders: win_blink_rel

6 years, 9 months ago (2014-03-08 09:56:47 UTC) #14

vsevik

6 years, 9 months ago (2014-03-08 10:02:17 UTC) #15

Message was sent while issue was closed.

Committed patchset #8 manually as r168775 (presubmit successful).

Expand Messages | Collapse Messages