slackhq / PanModal

An elegant and highly customizable presentation API for constructing bottom sheet modals on iOS.
MIT License
3.69k stars 533 forks source link

TableView scroll doesn't work on smaller screen devices while works on bigger. #93

Open hamdshah opened 4 years ago

hamdshah commented 4 years ago

Description

Describe your issue here.

Tableview scroll works on devices with bigger screens e.g iPhone 11 pro Max and but doesn't work on smaller devices e.g iPhone 8/6S etc

What type of issue is this? (place an x in one of the [ ])

Requirements (place an x in each of the [ ])


Bug Report

Filling out the following details about bugs will help us solve your issue sooner.

Reproducible in:

PanModal version:

iOS version: 13.3

Steps to reproduce:

  1. Implement any simple implementation of PandModel
  2. Run it on two different size of devices even simulator have the same issue
  3. try to scroll

Expected result:

Actual result:

What actually happened

Attachments:

Logs, screenshots, screencast, sample project, funny gif, etc.

Screen Recording 2020-04-14 at 5 11 09 PM

apptekstudios commented 4 years ago

Try setting alwaysBounceVertical to true on your tableView πŸ‘ Looks like the content on the iPhone 8 isn't tall enough to enable scrolling.

demon9733 commented 4 years ago

Hey guys! I faced just the same issue, and setting alwaysBounceVertical to true doesn't help.

A bit more details:

  1. I have a scroll view with the content size of 3-5 heights of the phone screen.
  2. I set panScrollable from PanModalPresentable protocol to my scroll view.
  3. Everything works just fine on bezel-less iPhones.
  4. Scroll doesn't work entirely on iPhones with the Home button.
  5. When I set panScrollable back to nil, scroll starts working once again, but in this case, it's really hard to dismiss the card, because it takes almost the full screen.

Any suggestions are appreciated πŸ™ŒπŸ™ŒπŸ™Œ

demon9733 commented 4 years ago

Bump.

qwertycommander commented 4 years ago

@ste57 @TosinAF -- The issue is not one off. I currently have implemented PanModalPresentable, and I get it to work with a table view for larger iPhones, but the iPhone SE does not work. The entire table view does not allow for any scrolling on the iPhone SE. Would greatly appreciate any help

abdulazizSi commented 4 years ago

Same here!

qwertycommander commented 4 years ago

@abdulazizSi @demon9733 @hamdshah

I figured out a fix that worked for me. I had the short form height the same as the long form height. Try this method out and report back.

    func shouldPrioritize(panModalGestureRecognizer: UIPanGestureRecognizer) -> Bool {
        let location = panModalGestureRecognizer.location(in: view)
//        You can do something with the location value, 
// like determine if it is in a place where you want the gesture recognizer to prioritize, but returning 
// false just solved my cases.
        return false
    }

This method is a method to be implemented by PanModalPresentable protocol.

abdulazizSi commented 4 years ago

@hamdshah @demon9733 @qwertycommander

What worked for me is just returning nil to "panScrollable" var panScrollable: UIScrollView? { return nil }

demon9733 commented 4 years ago

@qwertycommander Unfortunately that didn't work for me. Scroll still doesn't work entirely πŸ˜•. Setting different values for shortFormHeight and longFormHeight also didn't help.

@abdulazizSi Setting the panScrollable to nil indeed fixes the issue (I mentioned this in my first comment in this thread), but I really want to dismiss the card interactively.

What I ended up with is:

  1. Setting the panScrollable only for large devices:

    var panScrollable: UIScrollView? {
    if Device.isSizeOrLarger(.Inches_5_8) {
        return textScrollView
    } else {
        return nil
    }
    }
  2. Making the longFormHeight a bit smaller to make it easier for the user to dismiss on small devices:

    var longFormHeight: PanModalHeight {
    if Device.isSizeOrLarger(.Inches_5_8) {
        return .maxHeight
    } else {
        return .maxHeightWithTopInset(30)
    }
    }

I really hope to get this fixed soon since this behavior is really unsatisfying.

abdulazizSi commented 4 years ago

@hamdshah @demon9733 @qwertycommander

Doing this fix the problem , tested on simulator with UITableView and UIScrollView 🀯🀯 var allowsExtendedPanScrolling: Bool { return true }

AliJ91 commented 2 weeks ago

var allowsExtendedPanScrolling: Bool { return true }

@abdulazizSi how did you implement this to a tableView? can u provide more details?