DevTools: Allow shadow-editor value manipulation with arrow keys and mousewheel

DevTools: Allow shadow-editor value manipulation with arrow keys and mousewheel

BUG=639095
Committed: https://crrev.com/b3af58d2e0f9df1e6e625c3d6c3815e5e688f515
Cr-Commit-Position: refs/heads/master@{#415068}

[flandy, 2016-08-25 18:05:24]

flandy@google.com changed reviewers:
+ dgozman@chromium.org, lushnikov@chromium.org

[flandy, 2016-08-25 18:05:25]

Please, take a look.

[dgozman, 2016-08-26 19:10:58]

lgtm

https://codereview.chromium.org/2278203003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js (right):

https://codereview.chromium.org/2278203003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:134: if
(!arrowKeyOrMouseWheelEvent && !pageKeyPressed)
Doesn't createReplacementString check this for you?

[flandy, 2016-08-26 19:25:59]

https://codereview.chromium.org/2278203003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js (right):

https://codereview.chromium.org/2278203003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:134: if
(!arrowKeyOrMouseWheelEvent && !pageKeyPressed)
On 2016/08/26 19:10:58, dgozman wrote:
> Doesn't createReplacementString check this for you?

No, it's checked one level higher in WI.handleElementValueModifications.

[dgozman, 2016-08-26 20:49:35]

https://codereview.chromium.org/2278203003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js (right):

https://codereview.chromium.org/2278203003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:134: if
(!arrowKeyOrMouseWheelEvent && !pageKeyPressed)
On 2016/08/26 19:25:59, flandy wrote:
> On 2016/08/26 19:10:58, dgozman wrote:
> > Doesn't createReplacementString check this for you?
> 
> No, it's checked one level higher in WI.handleElementValueModifications.

If it doesn't, we should make it. But I think it does:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/devtools/front...

[flandy, 2016-08-26 22:19:42]

Please take another look. I've moved the check inwards

https://codereview.chromium.org/2278203003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js (right):

https://codereview.chromium.org/2278203003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/CSSShadowEditor.js:134: if
(!arrowKeyOrMouseWheelEvent && !pageKeyPressed)
On 2016/08/26 20:49:35, dgozman wrote:
> On 2016/08/26 19:25:59, flandy wrote:
> > On 2016/08/26 19:10:58, dgozman wrote:
> > > Doesn't createReplacementString check this for you?
> > 
> > No, it's checked one level higher in WI.handleElementValueModifications.
> 
> If it doesn't, we should make it. But I think it does:
>
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/devtools/front...

Yes, that can be used if we also return null from
_modifiedFloatNumber/_modifiedHexValue.

[flandy, 2016-08-29 16:53:42]

Ping

[dgozman, 2016-08-29 17:47:20]

lgtm

https://codereview.chromium.org/2278203003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js (right):

https://codereview.chromium.org/2278203003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js:389: * @param
{!Event} event
Let's add @return annotation.

[flandy, 2016-08-29 18:47:06]

https://codereview.chromium.org/2278203003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js (right):

https://codereview.chromium.org/2278203003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js:389: * @param
{!Event} event
On 2016/08/29 17:47:20, dgozman wrote:
> Let's add @return annotation.

Done. 

The lack of return annotations here skirted some closure compiler requirements
for customNumberHandler. We only use custom number handlers for regular numbers
and not hex values, so I've updated the code to reflect that.

[flandy, 2016-08-29 23:08:29]

The CQ bit was checked by flandy@google.com

[flandy, 2016-08-29 23:08:29]

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/2278203003/#ps60001
(title: "Add return annotations")

[commit-bot: I haz the power, 2016-08-29 23:53:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-30 04:02:14]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-08-30 04:04:34]

Description was changed from

==========
DevTools: Allow shadow-editor value manipulation with arrow keys and mousewheel

BUG=639095
==========

to

==========
DevTools: Allow shadow-editor value manipulation with arrow keys and mousewheel

BUG=639095

Committed: https://crrev.com/b3af58d2e0f9df1e6e625c3d6c3815e5e688f515
Cr-Commit-Position: refs/heads/master@{#415068}
==========

[commit-bot: I haz the power, 2016-08-30 04:04:35]

Patchset 4 (id:??) landed as
https://crrev.com/b3af58d2e0f9df1e6e625c3d6c3815e5e688f515
Cr-Commit-Position: refs/heads/master@{#415068}