Skip to content

Fix hotkeys for Multi images (Compound files) #1561

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
anthony-chaudhary merged 2 commits into from
Oct 20, 2023
+41 −10
Merged

Fix hotkeys for Multi images (Compound files) #1561

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
945bfc6
fix hotkeys firing for multiple image annotation panes
sergzak022 Oct 19, 2023
11959d0
update HotkeyListener documentation
sergzak022 Oct 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions frontend/src/components/annotation/image_and_video_annotation/annotation_core.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,14 @@ export default Vue.extend({
watch: {
is_active: function (){
this.canvas_element.style.cursor = ''
if ( !this.hotkeyListenerScope ) {
return
}
if ( this.is_active) {
this.hotkey_listener.setScopes([this.hotkeyListenerScope])
} else {
Copy link
Copy Markdown
Collaborator Author

@sergzak022 sergzak022 Oct 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The else block is needed for situations when user annotates in the compound context. For example text_annotation component currently uses a different mechanism for registering hotkeys. If user switches from image annotation to text in the same compound context, we needs to deactivate image annotation hotkeys.

this.hotkey_listener.removeScope(this.hotkeyListenerScope)
}
},
global_instance: function(){
this.$emit('global_instance_changed', this.working_file.id, this.global_instance)
Expand Down Expand Up @@ -1733,6 +1741,10 @@ export default Vue.extend({

this.setupHotkeys(this.hotkeyListenerScope)

if ( this.is_active ) {
Copy link
Copy Markdown
Collaborator Author

@sergzak022 sergzak022 Oct 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once image annotation components are mounted, need to activate registered hotkeys ofr the active one.

this.hotkey_listener.setScopes([this.hotkeyListenerScope])
}

if (
this.annotation_ui_context.working_file_list[0].id === this.annotation_ui_context.working_file.id
) {
Expand All @@ -1753,8 +1765,6 @@ export default Vue.extend({

setupHotkeys(scope) {

this.hotkey_listener.addScope(scope)

this.hotkey_listener.addFilter(this.hotkeyListenerFilter)

this.hotkey_listener.onKeydown({ keys: 's', scope }, () => {
Expand Down
37 changes: 29 additions & 8 deletions frontend/src/utils/hotkey_listener.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,10 @@ interface HotkeysOptions {
scope: string
element ?: HTMLElement,
debounceThreshold?: number
platformDependent: boolean
preventDefaultEvent: boolean
platformDependent?: boolean
preventDefaultEvent?: boolean
}

export type KeysOrOpts = HotkeysOptions

export type KeyEventHandler = (event: KeyboardEvent, handler: HotkeysEvent) => void
/**
* `HotkeyListener` - A utility class to manage keyboard hotkeys using the `hotkeys-js` library.
Expand All @@ -37,12 +35,13 @@ export type KeyEventHandler = (event: KeyboardEvent, handler: HotkeysEvent) => v
* - Fetch the listener instance: `const listener = HotkeyListener.getInstance()`.
*
* 2. **Hotkey Registration**:
* - Register a hotkey that reacts on keyup: `listener.onKeyup('ctrl+b', callback)`.
* - For keydown events: `listener.onKeydown('ctrl+b', callback)`.]
* - Register a hotkey that reacts on keyup: `listener.onKeyup({keys: 'ctrl+b', scope: 'foo'}, callback)`.
* - For keydown events: `listener.onKeydown({keys: 'ctrl+a,ctrl+b', scope: 'foo'}, callback)`.]
* - For binding to individual special keys like shift, ctrl, etc. use onSpecialKeydown and onSpecialKeyup: `listener.onSpecialKeydown('shift', callback)`
*
* 3. **Scopes**:
* - Select scope and create if doesn't exist already: `listener.addScope('myScope')`.
* - Select only scopes passed as an argument: `listener.setScopes(['scope_one', 'scope_two'])`.
* - Select scope and keep previously selected scopes selected: `listener.addScope('myScope')`.
* - Cancle selection of a scope: `listener.removeScope('myScope')`.
* - Delete scope and all hotkey bindings: `listener.deleteScope('myScope')`.
* - Note: A hotkey will only trigger if its scope is among the selected scopes.
Expand All @@ -55,8 +54,26 @@ export type KeyEventHandler = (event: KeyboardEvent, handler: HotkeysEvent) => v
* **Important**: Due to `hotkeys-js` providing a singleton instance, `HotkeyListener` uses the singleton pattern
* to ensure a single centralized management point.
*
* @example
* ```javascript
* const listener = HotkeyListener.getInstance(); // Initialization
*
* // Register hotkeys
* listener.onKeyup({keys: 'ctrl+a', scope: 'foo'}, () => {}); // Register callback to fire when 'ctrl+a' is pressed and scope 'foo' is active
* listener.onKeyup({keys: 'ctrl+b', scope: 'foo'}, () => {}); // Register callback to fire when 'ctrl+b' is pressed and scope 'foo' is active
* listener.onKeyup({keys: 'ctrl+c', scope: 'bar'}, () => {}); // Register callback to fire when 'ctrl+c' is pressed and scope 'bar' is active
*
* // Enable/disable scopes
* listener.addScopes('foo'); // Hotkey callbacks registered for 'foo' scope will be active
* listener.removeScope('foo'); // Hotkey callbacks registered for 'foo' scope won't fire
* listener.addScopes('bar'); // Hotkey callbacks registered for 'foo' and 'bar' scopes will be active
* listener.setScopes(['foo']); // Only hotkey callbacks registered for 'foo' scope will be active
* listener.deleteScope('foo'); // Unselects 'foo' scope and unregisters all the callbacks associated with that scope.

* listener.addScope('bar'); // Hotkeys for 'bar' scope will still fire callbacks, but not for 'foo' scope since deleteScope was called on it
* ```
* TODO:
* 1. If we are ever in situation where we need to have many too many scopes registered
* 1. If we are ever in situation where we need to have too many scopes registered
* at the same time, we can enhance HotkeyListener logic so it unregisters callbacks for non-active scopes.
* This will reduce the number of active callbacks
* @class
Expand Down Expand Up @@ -143,6 +160,10 @@ export class HotkeyListener {
return this.selectedScopes
}

setScopes(scopes: Array<string>) : void {
this.selectedScopes = scopes
}

addScope(scope: string) {
if (!this.selectedScopes.includes(scope)) {
this.selectedScopes.push(scope)
Expand Down