Talk Page Archives - Part 1 #4441
Conversation
mazevedofs
left a comment
mazevedofs
left a comment
There was a problem hiding this comment.
Working great, looking forward see this last bit of talk pages done!
| } | ||
|
|
||
| // Note: This subclass only seems necessary for proper nav bar hiding in iOS 14 & 15. It can be removed and switched to raw UIHostingControllers for iOS16+ | ||
| private class NavigationBarHidingHostingVC<Content: View>: UIHostingController<Content> { |
There was a problem hiding this comment.
It would be good to prevent the bar from hiding when VoiceOver is on or provide another way for VoiceOver users to go back from this view when it is hiding (This can be done on the part 2 PR).
There was a problem hiding this comment.
Great catch, I hadn't thought about Voice Over yet. 😅 I'll spin it off into a Part 3 PR since Part 2 was getting pretty big.
| fileprivate var overflowSubmenuActions: [UIAction] { | ||
|
|
||
| let goToArchivesAction = UIAction(title: TalkPageLocalizedStrings.archives, image: UIImage(systemName: "archivebox"), handler: { _ in | ||
| let goToArchivesAction = UIAction(title: TalkPageLocalizedStrings.archives, image: UIImage(systemName: "archivebox"), handler: { [weak self] _ in |
There was a problem hiding this comment.
So glad this warning will be gone 😅
staykids
left a comment
staykids
left a comment
There was a problem hiding this comment.
@tonisevener Very cool to see this. Clever direct usage of the underlying scroll view offset - whatever works!
Below are some notes from my first pass. I don't know how important some of these may be depending on your goals for this work and how much you anticipate we'll reuse it, but I wanted to share what stood out:
-
A common behavior we do is ignoring touches on the header to pass touches through to the underlying scroll view to allow scrolling while interacting with the header area. Our view hierarchy here is a little different - how might we support that behavior here?
-
Another existing behavior with these header views is "snapping" the header view/navigation bar's state to either entirely closed/hidden or entirely open/visible depending on how much of the view the user has scrolled. For example, on the Explore feed, scrolling the content begins collapsing and fading the navigation bar, but if the user lets go we don't leave the bar in a faded and offset state (akin to our old friend the programmatic scroll header partially visible bug), but instead either completely hide or show the view. I think logically something like if the user has scrolled the collapsing view more than 50% closed, snap to a hidden state, and if the collapsed view is more than 50% open snap to a visible state.
-
What do you think about removing
shiftOrderentirely? It's an extra annoyance to manage in creating new conforming views that I think might not be necessary. I think it's reasonable if we just respect the order of the views added toshiftingTopViews, and inShiftingTopViewsStack.swiftjust iterate the top views in reverse order:for view in self.shiftingTopViews.reversed()to get the behavior we want, where the header view (or any other views going up the line) collapses before the navigation bar. -
I guess it's more a concern/responsibility we'll have to remember for implementing new
ShiftingTopViews, but the demo one doesn't respect safe area guidelines and draws under the Dynamic Island when in landscape right orientation. -
I'm happy with whatever name you choose here, just sharing some other prefixes if they spark any other ideas 😀:
ScrollHiding,ScrollCollapsing,ScrollHideable,ScrollCollapsible.
|
FYI - I spun up T327693 so that this wonderful PR feedback is not forgotten.
@staykids Thanks - yeah I think that's just a bug with the demo header view and we'll just have to keep safe area in mind when we make these views. I didn't want to align the stack view itself to the safe area, just in case certain header elements need to expand all the way to the edge for the background color (like talk page coffee roll). |
3 participants
Phabricator:
https://phabricator.wikimedia.org/T321853
Notes
This is some foundational work for talk page archives. I've been wanting to use this simple list feature as an opportunity to tinker with alternative navigation and header views for SwiftUI. Our custom navigation setup (currently in ViewController.swift) is heavily tied to UIKit scroll views, so in order to lean on SwiftUI ScrollViews, I wrote a different setup. Here's loosley how it works, but I'm happy to explain further in PR review if needed.
UIHostingController, which in turn is added as a child view controller toTalkPageArchivesViewControllerTalkPageArchivesViewControlleralso adds aUIStackViewof the at the top of its view. This stack view holds the navigation bar and header view.ShiftingTopViewsDatathat holds the scroll view offset and total height of the stack view. SwiftUI updates the scroll view offset inShiftingTopViewsData, and the stack view listens to that value and adjusts its subviews accordingly. In turn the stack view updates its total height inShiftingTopViewsData, and the SwiftUI scroll view listens to that and adds top padding accordingly.I have added some mock UI to play with - I will delete it in the next PR.
I went with a super literal "ShiftingTopView" prefix everywhere, but happy to change this if it sounds weird. This was partly to distinguish it from the old system, and partly to avoid anything that sounded like a
UINavigationControllersubclass to reduce confusion.If you're interested in where this is headed, some prototype work is in the
talk-page-archives-prototypebranch. There I have it implemented onTalkPageViewController, and confirmed the sticky header issue is gone :).Test Steps