-
Notifications
You must be signed in to change notification settings - Fork 31
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
Label sorting with drag and drop #575
Conversation
} | ||
|
||
// Destroy existing Sortable instances if any | ||
if (self.sortables) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR looks great and works well!
The only question I have is about the destroy()
here...
If I create a sortable, then select a different panel in the figure then this SelectedPanelsLabelsView
gets lost without destroying the selectable(s) that were created.
I don't know if that really matters - probably not. Does it hurt if we don't clear destroy them? E.g. use up memory etc??
If we do want to make sure they all get destroyed, I wonder if a more global list of sortables (instead of just this
component) or possibly try to handle the removal of the component and destroy them there. E.g. see
omero-figure/src/js/views/right_panel_view.js
Line 756 in 6456608
clear: function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I followed the clear example and now destroy the sortables when the panel is deselected. Thanks for spotting that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix - testing locally looks good and functional testing on merge-ci is behaving as expected 👍
This is now released in omero-figure 7.1.0 |
Hello Will,
here's a PR to sort labels with a drag-and-drop. The cursor changes when hovering the labels, and moving the labels up or down, the order of the labels will be updated.
For the tests I conducted it works well. This handles:
This is the work from my new student Tehmina who is helping me with changes I'd like to make. @MinaEnayat