-
Notifications
You must be signed in to change notification settings - Fork 7.6k
For #6354: Tabbing between points in BezierEditor #6576
Conversation
Thanks. We're trying to finish Sprint 36, so not sure when I'll get to review this. |
No problem. |
} else { | ||
$(".P1").focus(); | ||
} | ||
e.preventDefault(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This block should return true;
to indicate that key was handled by this function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just fixed this.
Looks and works good. Just 1 comment. |
// Switch between the two points by tabbing | ||
if ($(e.target).hasClass("P1")) { | ||
$(".P2").focus(); | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we make sure (else if ($(e.target).hasClass("P2"))
) that this is really P2 here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way it is now, if focus is anywhere else, then it will return to P1. If you make that change, and focus goes somewhere else, it will never come back to the P1/P2 elements with Tab key. So, I think this is OK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright.
(But normally focus shouldn't go anywhere else)
Just changed the string to use "Tab" instead of the unicode char ↹ |
Looks good. Thanks! Merging. |
For #6354: Tabbing between points in BezierEditor
@redmunds: For #6354
With this PR, you can use the Tab key to switch between the two points of the Bezier editor.
It makes the BezierEditor completely keyboard-compatible.
Includes a german string change @couzteau