Bug 154739

Summary: Web Inspector: Keyboard controls to nudge control points in custom transition bezier curve editor would be nice
Product: WebKit Reporter: Timothy Hatcher <timothy>
Component: Web InspectorAssignee: Devin Rousso <drousso>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, commit-queue, drousso, graouts, joepeck, mattbaker, nvasilyev, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch none

Description Timothy Hatcher 2016-02-26 10:58:13 PST
The editor lets you select a control point but it would be nice to be able to hit left/right to nudge it some set amount of units or shift-left/right nudge some more units.

Right now it’s hard to move a control point in one dimension with the mouse.

Would also not mind some direct number input while in this mode, but I’m fine with that being a follow up bug if that’s more manageable.

<rdar://problem/24861498>
Comment 1 Matt Baker 2016-02-26 11:53:20 PST
Holding shift to lock the movement horizontally or vertically (based on mouse direction) would be great too. Photoshop's line/pencil/pen tools have this feature.
Comment 2 Devin Rousso 2016-02-26 13:21:42 PST
(In reply to comment #0)
> The editor lets you select a control point but it would be nice to be able
> to hit left/right to nudge it some set amount of units or shift-left/right
> nudge some more units.

So, I'm thinking that the best way to do this might be to allow left/right (with the shift version too) to nudge the most recently selected control point.  Since controlpoints are only selected so long as the mouse is pressed, forcing the user to hold the mouse down while pressing left/right would be awkward (especially if they use an external mouse, since the arrow keys are also on the right).

> Right now it’s hard to move a control point in one dimension with the mouse.

I like Matt's suggestion.  Adding a shift-drag would do a straight line on either the x or y axis.

> Would also not mind some direct number input while in this mode, but I’m
> fine with that being a follow up bug if that’s more manageable.

So you mean like having actual inputs for the coordinates for each controlpoint?
Comment 3 Timothy Hatcher 2016-02-26 14:15:57 PST
(In reply to comment #2)
> (In reply to comment #0)
> > The editor lets you select a control point but it would be nice to be able
> > to hit left/right to nudge it some set amount of units or shift-left/right
> > nudge some more units.
> 
> So, I'm thinking that the best way to do this might be to allow left/right
> (with the shift version too) to nudge the most recently selected control
> point.  Since controlpoints are only selected so long as the mouse is
> pressed, forcing the user to hold the mouse down while pressing left/right
> would be awkward (especially if they use an external mouse, since the arrow
> keys are also on the right).
> 
> > Right now it’s hard to move a control point in one dimension with the mouse.
> 
> I like Matt's suggestion.  Adding a shift-drag would do a straight line on
> either the x or y axis.

I do to!

> > Would also not mind some direct number input while in this mode, but I’m
> > fine with that being a follow up bug if that’s more manageable.
> 
> So you mean like having actual inputs for the coordinates for each
> controlpoint?

I think that is what the originator meant. We could consider that separately.
Comment 4 Devin Rousso 2016-02-26 19:08:54 PST
Created attachment 272390 [details]
Patch
Comment 5 WebKit Commit Bot 2016-02-26 20:54:31 PST
Comment on attachment 272390 [details]
Patch

Clearing flags on attachment: 272390

Committed r197233: <http://trac.webkit.org/changeset/197233>
Comment 6 WebKit Commit Bot 2016-02-26 20:54:34 PST
All reviewed patches have been landed.  Closing bug.
Comment 7 Joseph Pecoraro 2016-02-26 21:02:16 PST
Comment on attachment 272390 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=272390&action=review

Neat!

> Source/WebInspectorUI/UserInterface/Views/BezierEditor.js:109
> +        WebInspector.addWindowKeydownListener(this);

I don't see a call to WebInspector.removeWindowKeydownListener. Seems like this would leak listeners in the global list, because this can never be removed. Please address this in a follow-up!

Also this is the first I've seen WebInspector.addWindowKeydownListener. Calls to WebInspector._updateWindowKeydownListener attempt to re-add the shared listener every time a new listener is added. I'd rather see it only attempt when the listener count is 1, not when >1.

> Source/WebInspectorUI/UserInterface/Views/BezierEditor.js:164
> +

Style: Extra newline.
Comment 8 Devin Rousso 2016-02-28 22:22:33 PST
(In reply to comment #7)
> Comment on attachment 272390 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=272390&action=review
> 
> Neat!
> 
> > Source/WebInspectorUI/UserInterface/Views/BezierEditor.js:109
> > +        WebInspector.addWindowKeydownListener(this);
> 
> I don't see a call to WebInspector.removeWindowKeydownListener. Seems like
> this would leak listeners in the global list, because this can never be
> removed. Please address this in a follow-up!
> 
> Also this is the first I've seen WebInspector.addWindowKeydownListener.
> Calls to WebInspector._updateWindowKeydownListener attempt to re-add the
> shared listener every time a new listener is added. I'd rather see it only
> attempt when the listener count is 1, not when >1.
> 
> > Source/WebInspectorUI/UserInterface/Views/BezierEditor.js:164
> > +
> 
> Style: Extra newline.

Both of these are addressed in <https://bugs.webkit.org/show_bug.cgi?id=154809>.