Bug 154739 - Web Inspector: Keyboard controls to nudge control points in custom transition bezier curve editor would be nice
Summary: Web Inspector: Keyboard controls to nudge control points in custom transition...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Local Build
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-02-26 10:58 PST by Timothy Hatcher
Modified: 2016-02-28 22:22 PST (History)
9 users (show)

See Also:


Attachments
Patch (4.78 KB, patch)
2016-02-26 19:08 PST, Devin Rousso
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>.