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

Bugfix: documentation update for napari PR #5636 #123

Conversation

psobolewskiPhD
Copy link
Member

Description

Currently, the Viewer tutorial describes the delete button (trash) as being able to accept drag-n-drop events, making it work like the Trash on say macOS (and other OS with a similar desktop model). However, this behavior was broken in napari/napari#2441

It's not intuitive, so the broken behavior is being removed in napari PR #5636
This companion PR updates the Viewer tutorial for this change.

Type of change

  • Fixes or improves existing content
  • Adds new content page(s)
  • Fixes or improves workflow, documentation build or deployment

References

Companion to napari/napari#5636

Final checklist:

  • My PR is the minimum possible work for the desired functionality
  • I have commented my code, particularly in hard-to-understand areas
  • I have added alt text to new images included in this PR

@brisvag
Copy link
Contributor

brisvag commented Mar 20, 2023

Would be nice if we could add (cross-repo) dependencies to PRs to prevent accidental merge. Is that even a thing?

@melissawm
Copy link
Member

I think this is what merge queues are supposed to do, but as I understand it this can't be used for multiple repositories (yet?)

@brisvag
Copy link
Contributor

brisvag commented Mar 20, 2023

I think this is what merge queues are supposed to do, but as I understand it this can't be used for multiple repositories (yet?)

Not quite, I think, even within a single repo. It doesn't seem to have a way to create individual dependency/dependent relationship, it's just making sure that any time you click merge, anything before that in the single queue will update its branch to account for the new PR. What I'm referring to is a way to prevent this PR from merging (as if the tests were failed), until the pr in the main repo is merged.

@melissawm
Copy link
Member

Maybe something like this? https://github.com/marketplace/actions/pr-dependency-check

@brisvag
Copy link
Contributor

brisvag commented Mar 20, 2023

Exactly like that, it even works across repos!

@psobolewskiPhD
Copy link
Member Author

psobolewskiPhD commented Mar 20, 2023

That seems awesome. Makes the most sense to make the docs depend on the napari PR, right? (Not to encourage people to not make parallel docs PRs, but just logically speaking...)
So we need to add that action to this repo?

@brisvag
Copy link
Contributor

brisvag commented Mar 20, 2023

I'm in favour, but I wouldn't make it mandatory, it's mostly a convenience for us to avoid mismerges. Might be a useful action to have in general in the main repo to help with those big splittable PRs we sometimes have (like the async work).

DragaDoncila pushed a commit to napari/napari that referenced this pull request Mar 21, 2023
…n and shortcut (#5636)

# Description

The current trash-icon delete button in the viewer when clicked deletes
selected layers, but is also supposed to accept drag-n-drop events,
making it work like the Trash on say macOS (and other OS with a similar
desktop model).
However, this behavior was broken in
#2441

This (broken) behavior makes no sense, because napari *doesn't use* a
desktop model, so the Trash icon is not a folder, like the Trash bin in
a OS, a temporary storage for unwanted items. You can't open the napari
trash button to restore the layers you drag there.

The Trash icon in napari should be simply a button—and is stylized as
one—that responds to a single-click to delete selected layers.

This PR removes the broken behavior—eliminating the source of error. In
this way it simplifies the code, to treat the delete button the same as
other viewer buttons (QtViewerPushButton). as a result, the delete
button now also has a registered action and keybinding that is editable
in the Preferences, rather than hard-coded.


## 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)
- [x] Breaking change (fix or feature that would cause existing
functionality to not work as expected): it's been broken for 2 years
without being reported...
- [x] This change requires a documentation update: docs PR:
napari/docs#123

# References
closes #5629
See also discussion on zulip:
https://napari.zulipchat.com/#narrow/stream/212875-general/topic/Drag.20layer.20to.20trash

# 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
- [ ] 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
- [x] I have made corresponding changes to the documentation: docs PR
napari/docs#123
- [ ] 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).
@DragaDoncila
Copy link
Contributor

Just had a look and merged #5636. Agreed we should set up the dependency thing, would be very handy. Going to merge this now.

@DragaDoncila DragaDoncila merged commit 2e89d61 into napari:main Mar 21, 2023
@brisvag brisvag mentioned this pull request Mar 21, 2023
6 tasks
kcpevey pushed a commit to kcpevey/napari that referenced this pull request Apr 6, 2023
…n and shortcut (napari#5636)

# Description

The current trash-icon delete button in the viewer when clicked deletes
selected layers, but is also supposed to accept drag-n-drop events,
making it work like the Trash on say macOS (and other OS with a similar
desktop model).
However, this behavior was broken in
napari#2441

This (broken) behavior makes no sense, because napari *doesn't use* a
desktop model, so the Trash icon is not a folder, like the Trash bin in
a OS, a temporary storage for unwanted items. You can't open the napari
trash button to restore the layers you drag there.

The Trash icon in napari should be simply a button—and is stylized as
one—that responds to a single-click to delete selected layers.

This PR removes the broken behavior—eliminating the source of error. In
this way it simplifies the code, to treat the delete button the same as
other viewer buttons (QtViewerPushButton). as a result, the delete
button now also has a registered action and keybinding that is editable
in the Preferences, rather than hard-coded.


## 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)
- [x] Breaking change (fix or feature that would cause existing
functionality to not work as expected): it's been broken for 2 years
without being reported...
- [x] This change requires a documentation update: docs PR:
napari/docs#123

# References
closes napari#5629
See also discussion on zulip:
https://napari.zulipchat.com/#narrow/stream/212875-general/topic/Drag.20layer.20to.20trash

# 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
- [ ] 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
- [x] I have made corresponding changes to the documentation: docs PR
napari/docs#123
- [ ] 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).
melissawm pushed a commit to melissawm/napari-docs that referenced this pull request Apr 26, 2023
# Description

Currently, the Viewer tutorial describes the delete button (trash) as
being able to accept drag-n-drop events, making it work like the Trash
on say macOS (and other OS with a similar desktop model). However, this
behavior was broken in napari/napari#2441

It's not intuitive, so the broken behavior is being removed in napari PR
[#5636](napari/napari#5636)
This companion PR updates the Viewer tutorial for this change.

## Type of change
<!-- Please delete options that are not relevant. -->
- [x] Fixes or improves existing content
- [ ] Adds new content page(s)
- [ ] Fixes or improves workflow, documentation build or deployment

# References
Companion to napari/napari#5636

## 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 added [alt text](https://webaim.org/techniques/alttext/) to
new images included in this PR
Czaki pushed a commit that referenced this pull request May 4, 2023
# Description
Followup on [these
comments](#123 (comment)),
adding a dependency check for PRs.

This will automatically detect stuff like:
- depends on #123
- blocked by napari/napari#5650

and it also works cross-repo (which is especially useful here to ensure
we merge docs only if the feature actually landed in napari core).


<!-- What does this pull request (PR) do? Why is it necessary? -->
<!-- Tell us about your new content, improvement, or fix! -->
<!-- If you can, 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 -->

## Type of change
<!-- Please delete options that are not relevant. -->
- [ ] Fixes or improves existing content
- [ ] Adds new content page(s)
- [x] Fixes or improves workflow, documentation build or deployment

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

## Final checklist:
- [ ] My PR is the minimum possible work for the desired functionality
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added [alt text](https://webaim.org/techniques/alttext/) to
new images included in this PR
Czaki pushed a commit to napari/napari that referenced this pull request Jun 19, 2023
…n and shortcut (#5636)

The current trash-icon delete button in the viewer when clicked deletes
selected layers, but is also supposed to accept drag-n-drop events,
making it work like the Trash on say macOS (and other OS with a similar
desktop model).
However, this behavior was broken in
#2441

This (broken) behavior makes no sense, because napari *doesn't use* a
desktop model, so the Trash icon is not a folder, like the Trash bin in
a OS, a temporary storage for unwanted items. You can't open the napari
trash button to restore the layers you drag there.

The Trash icon in napari should be simply a button—and is stylized as
one—that responds to a single-click to delete selected layers.

This PR removes the broken behavior—eliminating the source of error. In
this way it simplifies the code, to treat the delete button the same as
other viewer buttons (QtViewerPushButton). as a result, the delete
button now also has a registered action and keybinding that is editable
in the Preferences, rather than hard-coded.

<!-- 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)
- [x] Breaking change (fix or feature that would cause existing
functionality to not work as expected): it's been broken for 2 years
without being reported...
- [x] This change requires a documentation update: docs PR:
napari/docs#123

closes #5629
See also discussion on zulip:
https://napari.zulipchat.com/#narrow/stream/212875-general/topic/Drag.20layer.20to.20trash

<!-- 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.

- [x] My PR is the minimum possible work for the desired functionality
- [ ] I have commented my code, particularly in hard-to-understand areas
- [x] I have made corresponding changes to the documentation: docs PR
napari/docs#123
- [ ] 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 to napari/napari that referenced this pull request Jun 21, 2023
…n and shortcut (#5636)

The current trash-icon delete button in the viewer when clicked deletes
selected layers, but is also supposed to accept drag-n-drop events,
making it work like the Trash on say macOS (and other OS with a similar
desktop model).
However, this behavior was broken in
#2441

This (broken) behavior makes no sense, because napari *doesn't use* a
desktop model, so the Trash icon is not a folder, like the Trash bin in
a OS, a temporary storage for unwanted items. You can't open the napari
trash button to restore the layers you drag there.

The Trash icon in napari should be simply a button—and is stylized as
one—that responds to a single-click to delete selected layers.

This PR removes the broken behavior—eliminating the source of error. In
this way it simplifies the code, to treat the delete button the same as
other viewer buttons (QtViewerPushButton). as a result, the delete
button now also has a registered action and keybinding that is editable
in the Preferences, rather than hard-coded.

<!-- 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)
- [x] Breaking change (fix or feature that would cause existing
functionality to not work as expected): it's been broken for 2 years
without being reported...
- [x] This change requires a documentation update: docs PR:
napari/docs#123

closes #5629
See also discussion on zulip:
https://napari.zulipchat.com/#narrow/stream/212875-general/topic/Drag.20layer.20to.20trash

<!-- 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.

- [x] My PR is the minimum possible work for the desired functionality
- [ ] I have commented my code, particularly in hard-to-understand areas
- [x] I have made corresponding changes to the documentation: docs PR
napari/docs#123
- [ ] 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 to napari/napari that referenced this pull request Jun 21, 2023
…n and shortcut (#5636)

The current trash-icon delete button in the viewer when clicked deletes
selected layers, but is also supposed to accept drag-n-drop events,
making it work like the Trash on say macOS (and other OS with a similar
desktop model).
However, this behavior was broken in
#2441

This (broken) behavior makes no sense, because napari *doesn't use* a
desktop model, so the Trash icon is not a folder, like the Trash bin in
a OS, a temporary storage for unwanted items. You can't open the napari
trash button to restore the layers you drag there.

The Trash icon in napari should be simply a button—and is stylized as
one—that responds to a single-click to delete selected layers.

This PR removes the broken behavior—eliminating the source of error. In
this way it simplifies the code, to treat the delete button the same as
other viewer buttons (QtViewerPushButton). as a result, the delete
button now also has a registered action and keybinding that is editable
in the Preferences, rather than hard-coded.

<!-- 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)
- [x] Breaking change (fix or feature that would cause existing
functionality to not work as expected): it's been broken for 2 years
without being reported...
- [x] This change requires a documentation update: docs PR:
napari/docs#123

closes #5629
See also discussion on zulip:
https://napari.zulipchat.com/#narrow/stream/212875-general/topic/Drag.20layer.20to.20trash

<!-- 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.

- [x] My PR is the minimum possible work for the desired functionality
- [ ] I have commented my code, particularly in hard-to-understand areas
- [x] I have made corresponding changes to the documentation: docs PR
napari/docs#123
- [ ] 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 to napari/napari that referenced this pull request Jun 21, 2023
…n and shortcut (#5636)

The current trash-icon delete button in the viewer when clicked deletes
selected layers, but is also supposed to accept drag-n-drop events,
making it work like the Trash on say macOS (and other OS with a similar
desktop model).
However, this behavior was broken in
#2441

This (broken) behavior makes no sense, because napari *doesn't use* a
desktop model, so the Trash icon is not a folder, like the Trash bin in
a OS, a temporary storage for unwanted items. You can't open the napari
trash button to restore the layers you drag there.

The Trash icon in napari should be simply a button—and is stylized as
one—that responds to a single-click to delete selected layers.

This PR removes the broken behavior—eliminating the source of error. In
this way it simplifies the code, to treat the delete button the same as
other viewer buttons (QtViewerPushButton). as a result, the delete
button now also has a registered action and keybinding that is editable
in the Preferences, rather than hard-coded.

<!-- 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)
- [x] Breaking change (fix or feature that would cause existing
functionality to not work as expected): it's been broken for 2 years
without being reported...
- [x] This change requires a documentation update: docs PR:
napari/docs#123

closes #5629
See also discussion on zulip:
https://napari.zulipchat.com/#narrow/stream/212875-general/topic/Drag.20layer.20to.20trash

<!-- 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.

- [x] My PR is the minimum possible work for the desired functionality
- [ ] I have commented my code, particularly in hard-to-understand areas
- [x] I have made corresponding changes to the documentation: docs PR
napari/docs#123
- [ ] 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).
@psobolewskiPhD psobolewskiPhD added this to the 0.4.18 milestone Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants