apptekstudios / ASCollectionView

A SwiftUI collection view with support for custom layouts, preloading, and more.
MIT License
1.36k stars 160 forks source link

Update collection view selection handling #177

Closed atdrendel closed 3 years ago

atdrendel commented 4 years ago

This pull request changes selection handling in ASCollectionView in a number of ways.

  1. Remove the limitation on selection only being allowed while editing. Now, selection or multiple selection can be triggered enabled/disabled through the use of the allowsSelection(_:) and allowsMultipleSelection(_:) modifiers on ASCollectionView.
  2. Rename ASSection.selectedItems to selectedIndexes to clarify the property holds indexes, not the model objects themselves.
  3. Make ASCollectionView respond to changes to selectedIndexes from the SwiftUI side. In other words, the Binding<Set<Int>>? has become a true, two-way binding, which is what Apple says Bindings should be.
  4. Add support for preventing selection/deselection for specific indexes in a section
  5. Update Waterfall layout example to add a sheet detail view

The demo app's screens have been updated to reflect these changes.

Closes #176

atdrendel commented 4 years ago

This might be a...controversial...change, but I wanted to put it out there in case y'all would find it useful. This pull request does not make any changes to ASTableView because we don't use it in our app, but I'd be more than willing to update that, if y'all think this change has a place in this library.

apptekstudios commented 4 years ago

Thanks @atdrendel this seems like a good addition. I'm just adding support to ASTableView and will include in the upcoming version 👍

atdrendel commented 4 years ago

@apptekstudios I'd be happy to add the ASTableView support to this pull request. I could probably do that sometime this week, if you wanted. The only reason I didn't add it immediately to this pull request is I didn't know if this would be a change you'd want to accept and, since we don't use ASTableView in our app, I didn't want to put in the time unless you were sure you'd be open to merging this change.

apptekstudios commented 4 years ago

If you check out the v1.8.0 branch I have actually done it already today (now that the iOS 14 rush is finally over 😆 ).

I did take a slightly different approach to declaring selection support, although I'd like to hear if you have a use case that this doesn't support: You pass a selectionMode enum setting for each section saying whether to support

.none
.highlighting(Binding<Set<Int>>)
.selectSingle((Int) → Void)
.selectMultiple(Binding<Set<Int>>)

This makes the assumption that you don't care about highlight state if multiple selection is enabled. Is this correct as you see it?