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

Improve dismissing ScrollView #79

Merged

Conversation

dstranz
Copy link
Contributor

@dstranz dstranz commented Jul 2, 2019

  • Support swipe in UIViewController embedded in UINavigationController
  • Fix position of UITableView header view - reason: applying transformation to tableHeaderView itself - as one of scrollView.subviews doesn't work as expected, it need's to be applied to header view content

@ivanvorobei
Copy link
Owner

You can attach gif or video with bug which you fix?

@dstranz
Copy link
Contributor Author

dstranz commented Jul 3, 2019

Fix position of UITableView header view - reason: applying transformation to tableHeaderView itself - as one of scrollView.subviews doesn't work as expected, it need's to be applied to header view content
SPStorkController - 1

@dstranz
Copy link
Contributor Author

dstranz commented Jul 3, 2019

Support swipe in UIViewController embedded in UINavigationController

This one is a code only bug. When you are showing UITableViewController embedded in UINavigationController it's not possible to dismiss it with swipe.

@Tavernari
Copy link

Support swipe in UIViewController embedded in UINavigationController

This one is a code only bug. When you are showing UITableViewController embedded in UINavigationController it's not possible to dismiss it with swipe.

Hi, I am facing to this issue and I created a PR just for this case

Could you test this Pull Request? #77

Maybe it'll resolve this issue.

Best

@dstranz
Copy link
Contributor Author

dstranz commented Jul 4, 2019

@Tavernari Nope, it doesn't helped when I'm showing UITableViewController embedded in UINavigationViewController as Stork view.

If I have only UITableViewController presented as Stork view it works fine but it works without your PR also. It only needs to add:
func scrollViewDidScroll(_ scrollView: UIScrollView) { SPStorkController.scrollViewDidScroll(scrollView) }
as mentioned in Readme.

@ivanvorobei
Copy link
Owner

@dstranz you can attach simple project with bug?
I am added header view and for me all work correctly.
I need see how it bug create
Thanks!

@dstranz
Copy link
Contributor Author

dstranz commented Jul 5, 2019

@ivanvorobei I've added this code

let headerView = UIView(frame: .init(x: 0, y: 0, width: tableView.frame.height, height: 100))
headerView.backgroundColor = .red
tableView.tableHeaderView = headerView

to viewDidLoad() in ModalTableViewController.swift in Stork demo app.

@ivanvorobei
Copy link
Owner

@dstranz thanks!
I am added it, also apply your changes, but bug actual. See code and simulator preview:

Screenshot 2019-07-05 at 14 24 07

@dstranz
Copy link
Contributor Author

dstranz commented Jul 5, 2019

@ivanvorobei OK, sorry. I wasn't precise. I haven't got any idea how to move headerView itself. That's the reason why I apply transformations to tableHeaderView.subviews.

But I will be of course the best to move tableHeaderView itself if we would know how to do that.

@ivanvorobei
Copy link
Owner

@dstranz I just see that this does not fix the situation. If this helped you, check if you have added any changes. So far, what I see is not working

@dstranz
Copy link
Contributor Author

dstranz commented Jul 5, 2019

@ivanvorobei That's the code that could replicate my case - for it my fix helps:

        let headerView = UIView(frame: .init(x: 0, y: 0, width: tableView.frame.height, height: 100))
        headerView.backgroundColor = .clear
        
        let testView = UIView(frame: headerView.frame)
        testView.backgroundColor = .red
        testView.translatesAutoresizingMaskIntoConstraints = false
        headerView.addSubview(testView)
        
        NSLayoutConstraint.activate([
            headerView.leadingAnchor.constraint(equalTo: testView.leadingAnchor),
            headerView.trailingAnchor.constraint(equalTo: testView.trailingAnchor),
            headerView.topAnchor.constraint(equalTo: testView.topAnchor),
            headerView.bottomAnchor.constraint(equalTo: testView.bottomAnchor),
        ])
        
        tableView.tableHeaderView = headerView

But it will be better to find better solution to move whole tableHeaderView - if you know how to move it ;)

@dstranz
Copy link
Contributor Author

dstranz commented Jul 5, 2019

@ivanvorobei I've just found a better solution for this issue, just transform whole UIScrollView:
scrollView.transform = CGAffineTransform(translationX: 0, y: -translation)

It works fine with #79 (comment) and #79 (comment)

@dstranz
Copy link
Contributor Author

dstranz commented Jul 5, 2019

Fixed

@ivanvorobei ivanvorobei merged commit 07abf8e into ivanvorobei:master Jul 5, 2019
@ivanvorobei
Copy link
Owner

@dstranz it work and I am merged

but problem with scroll indicator.... incorrect top inset. When I am use translation for each subviews, scroll save position and not change indicator inset.

Now indicator translate with scroll..

@ivanvorobei
Copy link
Owner

I also found a critical bug. If you use this code, a white area will appear from below if you pull the controller down. It is very strange.

While I returned the old version.

See screenshoot :

IMG_309ADFB69B52-1

@dstranz
Copy link
Contributor Author

dstranz commented Jul 9, 2019

@ivanvorobei Sorry, I didn't found that while testing (but it's also visible on my gif).

Do you have any idea how to fix tableHeaderView bug without adding this white space on the bottom?

@ivanvorobei
Copy link
Owner

@dstranz now I recommended create custom table view cell.

In next... I am try found solution

@iammxrn
Copy link

iammxrn commented Sep 29, 2019

Hi! Any updates?

ivanvorobei added a commit that referenced this pull request Mar 27, 2022
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

4 participants