You can test the changes on this Pull Request by downloading it from AppCenter here with build number: 32225. IPA is available here. If you need access to this, you can ask a maintainer to add you.

Generated by 🚫 dangerJS

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@iamthomasbishop

One more thing I'd like is to try applying the regular or chrome system material to the navbar and footer bar (in iOS 12+, iirc), so when scrolling they would become translucent. Is that doable here, @chipsnyder ?

It looks like chrome system material is available on iOS 13+ so I can apply the standard blur on lower versions (this is what the footer had on previous builds).

The way I have the header set up I'll need to make some changes to the layout to get that working but I can get that wrapped up in the morning probably.

As for the animation of the title view, I got that setup. Let me know what you think about it. The only piece I couldn't get working great is when you scroll quickly you'll see both headers as one scrolls into view and the other is animating out.

I just triggered a new build so that should be ready soon if you want to give it a try while I adjust the header.

The way I have the header set up I'll need to make some changes to the layout to get that working but I can get that wrapped up in the morning probably.

Sounds good 👍

As for the animation of the title view, I got that setup. Let me know what you think about it. The only piece I couldn't get working great is when you scroll quickly you'll see both headers as one scrolls into view and the other is animating out.

This feels really solid. The momentary double-title thing doesn't bother me a whole lot, but if you are able to solve it then even better. 😀

I did a fast test anchoring the title's alpha value to the largeTitleView.frame.maxY value and got an interesting result:

I'm going to revisit this path to solve the double case. It looks like you're using a different anchor point than I was. The way I tried it originally was phasing in the other small title by the % that the big title was off the screen and it didn't look great, because you would see both.

I'm not sure I fully understand what you mean by anchoring the title's alpha value to the largeTitleView.frame.maxY The way I have this set up is it animation will begin animating once the view disappears.

Can you share the snippit you used for your quick test?

@iamthomasbishop and @etoledom This is updated now with a new build (32185) generated for the header chrome style. However, It's hard to see the difference without setting the background color for cells. So we might want to focus on that in the next PR when I add some content to the rows.

I also didn't make any changes to the header animation. I started building in a cancel for that, but it started causing crashes. So I figured that might be better to adjust toward the end if we still want to.

By Anchoring I just meant making it (inverse) relative to largeTitleView.frame.maxY.

Can you share the snippit you used for your quick test?

This is a quite rough example (edited a bit more concise):

func scrollViewDidScroll(_ scrollView: UIScrollView) {
       // ....

        let alpha = (-largeTitleView.frame.maxY / 10.0).clamp(min: 0, max: 1)
        titleView.isHidden = alpha == 0
        titleView.alpha = alpha
}

Then you removed everything related to titleIsHidden.

There might be other considerations too with the code to handle small screen sizes.

It's hard to see the difference without setting the background color for cells. So we might want to focus on that in the next PR when I add some content to the rows.

I agree with this too! 👍

@etoledom Those static strings have been moved to be programmatic now. I left the original values for the labels in place just to help the system guess an appropriate intrinsic size after loading.

By Anchoring I just meant making it (inverse) relative to largeTitleView.frame.maxY.

Ah ok, I see what you're doing here. When I did it, I originally had

let alpha = (largeTitleView.frame.height-largeTitleView.frame.maxY / largeTitleView.frame.height)

So as the large title was getting close to disappearing, the small title would appear, and then you ended up with a case where both were showing which I didn't like.

I put this in place, and it seems to be working well. I just needed to make some adjustments to when it "snaps" into place and when entering compact views. There is an additional point in the scrolling where you don't see either header in this approach compared to the other however the "snap" fixes that so it wouldn't really be noticeable.

Just as a quick aside where did you come up with the 10? Or is this just a magic number that felt right?

Edit:

After doing some more testing on this approach I'm noticing the view flashes a bit. Since we're happy with the animation the way it is I think I would prefer to explore this approach at a later time. WDYT?

Thanks @etoledom, I appreciate your great review :)

With the changes on the header, is it still necessary to have a custom Navigation Controller?

The style for the navigation bar when we push the Preview screen is a bit different than the existing Light Navigation Controller so we'll still want the customized one for that.