Skip to content

[Just In Time Messages] Animate hiding of message in My Store screen #8293

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Dec 5, 2022
Merged

[Just In Time Messages] Animate hiding of message in My Store screen #8293

Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f0b3e1f
Initial approach to animating the JITM message
koke Dec 1, 2022
196d3af
Simplify header hiding logic
koke Dec 1, 2022
e03d673
Switch to UIViewPropertyAnimator for animating the JITM
koke Dec 1, 2022
ef31cf2
Don't reuse animator and cancel any existing animations instead
koke Dec 2, 2022
c22901d
Document headerAnimator
koke Dec 2, 2022
3b18330
Pause navbar observer when view is not visible
koke Dec 2, 2022
15795b1
Consolidate header animation code and allow updates without animation
koke Dec 2, 2022
1f169d7
Avoid retain cycles in animations
koke Dec 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 75 additions & 29 deletions WooCommerce/Classes/ViewRelated/Dashboard/DashboardViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,14 @@ final class DashboardViewController: UIViewController {
return view
}()

/// Constraint to attach the content view's top to the bottom of the header
/// When we hide the header, we disable this constraint so the content view can grow to fill the screen
private var contentTopToHeaderConstraint: NSLayoutConstraint?

/// Stores an animator for showing/hiding the header view while there is an animation in progress
/// so we can interrupt and reverse if needed
private var headerAnimator: UIViewPropertyAnimator?

// Used to trick the navigation bar for large title (ref: issue 3 in p91TBi-45c-p2).
private let hiddenScrollView = UIScrollView()

Expand Down Expand Up @@ -119,7 +127,6 @@ final class DashboardViewController: UIViewController {
configureBottomJetpackBenefitsBanner()
observeSiteForUIUpdates()
observeBottomJetpackBenefitsBannerVisibilityUpdates()
observeNavigationBarHeightForHeaderExtrasVisibility()
observeStatsVersionForDashboardUIUpdates()
observeAnnouncements()
observeShowWebViewSheet()
Expand All @@ -136,6 +143,11 @@ final class DashboardViewController: UIViewController {
configureTitle()
}

override func viewDidAppear(_ animated: Bool) {
super.viewDidAppear(animated)
observeNavigationBarHeightForHeaderVisibility()
}

override func viewDidLayoutSubviews() {
super.viewDidLayoutSubviews()
dashboardUI?.view.frame = containerView.bounds
Expand All @@ -145,16 +157,42 @@ final class DashboardViewController: UIViewController {
return true
}

/// Hide the announcement card when the navigation bar is compact
///
func updateAnnouncementCardVisibility() {
announcementView?.isHidden = navigationBarIsShort
func showHeader() {
Copy link
Contributor

@toupper toupper Dec 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a blocker but perhaps we could collapse these two methods that look very similar into one with a parameter:

func changeHeaderVisibility(isHidden: Bool) {
        contentTopToHeaderConstraint?.isActive = !isHidden
        headerStackView.alpha = isHidden ? 1 : 0
        view.layoutIfNeeded()
}

Copy link
Member Author

@koke koke Dec 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On one hand, I like the idea of reducing duplication, but I think I lean towards the current implementation since it seems clearer what showing or hiding would do, without having to parse inline conditionals.

contentTopToHeaderConstraint?.isActive = true
headerStackView.alpha = 1
view.layoutIfNeeded()
}

/// Hide the store name when the navigation bar is compact
///
func updateStoreNameLabelVisibility() {
storeNameLabel.isHidden = !shouldShowStoreNameAsSubtitle || navigationBarIsShort
func hideHeader() {
contentTopToHeaderConstraint?.isActive = false
headerStackView.alpha = 0
view.layoutIfNeeded()
}

func showHeaderWithAnimation() {
headerAnimator?.stopAnimation(true)
headerAnimator = UIViewPropertyAnimator.runningPropertyAnimator(
withDuration: Constants.animationDurationSeconds,
delay: 0,
animations: { [weak self] in
self?.showHeader()
},
completion: { [weak self] position in
self?.headerAnimator = nil
})
}

func hideHeaderWithAnimation() {
headerAnimator?.stopAnimation(true)
headerAnimator = UIViewPropertyAnimator.runningPropertyAnimator(
withDuration: Constants.animationDurationSeconds,
delay: 0,
animations: { [weak self] in
self?.hideHeader()
},
completion: { [weak self] position in
self?.headerAnimator = nil
})
}
}

Expand Down Expand Up @@ -208,8 +246,19 @@ private extension DashboardViewController {

func addViewBelowHeaderStackView(contentView: UIView) {
contentView.translatesAutoresizingMaskIntoConstraints = false

// This constraint will pin the bottom of the header to the top of the content
// We want this to be active when the header is visible
contentTopToHeaderConstraint = contentView.topAnchor.constraint(equalTo: headerStackView.bottomAnchor)
contentTopToHeaderConstraint?.isActive = true

// This constraint has a lower priority and will pin the top of the content view to its superview
// This way, it has a defined height when contentTopToHeaderConstraint is disabled
let contentTopToContainerConstraint = contentView.topAnchor.constraint(equalTo: containerView.safeTopAnchor)
contentTopToContainerConstraint.priority = .defaultLow

NSLayoutConstraint.activate([
contentView.topAnchor.constraint(equalTo: headerStackView.bottomAnchor),
contentTopToContainerConstraint,
contentView.leadingAnchor.constraint(equalTo: containerView.leadingAnchor),
contentView.trailingAnchor.constraint(equalTo: containerView.trailingAnchor),
])
Expand Down Expand Up @@ -377,8 +426,6 @@ private extension DashboardViewController {
let indexAfterHeader = (headerStackView.arrangedSubviews.firstIndex(of: innerStackView) ?? -1) + 1
headerStackView.insertArrangedSubview(uiView, at: indexAfterHeader)

updateAnnouncementCardVisibility()

hostingController.didMove(toParent: self)
hostingController.view.layoutIfNeeded()
}
Expand All @@ -402,11 +449,12 @@ private extension DashboardViewController {
guard siteName.isNotEmpty else {
shouldShowStoreNameAsSubtitle = false
storeNameLabel.text = nil
storeNameLabel.isHidden = true
return
}
shouldShowStoreNameAsSubtitle = true
storeNameLabel.isHidden = false
storeNameLabel.text = siteName
updateStoreNameLabelVisibility()
}
}

Expand Down Expand Up @@ -585,31 +633,28 @@ private extension DashboardViewController {
}.store(in: &subscriptions)
}

func observeNavigationBarHeightForHeaderExtrasVisibility() {
func observeNavigationBarHeightForHeaderVisibility() {
navigationController?.navigationBar.publisher(for: \.frame, options: [.initial, .new])
.map({ [collapsedNavigationBarHeight] rect in
rect.height <= collapsedNavigationBarHeight
}) // true if navigation bar is collapsed
.removeDuplicates()
.sink(receiveValue: { [weak self] _ in
guard let self else { return }
self.updateStoreNameLabelVisibility()
self.updateAnnouncementCardVisibility()
.sink(receiveValue: { [weak self] navigationBarIsShort in
if navigationBarIsShort {
self?.hideHeaderWithAnimation()
} else {
self?.showHeaderWithAnimation()
}
})
.store(in: &subscriptions)
}

/// Returns true if the navigation bar has a compact height as opposed to showing a large title
///
var navigationBarIsShort: Bool {
guard let navigationBarHeight = navigationController?.navigationBar.frame.height else {
return false
}

let collapsedNavigationBarHeight: CGFloat
var collapsedNavigationBarHeight: CGFloat {
if self.traitCollection.userInterfaceIdiom == .pad {
collapsedNavigationBarHeight = Constants.iPadCollapsedNavigationBarHeight
return Constants.iPadCollapsedNavigationBarHeight
} else {
collapsedNavigationBarHeight = Constants.iPhoneCollapsedNavigationBarHeight
return Constants.iPhoneCollapsedNavigationBarHeight
}
return navigationBarHeight <= collapsedNavigationBarHeight
}
}

Expand All @@ -623,6 +668,7 @@ private extension DashboardViewController {
}

enum Constants {
static let animationDurationSeconds = CGFloat(0.3)
static let bannerBottomMargin = CGFloat(8)
static let horizontalMargin = CGFloat(16)
static let storeNameTextColor: UIColor = .secondaryLabel
Expand Down