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

[Paywalls V2] Navigation bar component #4775

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

joshdholtz
Copy link
Member

@joshdholtz joshdholtz commented Feb 10, 2025

Motivation

Allow configuration of native navigation bars.

Description

Added an optional NavigationBarComponent to the base paywall config

  • Optional leading stack
  • Optional trailing stack
{
    "base": {
        "stack": { /* stack */ },
        "navigation_bar": {
            "leading": { /* a stack */ },
            "trailing": { /* a stack */ }
        },
        "sticky_footer": { /* sticky footer */ }
    }
}

NavigationViewIfNeeded

Added a custom NavigationViewIfNeeded view

  • Has a somewhat hacky SwiftUI detector to check if the PaywallView is in already in a navigation stack
  • If so, it does nothing
  • If not, it wraps in a NavigationStack (or NavigationView)

‼️ INPUT NEEDED: We probably don't want to always rely on this hack. I would propose that we add a new parameter that is like presentingFrom and its values are automatic, navigation, and modal (or something like that). we can default to automatic but allow the developer to explicitly say if needed.

Demo

Screen.Recording.2025-02-09.at.8.15.51.PM.mov

Comment on lines -65 to -66
case .stickyFooter(let viewModel):
StickyFooterComponentView(viewModel: viewModel)
Copy link
Member Author

Choose a reason for hiding this comment

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

Sticky footer is not a standalone component (not in the component tree) so removed it from here. I did this in this PR since the navigation bar fell in the same category 🤷‍♂️

Copy link

emerge-tools bot commented Feb 10, 2025

1 build increased size

Name Version Download Change Install Change Approval
Paywalls
com.revenuecat.PaywallsTester
1.0 (1) 10.9 MB ⬆️ 52.6 kB (0.49%) 41.1 MB ⬆️ 211.4 kB (0.52%) N/A

Paywalls 1.0 (1)
com.revenuecat.PaywallsTester

⚖️ Compare build
📦 Install build
⏱️ Analyze build performance

Total install size change: ⬆️ 211.4 kB (0.52%)
Total download size change: ⬆️ 52.6 kB (0.49%)

Largest size changes

Item Install Size Change
DYLD.String Table ⬆️ 81.6 kB
Code Signature ⬆️ 5.6 kB
DYLD.Exports ⬆️ 2.6 kB
📝 RevenueCat.PaywallComponent.NavigationBarComponent.NavigationBarC... ⬆️ 1.7 kB
🗑 RCPurchases.applicationDidBecomeActive ⬇️ -900 B
View Treemap

Image of diff


🛸 Powered by Emerge Tools

Comment trigger: Size diff threshold of 100.00kB exceeded

@@ -28,7 +28,6 @@ enum PaywallComponentViewModel {
case button(ButtonComponentViewModel)
case package(PackageComponentViewModel)
case purchaseButton(PurchaseButtonComponentViewModel)
case stickyFooter(StickyFooterComponentViewModel)
Copy link
Member Author

Choose a reason for hiding this comment

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

Same as other spot... removed here because sticky footer is never going to be in the normal view stack tree thingy


public extension PaywallComponent {

final class NavigationBarComponent: PaywallComponentBase {
Copy link
Member

Choose a reason for hiding this comment

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

Could we name this StickyHeaderComponent? Or maybe HeaderComponent if it's not sticky? This would be more consistent with our StickyFooterComponent, and also avoid confusion on the Android side.

On Android, this would be a "Top App Bar". The "Navigation Bar" on Android is the bar at the bottom, controlled by the OS, containing the gesture handle thing or back/recents/home buttons.

Comment on lines +22 to +23
public let leadingStack: PaywallComponent.StackComponent?
public let trailingStack: PaywallComponent.StackComponent?
Copy link
Member

Choose a reason for hiding this comment

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

This feels a bit like iOS implementation details are leaking. From a platform agnostic point of view, I would expect that this component is just a single (horizontal) stack.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants