DevTools: allow editing custom color palette, move it out of experimental.

DevTools: allow editing custom color palette, move it out of experimental.

NOTRY=true

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

[pfeldman, 2015-08-20 22:38:39]

pfeldman@chromium.org changed reviewers:
+ dgozman@chromium.org

[dgozman, 2015-08-21 01:11:29]

lgtm with comments

https://codereview.chromium.org/1308563002/diff/30001/Source/devtools/front_e...
File Source/devtools/front_end/elements/Spectrum.js (right):

https://codereview.chromium.org/1308563002/diff/30001/Source/devtools/front_e...
Source/devtools/front_end/elements/Spectrum.js:245: colorElement.__mutable =
true;
Could use |this._paletteContainerMutable| instead.

https://codereview.chromium.org/1308563002/diff/30001/Source/devtools/front_e...
Source/devtools/front_end/elements/Spectrum.js:277: var col = localX / 24 % 8 |
0;
nit: add parenthesis for consistency.

https://codereview.chromium.org/1308563002/diff/30001/Source/devtools/front_e...
Source/devtools/front_end/elements/Spectrum.js:277: var col = localX / 24 % 8 |
0;
Why %8, and not Math.min(, 7)?

https://codereview.chromium.org/1308563002/diff/30001/Source/devtools/front_e...
Source/devtools/front_end/elements/Spectrum.js:278: var row = (localY / 24) | 0;
24 to named constant!

https://codereview.chromium.org/1308563002/diff/30001/Source/devtools/front_e...
Source/devtools/front_end/elements/Spectrum.js:279: return Math.min(row * 8 +
col, this._paletteContainer.children.length - 2);
Let's move add button out of container.

https://codereview.chromium.org/1308563002/diff/30001/Source/devtools/front_e...
Source/devtools/front_end/elements/Spectrum.js:316:
this._paletteContainer.insertBefore(this._dragElement, children[newIndex > index
? newIndex + 1 : newIndex]);
nit: weird indent. Presubmit?

https://codereview.chromium.org/1308563002/diff/30001/Source/devtools/front_e...
Source/devtools/front_end/elements/Spectrum.js:415:
contextMenu.appendItem(WebInspector.UIString("Remove color"),
this._deletePaletteColors.bind(this, colorIndex, false));
We use |UIString.capitalize| in context menus.

[pfeldman, 2015-08-21 01:34:23]

all comments addressed except for capitalize.

[pfeldman, 2015-08-21 01:35:34]

The CQ bit was checked by pfeldman@chromium.org

[pfeldman, 2015-08-21 01:35:34]

The patchset sent to the CQ was uploaded after l-g-t-m from dgozman@chromium.org
Link to the patchset: https://codereview.chromium.org/1308563002/#ps50001
(title: " ")

[commit-bot: I haz the power, 2015-08-21 01:35:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1308563002/50001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1308563002/50001

[commit-bot: I haz the power, 2015-08-21 01:37:32]

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

[commit-bot: I haz the power, 2015-08-21 01:37:33]

Try jobs failed on following builders:
  linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[pfeldman, 2015-08-21 17:22:09]

The CQ bit was checked by pfeldman@chromium.org

[pfeldman, 2015-08-21 17:22:10]

The patchset sent to the CQ was uploaded after l-g-t-m from dgozman@chromium.org
Link to the patchset: https://codereview.chromium.org/1308563002/#ps70001
(title: "rebaselined")

[commit-bot: I haz the power, 2015-08-21 17:22:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1308563002/70001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1308563002/70001

[commit-bot: I haz the power, 2015-08-21 20:15:54]

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

[commit-bot: I haz the power, 2015-08-21 20:15:55]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[pfeldman, 2015-08-21 20:18:46]

The CQ bit was checked by pfeldman@chromium.org

[commit-bot: I haz the power, 2015-08-21 20:19:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1308563002/70001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1308563002/70001

[commit-bot: I haz the power, 2015-08-21 20:23:39]

Committed patchset #5 (id:70001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=201008