Bug 154739 – Web Inspector: Keyboard controls to nudge control points in custom transition bezier curve editor would be nice

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Timothy Hatcher]

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>

[Matt Baker]

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.

[Devin Rousso]

(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?

[Timothy Hatcher]

(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.

[Devin Rousso]

Created attachment 272390 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 272390 [details]
Patch

Clearing flags on attachment: 272390

Committed r197233: <http://trac.webkit.org/changeset/197233>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Joseph Pecoraro]

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.

[Devin Rousso]

(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>.