Skip to content

Fix hotkeys for Multi images (Compound files)#1561

Merged
anthony-chaudhary merged 2 commits intomasterfrom
fix_hotkeys_for_multy_images
Oct 20, 2023
Merged

Fix hotkeys for Multi images (Compound files)#1561
anthony-chaudhary merged 2 commits intomasterfrom
fix_hotkeys_for_multy_images

Conversation

@sergzak022
Copy link
Copy Markdown
Collaborator

Description

Fix a bug with hotkeys that happens in compound file context with multiple images. Updated HotkeyListener documentation to make it easier to use.

Bug Description:
Pressing a hotkey makes registered hotkey callbacks fire for all the image annotation panes


this.setupHotkeys(this.hotkeyListenerScope)

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

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.

}
if ( this.is_active) {
this.hotkey_listener.setScopes([this.hotkeyListenerScope])
} else {
Copy link
Copy Markdown
Collaborator Author

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.

@sergzak022
Copy link
Copy Markdown
Collaborator Author

Hey Anthony, I'm not removing old hotkey manager class because it looks like it's still used by text_annotation_core component. Please see listeners_map method of src/components/annotation/annotation_ui_factory.vue and keydown_event_listeners/keyup_event_listeners methods of text_annotation_core.vue. Once we move text_annotation_core to use the new HotkeyListener, we can get rid of HotkeyManager.

@anthony-chaudhary anthony-chaudhary changed the title Fix hotkeys for multy images Fix hotkeys for Multi images (Compound files) Oct 20, 2023
@anthony-chaudhary
Copy link
Copy Markdown
Member

Makes sense regarding keeping legacy HotkeyManager until text moves over to new one.

Tested manually and appears all working as expected now, thanks Sergey!

@anthony-chaudhary
Copy link
Copy Markdown
Member

anthony-chaudhary commented Oct 20, 2023

One small note for future is when we do remove legacy HotkeyManager, I believe we want to move the is_active scope setting to the higher level component (e.g. where HotkeyManager is currently sitting) to minimize code in "leaf" components.

(Edit: That's my interpretation and agreement of your idea (re: thin leaf nodes) from prior call)

@anthony-chaudhary anthony-chaudhary merged commit 1cc6678 into master Oct 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants