Q-Mobile / QGrid

🎛 QGrid: The missing SwiftUI collection view.
MIT License
1.64k stars 104 forks source link

Make scrolling optional #28

Closed bryanbartow closed 4 years ago

bryanbartow commented 4 years ago

Add the isScrollable property to make embedding in a ScrollView optional

kevin-hv commented 4 years ago

@bryanbartow For me this change does not work as expected, the QGrid's size is not calculated automatically based on the elements height & width, so it cannot be properly scrolled.

karolkulesza commented 4 years ago

@bryanbartow : Thanks for contributing!

@kevin-hv : I've just verified that using PeopleView in QGridTestApp: After you specify isScrollable: false for QGrid, you'll need to specify the .frame(height: ...) for the QGrid cell content, as in the following example:

struct GridCell: View {
  var person: Person

  var body: some View {
    VStack() {
      Image(person.imageName)
        .resizable()
        .scaledToFit()
        .clipShape(Circle())
        .shadow(color: .primary, radius: 5)
        .padding([.horizontal, .top], 7)
      Text(person.firstName).lineLimit(1)
      Text(person.lastName).lineLimit(1)
    }
    .font(.headline).foregroundColor(.white)
    .frame(height: 150)
  }
}
kevin-hv commented 4 years ago

@karolkulesza , @bryanbartow Sorry I was not clear enough, I was testing it with the following code

enum Test : Int, Identifiable, CaseIterable {

    var id: Int {
        return self.rawValue
    }

    case o1, o2, o3, o4, o5, o6, o7, o8, o9 ,o10, o11, o12, o13, o14, o15, o16, o17, o18, o19, o20, o21, o22, o23, o24, o25, o26, o27, o28, o29, o30

}
ScrollView{
            VStack{

                QGrid(Test.allCases, columns: 3, isScrollable: false){
                    i in
                    Circle()
                    .frame(width: 50, height: 50)
                }
            }
        }

And I just tested again and it does not seem to scroll; am I doing something wrong here? I think I specify the element's frame properly.

bryanbartow commented 4 years ago

@kevin-hv You're right. Using this code in an empty project produces the same result you're getting.

Oddly, using this in my production app does not product the same result. For some reason it works as intended.

cc @karolkulesza

rubengarciam commented 4 years ago

Changing the isScrollable property produces a different output for me. My (rough) structure:

List {
   ... foreach
   Section {
      QGRid(.. isScrollable: true/false) {
         NavigationLink {
            Image
         }
      }
   }
{

If isScrollable=true it works as intended and each image of the grid navigates to its own view.

If isScrollable=false the non-scrolling works as expected but the all items in the grid become a single NavigationLink (?), opening as many views as linked within the grid