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

Overlays 2.0 #4894

Merged
merged 101 commits into from Feb 6, 2023
Merged

Overlays 2.0 #4894

merged 101 commits into from Feb 6, 2023

Conversation

brisvag
Copy link
Contributor

@brisvag brisvag commented Aug 2, 2022

Description

This PR overhauls the "overlay" concept by unifying their behaviour and implementing a framework that allows us to expand it more easily and eventually expose it to plugins.

Here's a summary of the changes to help you navigate the diff:

General idea:

  1. Overlays are no longer special cased and handled all through QtViewer. Instead, they go through a machinery very similar to that of Layers, where an VispyOverlay object gets autogenerated from an Overlay evented model by the overlay_to_visual in _vispy.utils.visual.py. This object takes care of hooking up events to the actual vispy backend visualisation.
  2. Overlays can differ on 2 axes which can be combined in any way:
    • canvas/scene: canvas overlays live in canvas space (2D, unaffected by world transforms), while scene overlays live in scene space (2D or 3D, "displayed world")
    • viewer/layer: viewer overlays (scale_bar, text, axes) live on the ViewerModel, while layer overlays (bounding_box, transform_box, selection_box) live on each Layer.
  3. I tried to improve the split between the "visual" logic and the "napari-to-vispy" logic in the _vispy.visuals and _vispy.overlays modules respectively, to reflect what we have for layers.
  4. Both on ViewerModel and Layer they are kept in an EventedDict called _overlays. The ones meant for public use are exposed through properties.

Changes in individual overlays:

  • text, scale_bar and axes are mostly unchanged, except for additional parameters such as opacity and order. They are exposed publicly on the viewer through properties, so this is basically backwards compatible.
  • bounding_box is new, see Layer bounding box #4849. It's exposed publicly on every layer as layer.bounding_box and disabled by default.
  • interaction_box was the main blocker here, cause it used a completely custom machinery. I had to remove it and reimplement it using the new system. It was completely removed from where it used to be (viewer.overlays.interaction_box), and is now a relatively simple overlay called transform_box (not publicly exposed, for now). The whole interactive transform logic was moved to proper layer callbacks, rather than using a custom stateful object on QtViewer. (it now lives in layers.base._base_mouse_bindings.py). This simplified the code a fair bit, but in turn I had to make changes to:
    • allow every layer to change mode and use the transform mode (previouysly only image could do it)
    • fix action manager and qt layer controls where they broke due to this.
    • maybe TODO: make transform mode simply non-selectable in 3D?
  • selection_box is using the same overlay machinery from transform_box, but it's intended to replace the custom selection logic that Points and other layers implement with a unified system, while also allowing to transform the selected object (like interaction_box was meant to do before, at least for points). This is not yet implemented, but I think it should happen in a followup PR. @jojoelfe: as the one who implemented this the first time around, how do you feel about these changes? Particularly, what did I break that you might rely on?

Of course I'm also missing some dosctrings and comments, but I'll get around to it soon :)

Here's a snippet to test out everything:

import napari
import numpy as np

v = napari.Viewer(ndisplay=3)
img = v.add_image(np.random.rand(20, 10, 15), scale=[5, 15, 10])
pts = v.add_points(np.random.rand(20, 3) * 200, translate=[20, -10])

v.scale_bar.visible = True
v.axes.visible = True
v.text_overlay.visible = True
v.text_overlay.text = 'Cool stuff!'
v.text_overlay.position = 'top_center'

img.bounding_box.visible = True
img.bounding_box.line_color = 'green'
img.bounding_box.points = False

pts.bounding_box.visible = True

img.mode = 'transform'
v.reset_view()

To test the transform_box, switch to 2D and select the image layer.

overlays2.mp4

Type of change

  • Bug-fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

References

@brisvag brisvag changed the title tmp overlay work [WIP] Overlays 2.0 Aug 2, 2022
@github-actions github-actions bot added qt Relates to qt tests Something related to our tests labels Aug 2, 2022
Copy link
Contributor Author

@brisvag brisvag left a comment

Choose a reason for hiding this comment

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

I added a few comments to help reviewing. Many of the changes inside each individual VispyOverlay are not strictly necessary for this PR (though they help a lot with reasoning about it) and could be factored out and merged beforehand, so you can ignore them for the purposes of understanding the overall changes. In practice, I split out the Visual component from the machinery that napari needs to communicate to the visual how to change (VispyOverlay).

napari/_qt/qt_viewer.py Outdated Show resolved Hide resolved
napari/_vispy/overlays/base.py Outdated Show resolved Hide resolved
napari/_vispy/overlays/base.py Outdated Show resolved Hide resolved
napari/components/overlays/base.py Outdated Show resolved Hide resolved
@brisvag
Copy link
Contributor Author

brisvag commented Aug 3, 2022

Good progress! I incorporated the bounding box code from #4849, so now we also have an example of LayerOverlay!

You can now test everything with this little snippet:

import napari
import numpy as np
v = napari.Viewer(ndisplay=3)
img = v.add_image(np.random.rand(20, 10, 15), scale=[5, 15, 10])
pts = v.add_points(np.random.rand(20, 3) * 200, translate=[20, -10])
v.overlays['scale_bar'].visible = True
v.overlays['axes'].visible = True
v.overlays['text'].visible = True
v.overlays['text'].text = 'Cool stuff!'
v.overlays['text'].position = 'top_right'
img.overlays['bounding_box'].visible = True
img.overlays['bounding_box'].line_color = 'green'
img.overlays['bounding_box'].points = False
pts.overlays['bounding_box'].visible = True
v.reset_view()
overlays.mp4

@brisvag brisvag mentioned this pull request Aug 4, 2022
12 tasks
@github-actions
Copy link
Contributor

Click here to download the docs artifacts
docs
(zip file)

@jni jni added this to the 0.5.0 milestone May 25, 2023
@Czaki Czaki modified the milestones: 0.5.0, 0.4.18 Jun 13, 2023
@Czaki Czaki mentioned this pull request Jun 15, 2023
Czaki added a commit that referenced this pull request Jun 18, 2023
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com>
Czaki pushed a commit that referenced this pull request Jun 18, 2023
See #4894 (comment).
This code now lives in the base layer, so this was a duplicate.

<!-- What does this pull request (PR) do? Why is it necessary? -->
<!-- Tell us about your new feature, improvement, or fix! -->
<!-- If your change includes user interface changes, please add an
image, or an animation "An image is worth a thousand words!" -->
<!-- You can use https://www.cockos.com/licecap/ or similar to create
animations -->

<!-- Please delete options that are not relevant. -->
- [x] Bug-fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] This change requires a documentation update

<!-- What resources, documentation, and guides were used in the creation
of this PR? -->
<!-- If this is a bug-fix or otherwise resolves an issue, reference it
here with "closes #(issue)" -->

<!-- Please describe the tests that you ran to verify your changes. -->
- [ ] example: the test suite for my feature covers cases x, y, and z
- [ ] example: all tests pass with my change
- [ ] example: I check if my changes works with both PySide and PyQt
backends
      as there are small differences between the two Qt bindings.

- [ ] My PR is the minimum possible work for the desired functionality
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] If I included new strings, I have used `trans.` to make them
localizable.
For more information see our [translations
guide](https://napari.org/developers/translations.html).
Czaki added a commit that referenced this pull request Jun 19, 2023
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com>
Czaki pushed a commit that referenced this pull request Jun 19, 2023
See #4894 (comment).
This code now lives in the base layer, so this was a duplicate.

<!-- What does this pull request (PR) do? Why is it necessary? -->
<!-- Tell us about your new feature, improvement, or fix! -->
<!-- If your change includes user interface changes, please add an
image, or an animation "An image is worth a thousand words!" -->
<!-- You can use https://www.cockos.com/licecap/ or similar to create
animations -->

<!-- Please delete options that are not relevant. -->
- [x] Bug-fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] This change requires a documentation update

<!-- What resources, documentation, and guides were used in the creation
of this PR? -->
<!-- If this is a bug-fix or otherwise resolves an issue, reference it
here with "closes #(issue)" -->

<!-- Please describe the tests that you ran to verify your changes. -->
- [ ] example: the test suite for my feature covers cases x, y, and z
- [ ] example: all tests pass with my change
- [ ] example: I check if my changes works with both PySide and PyQt
backends
      as there are small differences between the two Qt bindings.

- [ ] My PR is the minimum possible work for the desired functionality
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] If I included new strings, I have used `trans.` to make them
localizable.
For more information see our [translations
guide](https://napari.org/developers/translations.html).
Czaki pushed a commit that referenced this pull request Jun 19, 2023
…5669)

This fixes issues with Pan/Zoom buttons (layer controls) not working and
the spacebar keybinding to activate pan/zoom mode also not working.
In #4894 there was some refactoring
which introduced the bugs, see my investigation here:
#5654

This PR fixes the typo, lack of proper layer types, and now moves the
`hold_for_pan_zoom` to be a Viewer keybinding. As a result, this
keybinding isn't hard-coded anymore, but is now settable in the
Preferences > Shortcuts.

<!-- Please delete options that are not relevant. -->
- [x] Bug-fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] This change requires a documentation update

closes #5654

Good question! All tests were passing before this PR, despite the fact
that things were broken. So I'm open to suggestions how to improve tests
so we catch stuff better.

- [x] My PR is the minimum possible work for the desired functionality
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] If I included new strings, I have used `trans.` to make them
localizable.
For more information see our [translations
guide](https://napari.org/developers/translations.html).
Czaki pushed a commit that referenced this pull request Jun 19, 2023
# Description
closes #5677

I tracked down the source of the added Transform and Pan/zoom in the
Viewer keybindings.
Those are actions/keybinds added in
#4894 for the Tracks layer. But
Tracks layer wasn't added as an option in the shortcuts table, so they
appear in the default (Viewer).
With this PR, I import Tracks and add it to the dropdown so Tracks has
it's own table and now the actions appear in the proper table, rather
than under Viewer keybindings.

<img width="1198" alt="image"
src="https://user-images.githubusercontent.com/76622105/228287187-189cb745-fb6e-4c31-93f1-6ba22e965a56.png">

<img width="1198" alt="image"
src="https://user-images.githubusercontent.com/76622105/228287227-da2642b7-8f76-4451-bdfa-e6801f47400a.png">



## Type of change
<!-- Please delete options that are not relevant. -->
- [x] Bug-fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] This change requires a documentation update

# References


# How has this been tested?
<!-- Please describe the tests that you ran to verify your changes. -->
- [ ] example: the test suite for my feature covers cases x, y, and z
- [x] example: all tests pass with my change
- [ ] example: I check if my changes works with both PySide and PyQt
backends
      as there are small differences between the two Qt bindings.  

## Final checklist:
- [x] My PR is the minimum possible work for the desired functionality
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] If I included new strings, I have used `trans.` to make them
localizable.
For more information see our [translations
guide](https://napari.org/developers/translations.html).
Czaki added a commit that referenced this pull request Jun 21, 2023
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com>
Czaki pushed a commit that referenced this pull request Jun 21, 2023
See #4894 (comment).
This code now lives in the base layer, so this was a duplicate.

<!-- What does this pull request (PR) do? Why is it necessary? -->
<!-- Tell us about your new feature, improvement, or fix! -->
<!-- If your change includes user interface changes, please add an
image, or an animation "An image is worth a thousand words!" -->
<!-- You can use https://www.cockos.com/licecap/ or similar to create
animations -->

<!-- Please delete options that are not relevant. -->
- [x] Bug-fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] This change requires a documentation update

<!-- What resources, documentation, and guides were used in the creation
of this PR? -->
<!-- If this is a bug-fix or otherwise resolves an issue, reference it
here with "closes #(issue)" -->

<!-- Please describe the tests that you ran to verify your changes. -->
- [ ] example: the test suite for my feature covers cases x, y, and z
- [ ] example: all tests pass with my change
- [ ] example: I check if my changes works with both PySide and PyQt
backends
      as there are small differences between the two Qt bindings.

- [ ] My PR is the minimum possible work for the desired functionality
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] If I included new strings, I have used `trans.` to make them
localizable.
For more information see our [translations
guide](https://napari.org/developers/translations.html).
Czaki pushed a commit that referenced this pull request Jun 21, 2023
…5669)

This fixes issues with Pan/Zoom buttons (layer controls) not working and
the spacebar keybinding to activate pan/zoom mode also not working.
In #4894 there was some refactoring
which introduced the bugs, see my investigation here:
#5654

This PR fixes the typo, lack of proper layer types, and now moves the
`hold_for_pan_zoom` to be a Viewer keybinding. As a result, this
keybinding isn't hard-coded anymore, but is now settable in the
Preferences > Shortcuts.

<!-- Please delete options that are not relevant. -->
- [x] Bug-fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] This change requires a documentation update

closes #5654

Good question! All tests were passing before this PR, despite the fact
that things were broken. So I'm open to suggestions how to improve tests
so we catch stuff better.

- [x] My PR is the minimum possible work for the desired functionality
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] If I included new strings, I have used `trans.` to make them
localizable.
For more information see our [translations
guide](https://napari.org/developers/translations.html).
Czaki pushed a commit that referenced this pull request Jun 21, 2023
# Description
closes #5677

I tracked down the source of the added Transform and Pan/zoom in the
Viewer keybindings.
Those are actions/keybinds added in
#4894 for the Tracks layer. But
Tracks layer wasn't added as an option in the shortcuts table, so they
appear in the default (Viewer).
With this PR, I import Tracks and add it to the dropdown so Tracks has
it's own table and now the actions appear in the proper table, rather
than under Viewer keybindings.

<img width="1198" alt="image"
src="https://user-images.githubusercontent.com/76622105/228287187-189cb745-fb6e-4c31-93f1-6ba22e965a56.png">

<img width="1198" alt="image"
src="https://user-images.githubusercontent.com/76622105/228287227-da2642b7-8f76-4451-bdfa-e6801f47400a.png">



## Type of change
<!-- Please delete options that are not relevant. -->
- [x] Bug-fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] This change requires a documentation update

# References


# How has this been tested?
<!-- Please describe the tests that you ran to verify your changes. -->
- [ ] example: the test suite for my feature covers cases x, y, and z
- [x] example: all tests pass with my change
- [ ] example: I check if my changes works with both PySide and PyQt
backends
      as there are small differences between the two Qt bindings.  

## Final checklist:
- [x] My PR is the minimum possible work for the desired functionality
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] If I included new strings, I have used `trans.` to make them
localizable.
For more information see our [translations
guide](https://napari.org/developers/translations.html).
Czaki added a commit that referenced this pull request Jun 21, 2023
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com>
Czaki pushed a commit that referenced this pull request Jun 21, 2023
See #4894 (comment).
This code now lives in the base layer, so this was a duplicate.

<!-- What does this pull request (PR) do? Why is it necessary? -->
<!-- Tell us about your new feature, improvement, or fix! -->
<!-- If your change includes user interface changes, please add an
image, or an animation "An image is worth a thousand words!" -->
<!-- You can use https://www.cockos.com/licecap/ or similar to create
animations -->

<!-- Please delete options that are not relevant. -->
- [x] Bug-fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] This change requires a documentation update

<!-- What resources, documentation, and guides were used in the creation
of this PR? -->
<!-- If this is a bug-fix or otherwise resolves an issue, reference it
here with "closes #(issue)" -->

<!-- Please describe the tests that you ran to verify your changes. -->
- [ ] example: the test suite for my feature covers cases x, y, and z
- [ ] example: all tests pass with my change
- [ ] example: I check if my changes works with both PySide and PyQt
backends
      as there are small differences between the two Qt bindings.

- [ ] My PR is the minimum possible work for the desired functionality
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] If I included new strings, I have used `trans.` to make them
localizable.
For more information see our [translations
guide](https://napari.org/developers/translations.html).
Czaki pushed a commit that referenced this pull request Jun 21, 2023
…5669)

This fixes issues with Pan/Zoom buttons (layer controls) not working and
the spacebar keybinding to activate pan/zoom mode also not working.
In #4894 there was some refactoring
which introduced the bugs, see my investigation here:
#5654

This PR fixes the typo, lack of proper layer types, and now moves the
`hold_for_pan_zoom` to be a Viewer keybinding. As a result, this
keybinding isn't hard-coded anymore, but is now settable in the
Preferences > Shortcuts.

<!-- Please delete options that are not relevant. -->
- [x] Bug-fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] This change requires a documentation update

closes #5654

Good question! All tests were passing before this PR, despite the fact
that things were broken. So I'm open to suggestions how to improve tests
so we catch stuff better.

- [x] My PR is the minimum possible work for the desired functionality
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] If I included new strings, I have used `trans.` to make them
localizable.
For more information see our [translations
guide](https://napari.org/developers/translations.html).
Czaki pushed a commit that referenced this pull request Jun 21, 2023
# Description
closes #5677

I tracked down the source of the added Transform and Pan/zoom in the
Viewer keybindings.
Those are actions/keybinds added in
#4894 for the Tracks layer. But
Tracks layer wasn't added as an option in the shortcuts table, so they
appear in the default (Viewer).
With this PR, I import Tracks and add it to the dropdown so Tracks has
it's own table and now the actions appear in the proper table, rather
than under Viewer keybindings.

<img width="1198" alt="image"
src="https://user-images.githubusercontent.com/76622105/228287187-189cb745-fb6e-4c31-93f1-6ba22e965a56.png">

<img width="1198" alt="image"
src="https://user-images.githubusercontent.com/76622105/228287227-da2642b7-8f76-4451-bdfa-e6801f47400a.png">



## Type of change
<!-- Please delete options that are not relevant. -->
- [x] Bug-fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] This change requires a documentation update

# References


# How has this been tested?
<!-- Please describe the tests that you ran to verify your changes. -->
- [ ] example: the test suite for my feature covers cases x, y, and z
- [x] example: all tests pass with my change
- [ ] example: I check if my changes works with both PySide and PyQt
backends
      as there are small differences between the two Qt bindings.  

## Final checklist:
- [x] My PR is the minimum possible work for the desired functionality
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] If I included new strings, I have used `trans.` to make them
localizable.
For more information see our [translations
guide](https://napari.org/developers/translations.html).
Czaki added a commit that referenced this pull request Jun 21, 2023
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com>
Czaki pushed a commit that referenced this pull request Jun 21, 2023
See #4894 (comment).
This code now lives in the base layer, so this was a duplicate.

<!-- What does this pull request (PR) do? Why is it necessary? -->
<!-- Tell us about your new feature, improvement, or fix! -->
<!-- If your change includes user interface changes, please add an
image, or an animation "An image is worth a thousand words!" -->
<!-- You can use https://www.cockos.com/licecap/ or similar to create
animations -->

<!-- Please delete options that are not relevant. -->
- [x] Bug-fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] This change requires a documentation update

<!-- What resources, documentation, and guides were used in the creation
of this PR? -->
<!-- If this is a bug-fix or otherwise resolves an issue, reference it
here with "closes #(issue)" -->

<!-- Please describe the tests that you ran to verify your changes. -->
- [ ] example: the test suite for my feature covers cases x, y, and z
- [ ] example: all tests pass with my change
- [ ] example: I check if my changes works with both PySide and PyQt
backends
      as there are small differences between the two Qt bindings.

- [ ] My PR is the minimum possible work for the desired functionality
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] If I included new strings, I have used `trans.` to make them
localizable.
For more information see our [translations
guide](https://napari.org/developers/translations.html).
Czaki pushed a commit that referenced this pull request Jun 21, 2023
…5669)

This fixes issues with Pan/Zoom buttons (layer controls) not working and
the spacebar keybinding to activate pan/zoom mode also not working.
In #4894 there was some refactoring
which introduced the bugs, see my investigation here:
#5654

This PR fixes the typo, lack of proper layer types, and now moves the
`hold_for_pan_zoom` to be a Viewer keybinding. As a result, this
keybinding isn't hard-coded anymore, but is now settable in the
Preferences > Shortcuts.

<!-- Please delete options that are not relevant. -->
- [x] Bug-fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] This change requires a documentation update

closes #5654

Good question! All tests were passing before this PR, despite the fact
that things were broken. So I'm open to suggestions how to improve tests
so we catch stuff better.

- [x] My PR is the minimum possible work for the desired functionality
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] If I included new strings, I have used `trans.` to make them
localizable.
For more information see our [translations
guide](https://napari.org/developers/translations.html).
Czaki pushed a commit that referenced this pull request Jun 21, 2023
# Description
closes #5677

I tracked down the source of the added Transform and Pan/zoom in the
Viewer keybindings.
Those are actions/keybinds added in
#4894 for the Tracks layer. But
Tracks layer wasn't added as an option in the shortcuts table, so they
appear in the default (Viewer).
With this PR, I import Tracks and add it to the dropdown so Tracks has
it's own table and now the actions appear in the proper table, rather
than under Viewer keybindings.

<img width="1198" alt="image"
src="https://user-images.githubusercontent.com/76622105/228287187-189cb745-fb6e-4c31-93f1-6ba22e965a56.png">

<img width="1198" alt="image"
src="https://user-images.githubusercontent.com/76622105/228287227-da2642b7-8f76-4451-bdfa-e6801f47400a.png">



## Type of change
<!-- Please delete options that are not relevant. -->
- [x] Bug-fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] This change requires a documentation update

# References


# How has this been tested?
<!-- Please describe the tests that you ran to verify your changes. -->
- [ ] example: the test suite for my feature covers cases x, y, and z
- [x] example: all tests pass with my change
- [ ] example: I check if my changes works with both PySide and PyQt
backends
      as there are small differences between the two Qt bindings.  

## Final checklist:
- [x] My PR is the minimum possible work for the desired functionality
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] If I included new strings, I have used `trans.` to make them
localizable.
For more information see our [translations
guide](https://napari.org/developers/translations.html).
@imagesc-bot
Copy link

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/creating-a-separate-cursor-in-the-view/88608/5

@imagesc-bot
Copy link

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/layer-legend-color/91512/6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api PR/Issue involving API design changes breaking change Breaking change requiring a deprecation notice feature New feature or request qt Relates to qt tests Something related to our tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants