Repository navigation
feat: Added keyboard shortcut for displaying tooltip - #9755
Conversation
| /** | ||
| * Display the tooltip for a given target. | ||
| */ | ||
| export function display(target: AnyDuringMigration, workspace?: WorkspaceSvg) { |
There was a problem hiding this comment.
The show() function is relying on some checks already having been made, so I added this new function rather than exporting that one.
| /** | ||
| * Display the tooltip for a given target. | ||
| */ | ||
| export function display(target: AnyDuringMigration, workspace?: WorkspaceSvg) { |
There was a problem hiding this comment.
Maybe make this @internal and use IFocusableNode as the type for target? If we're not expecting third party callers to use this it's probably better to keep it locked down for now.
There was a problem hiding this comment.
Would be nice to add @param lines to the docstring too.
| assert.deepEqual(oldBounds, newBounds); | ||
| }); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Throughout, assert.isTrue()/assert.isFalse() might make the intent a bit clearer.
github-actions
Bot
added
PR: feature
|
Regarding the shortcut, cmd J while using VoiceOver on mac works to open the tooltip, but it also triggers VoiceOver to output "jump to selection". Incidental, but when I was trying this out, I immediately tried cmd J to toggle the tooltip closed again, but you need Escape. No strong view either way. |
|
@microbit-robert Thanks for bringing that to our attention. This is a placeholder until we can find a better solution. I did not run into this issue myself when testing with VO, but we'll be replacing the shortcut with something else once it's been further discussed anyway so I'm not too concerned. Let us know if you have input on that; we've discussed Ctrl/Cmd + K, Ctrl/Cmd + I, but that is more complex than we'd like, Ctrl/Cmd + Q, but that is the quit application shortcut in Mac, and Ctrl/Cmd + F1, but we've been avoiding function keys up to this point. Happy to hear any suggestions! |
3 participants
The basics
The details
Resolves
Fixes RaspberryPiFoundation/blockly-keyboard-experimentation#447
Proposed Changes
Adds a keyboard shortcut for displaying the tooltip.
Reason for Changes
Adding the ability to display the tooltip from a keyboard shortcut increases accessibility for keyboard users.
Test Coverage
Added unit tests around basic functionality, ensuring an existing tooltip does not block a new tooltip from popping up, and ensuring it does not display while dragging is happening.
Additional Information
Screenshots:
[ ] TODO: Update the key mapping to something that makes sense.
The key mapping in this PR is a placeholder; which actual key(s) to use is something that still needs to be decided. I picked Ctrl+J as it wasn't currently in use. I saw that VS Code uses Ctrl+K followed by Ctrl+I but I didn't see an obvious way to register this in the code and didn't want to spend extra time on it before we even know if we need to do so.