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

add initialSourceIndexPath and finalDestinationIndexPath #9

Merged
merged 5 commits into from Sep 2, 2017
Merged

add initialSourceIndexPath and finalDestinationIndexPath #9

merged 5 commits into from Sep 2, 2017

Conversation

RickeyBoy
Copy link
Contributor

When I intent to send a urlRequest that inform back-end the changes of model, I found that I can only do this in tableview(_:reorderRowAt:to).
However this means that during the Drag&Drop I have to send lots of urlRequest which is not necessary.
So I added two properties to help.
Usage:

func tableViewDidFinishReordering(_ tableView: UITableView) {
guard let rowS = tableView.reorder.initialSourceIndexPath?.row,
let rowD = tableView.reorder.finalDestinationIndexPath?.row else { return }
print("move (rowS) to (rowD)")
}

(This is my first pull request. Hope you don't mind if I make some mistakes. )

@adamshin
Copy link
Owner

adamshin commented Aug 29, 2017

Hey, thanks for contributing. This could definitely be useful.

I think initialSourceIndexPath and finalDestinationIndexPath might make more sense as parameters to the tableViewDidFinishReordering delegate method. That way there's no mutable state & no need to unwrap optionals. What do you think?

@RickeyBoy
Copy link
Contributor Author

Yeah, I think you're right. And I changed initialSourceIndexPath and finalDestinationIndexPath to private.

Usage now becomes:

func tableViewDidFinishReordering(_ tableView: UITableView, initSource: IndexPath, finalDestination: IndexPath) {
print("(initSource) to (finalDestination)")
}

@adamshin
Copy link
Owner

You should be able to remove the initialSourceIndexPath and finalDestinationIndexPath properties altogether - both those values are available in the ReorderState.reordering case. We're unpacking the destination indexpath at the beginning of endReorder(), and we can access the source indexpath the same way. Currently that value's just being ignored.

@RickeyBoy
Copy link
Contributor Author

I've removed those two properties in the way you said. Hope it works well now :)

@adamshin
Copy link
Owner

adamshin commented Sep 1, 2017

Looks good thanks!

One last thought: can you change the parameter names in that delegate method to

tableViewDidFinishReordering(_ tableView: UITableView, from initialSourceIndexPath: IndexPath, to finalDestinationIndexPath: IndexPath)?

A little more verbose, but consistent with the other methods.

@RickeyBoy
Copy link
Contributor Author

All done! Really appreciate your patience and efforts- definitely made my first contributing experience an amazing one! Looking forward to cooperation in the future :)

@adamshin adamshin merged commit 7235482 into adamshin:master Sep 2, 2017
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

2 participants