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

Order items in list via drag an drop #399

Open
wants to merge 31 commits into
base: main
Choose a base branch
from
Open

Conversation

inukshuk
Copy link
Member

@inukshuk inukshuk commented Sep 17, 2019

  • Adjust item iterable drop target to show sort indicators above/below
  • Enable only in lists when the position column is active
  • Handle drop to re-order cached copy (if needed)
  • Handle drop and dispatch command
  • Implement command
  • Actually sort by 'position' (not just 'added')
  • Migration to populate position in list_items (currently unused) based on order by added
  • Manage position on insert into list (and remove?)

src/components/item/grid.js Outdated Show resolved Hide resolved
src/components/item/grid.js Outdated Show resolved Hide resolved
src/browser/tropy.js Outdated Show resolved Hide resolved
@@ -324,6 +369,42 @@ class ItemIterator extends Iterator {
}
}

const DropTargetSpec = {
drop({ list, onItemImport }, monitor) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Careful! Both drop and canDrop have changed since you copied them -- please use the latest version here or we're losing support for dropping URLs.

Copy link
Contributor

Choose a reason for hiding this comment

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

Here I copied what was on project view: returning true as default.

Copy link
Member Author

Choose a reason for hiding this comment

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

Please take a look at the original drop and canDrop which this PR removes (it changed after you copied it here when you rebased the branch): we need those here, otherwise this PR will revert the import URL functionality.

Copy link
Contributor

Choose a reason for hiding this comment

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

done
I brought the changes from ProjectView done on 019f4e2 to the drop & canDrop methods

Copy link
Member Author

Choose a reason for hiding this comment

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

There were more changes than just this one commit. You need to compare the branches: what this PR removes from project view needs to be identical to what is now in the item iterator

Copy link
Member Author

Choose a reason for hiding this comment

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

You're removing this code:
Screenshot from 2019-11-18 17-26-29

You're adding this code:
Screenshot from 2019-11-18 17-27-21
Screenshot from 2019-11-18 17-26-59


return {
dt: connect.dropTarget(),
isOverFile: isOver && type === NativeTypes.FILE,
Copy link
Member Author

Choose a reason for hiding this comment

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

isOverFile should also be true for dragging URLs

Copy link
Contributor

Choose a reason for hiding this comment

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

done

@inukshuk inukshuk force-pushed the feature/order_items branch 2 times, most recently from 1ce7e38 to 9d13346 Compare November 20, 2019 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants