Skip to content
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.

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

Hotkeys not working: Z, PageUp, PageDown, = #1095

Closed
mirnasilva opened this issue Oct 25, 2019 · 5 comments · Fixed by #1446
Closed

Hotkeys not working: Z, PageUp, PageDown, = #1095

mirnasilva opened this issue Oct 25, 2019 · 5 comments · Fixed by #1446
Assignees
Labels
Bug Verified Bug reported, reproducible, and verified.

Comments

@mirnasilva
Copy link
Contributor

mirnasilva commented Oct 25, 2019

Bug Report

Issue 1 - Description:

Hotkeys not working on the Viewer page: Z, PageUp, PageDown.

Steps to Reproduce

  1. Open a study
  2. Try to use the following hotkeys: Z, PageUp, PageDown
  3. Checks that nothing happened.

Actual Result

Hotkeys Z, PageUp, PageDown are not working, nothing happens at the viewer page.

Expected Result

  1. When user hits hotkey Z, the Zoom tool should be activated at the toolbar
  2. User should be able to navigate on the thumbnails using hotkeys: PageUp and PageDown
    2.1. When user hits hotkey PageDown, the next thumbnail series should be loaded in the viewport
    2.2. When user hits hotkey PageUp, the previous thumbnail series should be loaded in the viewport

Issue 2 - Description:

When there are multiple viewports, hotkey = does not reset the inverted status of the image in the active viewport.

In the gif below, user presses the = hotkey to reset all viewports before clicking on the Reset button

bug-hotkey-equal

Steps to Reproduce

  1. Open a study
  2. Change layout to 3 viewports
  3. Invert the image by pressing hotkey I
  4. Press hotkey =

Actual Result

Hotkey = does not reset the inverted status of the image. If user clicks on the Reset button, the inverted image will be reset with success.

Expected Result

User should be able to reset the inverted image (in the active viewport) by pressing hotkey = or clicking on Reset button on toolbar.

@mirnasilva mirnasilva changed the title Hotkeys not working: Z, PageUp, PageDown Hotkeys not working: Z, PageUp, PageDown, = Oct 25, 2019
@dannyrb dannyrb added Bug Verified Bug reported, reproducible, and verified. and removed Community: Report 🐛 labels Oct 26, 2019
@dannyrb
Copy link
Member

dannyrb commented Oct 26, 2019

Quick Overview

  • Extensions have a "CommandModule"
  • A "CommandModule" adds named commands
    • Commands are used if there "context" is active

For example, the cornerstone extension has some command named Rotate that is active in the ACTIVE_VIEWPORT::CORNERSTONE context. This means that, if something fires the Rotate command, cornerstone's implementation of it will only be called/triggered if the active viewport is a cornerstone viewport.

  • Hotkeys are configured in the application config
    • Default is <root>/platform/viewer/public/config/default.js
  • Hotkeys are defined as key(s), and a command name

This setup is what allows multiple extensions to register similar commands that can leverage the same hotkeys, but intelligently know when which one should be used.

Fix

  • Update default.js to use the correct command names
  • Verify that the implementations in the cornerstone extension still work
  • Update all other configs that contain hotkeys to use these update mappings
    • In the future, we will have "default" hotkeys an extension can set to reduce this maintenance

@ladeirarodolfo ladeirarodolfo self-assigned this Nov 1, 2019
@mirnasilva
Copy link
Contributor Author

@ladeirarodolfo Please disregard issue #2. The correct hotkey responsible for reset the viewport is SPACEBAR. The functionality is working as expected :)

@cmcgee-mac
Copy link
Contributor

How would one find out what the command is to enable a tool, like the zoom tool in this case? It seems that "setZoomTool" is not the correct one for the zoom tool.

@dannyrb
Copy link
Member

dannyrb commented Nov 15, 2019

👋 @cmcgee-mac

We're trying to document the available commands better. The end-goal is to have them listed in the readme for our maintained extensions. You can see a few listed here:

https://github.com/OHIF/Viewers/tree/master/extensions/cornerstone#commands-module

Those might be out of date. You can always dig in to the source and see the list of named commands in the extension's command definitions:

https://github.com/OHIF/Viewers/blob/master/extensions/cornerstone/src/commandsModule.js#L167-L246


It looks like, at some point, we switched from setZoomTool to a more generic setToolActive that takes a tool's name as a parameter:

https://github.com/OHIF/Viewers/blob/master/extensions/cornerstone/src/commandsModule.js#L84-L89

We are more than happy to hear any thoughts you have around improving discovery of commands, or PRs to improve our documentation ^_^

@galelis
Copy link
Contributor

galelis commented Feb 14, 2020

@mirnasilva
We actually have 3 issues here.

  • Z hotkey not working
    Fixed, we were missing the command setZoomTool.

  • PageUp, PageDown hotkey not working
    Fixed, we have never implemented the method to navigate through current displaySets

  • = hotkey not resetting inverted configurations.
    Wht happens is that = is current attached to zoom to fit so it's not suposed to reset invert. We have space currently set for Reset and it was properly resetting the invert configurations. If you can take a look again and let me know if I tested it wrongly.

Fixable issues should be fixed by the PR: #1446

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Verified Bug reported, reproducible, and verified.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants