Awful / Awful.app

Something Awful Forums browser for iOS
https://forums.somethingawful.com/showthread.php?threadid=3837546&perpage=40&noseen=1
136 stars 45 forks source link

V1 of the double tap to scroll to the end of posts feature. Here goes #1071

Closed dfsm closed 3 years ago

dfsm commented 3 years ago

hello, commie kong here. here's the double tap to scroll long posts patch

my first ios thing and also my first github PR. hope it finds you well

Let me know if i've stuffed anything up or if the code is bad (likely)

for reference: https://forums.somethingawful.com/showthread.php?threadid=3837546&pagenumber=96#post512330106

nolanw commented 3 years ago

@dfsm This is awesome! Thanks so much. And congrats on your first iOS thing!

I wanted to make sure what I mentioned in comments was actually possible before suggesting it. That was more difficult than I expected, so I thought it'd be helpful to push my fiddling: https://github.com/Awful/Awful.app/commit/cc9f609b1baae0ab8c9580e819a293ee667ab6bb.

The JavaScript gets way simpler if we just pass the double-tapped post's rect instead of finding the next post, and I'm a big fan of less JavaScript, so I gave that a try. I also switched to UIScrollView's scrollRectToVisible() method, because setting the contentOffset directly can sometimes get us in weird situations (e.g. if I double-tap the top post right after loading the page, it scrolls the post down and leaves a weird gap above it that disappears as soon as I try to scroll).

Let me know if it feels right to you!

dfsm commented 3 years ago

@nolanw Haha that "first post double-tap = weird gap" thing is an issue I introduced recently. It didn't always do that. But then I changed something...

I tested your much improved version and it's good. Double-tapping the first post overshoots slightly (by about the height of the bottom/top menu) while testing on an 11 Pro Max simulator. It always did overshoot a little bit on the first post, tho

Also, I did try scrollRectToVisible() at one point, but I enjoyed being able to double-tap on posts with points that were actually visible. Like scrolling up a little and tapping the last sentence of a previous post would move the footer of that post to the bottom of the screen (i.e. scroll up).

That might just be something that I found fun while testing it, though. Not sure if anyone would actually want to do that in reality. It's not something I'd ask to be put back in

Thank you for making the changes yourself. It's really interesting to see what you've done. Guess I didn't need all that stuff for a single setting, eh? Now I know :)

nolanw commented 3 years ago

I squashed some of my changes in and merged as 5b9b51a4d91aae72843cf35af67ee130d20e7c83. Thanks again!

nolanw commented 3 years ago

@dfsm I sent you an invite for write access to this repo. I'm more than happy to continue with PRs (feel free to open branches on your fork or branches here), but if/when you're confident pushing updates directly to main that's cool too :)