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

Drag drop layers to rearrange #64

Merged
merged 10 commits into from
Nov 19, 2018
Merged

Conversation

sofroniewn
Copy link
Contributor

What does this PR do? Why are doing this? References?

This PR adds support for dragging and dropping layer widgets to rearrange them. Rearranging them changes the order of display of the images in the canvas. Multiple widgets can be simultaneously selected and rearranged at the same time. During dragging the layer name appears by the cursor.

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

How Has This Been Tested?

  • TestCase Run examples/layers.ipynb and manually drag and drop layer widgets

Final Checklist:

  • My PR is the possible minimum 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

@sofroniewn sofroniewn added the feature New feature or request label Nov 18, 2018
@sofroniewn sofroniewn requested a review from a team November 18, 2018 02:06
Copy link
Member

@kne42 kne42 left a comment

Choose a reason for hiding this comment

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

Looks great, thank you Nick!

Only comment I have is that I think it would seem more intuitive to have the divider appear when you're moving the layer back into its original place. i.e. I select a layer and try to move it around, currently a divider only appears before or after another layer if that space is not already adjacent to the selected layer.

@sofroniewn
Copy link
Contributor Author

@kne42 - thanks! On the dividers appearing when moving the layer back. We could just have the closest divider always highlighted, not sure how everyone else feels. It's an easy change to make. We can also revisit in a future PR after we spend some time playing around with it. Maybe next time at biohub you show me exactly the behaviour you mean and I'll have a better understanding of it.

@pep8speaks
Copy link

Hello @sofroniewn! Thanks for updating the PR.

Line 1:80: E501 line too long (114 > 79 characters)
Line 8:24: E231 missing whitespace after ','
Line 8:32: E231 missing whitespace after ','
Line 9:25: E231 missing whitespace after ','
Line 9:33: E231 missing whitespace after ','
Line 11:1: E302 expected 2 blank lines, found 1
Line 15:80: E501 line too long (123 > 79 characters)
Line 16:80: E501 line too long (127 > 79 characters)
Line 22:80: E501 line too long (87 > 79 characters)
Line 39:80: E501 line too long (82 > 79 characters)
Line 48:80: E501 line too long (83 > 79 characters)
Line 98:24: E225 missing whitespace around operator
Line 98:80: E501 line too long (102 > 79 characters)
Line 123:9: E265 block comment should start with '# '
Line 124:9: E265 block comment should start with '# '
Line 125:9: E265 block comment should start with '# '

Line 4:1: E302 expected 2 blank lines, found 1
Line 7:80: E501 line too long (137 > 79 characters)
Line 8:80: E501 line too long (133 > 79 characters)

Line 5:1: E302 expected 2 blank lines, found 1
Line 10:9: E265 block comment should start with '# '
Line 52:63: E231 missing whitespace after ','
Line 53:61: E231 missing whitespace after ','
Line 71:80: E501 line too long (86 > 79 characters)
Line 75:80: E501 line too long (100 > 79 characters)
Line 95:80: E501 line too long (100 > 79 characters)
Line 111:24: E225 missing whitespace around operator
Line 114:29: E231 missing whitespace after ','

@kne42
Copy link
Member

kne42 commented Nov 18, 2018

@sofroniewn Yeah that sounds reasonable. I will be at the Redwood City CZI office on Monday for a few hours though if you'd have time to chat :)

@sofroniewn sofroniewn mentioned this pull request Nov 18, 2018
9 tasks
Copy link
Contributor

@AhmetCanSolak AhmetCanSolak left a comment

Choose a reason for hiding this comment

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

Thank you @sofroniewn !

@sofroniewn sofroniewn merged commit f23e281 into napari:master Nov 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants