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

[Customer Center] Improving customer center buttons #4165

Merged
merged 9 commits into from
Aug 14, 2024

Conversation

codykerns
Copy link
Member

@codykerns codykerns commented Aug 9, 2024

Various improvements around buttons for Customer Center. Several changes for the case where the customer center is presented full screen but wasn't dismiss-able.

NoSubscriptionsView

  • Uses 'dismiss' localized string instead of 'cancel' for dismiss button

Button styles

  • Renames existing ManageSubscriptionButtonStyle to ProminentButtonStyle as it's used elsewhere
  • Adds a new reusable SubtleButtonStyle button style (standard, borderless style)
  • Slightly thickens button fonts to more closely match buttons in system modals
  • Slightly increases corner radius to more closely match buttons in system modals
New Old

WrongPlatformView

  • Adds an explicit 'dismiss' button
New Old

ManageSubscriptionsView

  • Fixes SDK-3537. Adds a toolbar 'dismiss' button to make sure it's dismiss-able when presented in full screen (or in scenarios where a swipe to dismiss isn't supported)
New Old

@codykerns codykerns requested a review from a team August 9, 2024 18:28
@codykerns codykerns marked this pull request as ready for review August 9, 2024 18:29
@aboedo aboedo requested a review from jamesrb1 August 9, 2024 18:36
@jamesrb1
Copy link
Contributor

Looks good, will give this a thoughtful review Monday morning.

Copy link
Contributor

@jamesrb1 jamesrb1 left a comment

Choose a reason for hiding this comment

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

I think these are good changes, nice attention to detail. Pre-approving, but please see comments.

Side comment: I think this title area needs some UI design changes - it looks a bit cramped (localization may be hard), and having the dismiss button the same size and on the same as the screen's title makes the visual hierarchy not great.

image

Copy link
Contributor

@jamesrb1 jamesrb1 left a comment

Choose a reason for hiding this comment

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

Looks good!

Comment on lines +89 to 99
var body: some View {
if #available(iOS 16.0, *) {
NavigationStack {
content
}
} else {
NavigationView {
content
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This view will always be presented modally in some manner? If so this is fine, but if someone were to add this to an existing navigation stack, weird things could happen.

Additionally, I see we've used this pattern twice, if we use it a third time, should consider creating a component we can reuse for this.

Copy link
Member Author

Choose a reason for hiding this comment

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

The modal question is one we'll need to answer, good call 👍

Agreed on the reusable component - will pull this out into a new component in a separate PR.

@codykerns codykerns merged commit 76cbce1 into main Aug 14, 2024
5 checks passed
@codykerns codykerns deleted the cody/customer_center_button_polish branch August 14, 2024 22:02
joshdholtz added a commit that referenced this pull request Aug 22, 2024
**This is an automatic release.**

### New Features
* Price rounding logic (#4132) via James Borthwick (@jamesrb1)
### Bugfixes
* [Customer Center] Migrate to List style (#4190) via Cody Kerns
(@codykerns)
* [Paywalls] Improve locale consistency (#4158) via Josh Holtz
(@joshdholtz)
* Set Paywalls Tester deployment target to iOS 15 (#4196) via James
Borthwick (@jamesrb1)
* [Customer Center] Hide Contact Support button if URL can't be created
(#4192) via Cesar de la Vega (@vegaro)
* Fix the setting for SKIP_INSTALL in Xcode project (#4195) via Andy
Boedo (@aboedo)
* [Customer Center] Improving customer center buttons (#4165) via Cody
Kerns (@codykerns)
* Revert workaround for iOS 18 beta 5 SwiftUI crash (#4173) via Mark
Villacampa (@MarkVillacampa)
* [Paywalls] Make iOS version calculation lazy (#4163) via Mark
Villacampa (@MarkVillacampa)
* [Paywalls] Observe `PurchaseHandler` when owned externally (#4097) via
James Borthwick (@jamesrb1)
### Other Changes
* [Customer Center] Clean up colors in WrongPlatformView and
NoSubscriptionsView (#4204) via Cesar de la Vega (@vegaro)
* Fix failing `all-tests` and retry more flaky tests (#4188) via Josh
Holtz (@joshdholtz)
* Compatibility content unavailable improvements (#4197) via James
Borthwick (@jamesrb1)
* Create lane to enable customer center (#4191) via Cesar de la Vega
(@vegaro)
* XCFramework artifacts in CircleCI (#4189) via Andy Boedo (@aboedo)
* [Customer Center] CustomerCenterViewModel checks whether the app is
the latest version (#4169) via JayShortway (@JayShortway)
* export RevenueCatUI xcframework (#4172) via Andy Boedo (@aboedo)
* Corrects references from ManageSubscriptionsButtonStyle to
ButtonsStyle. (#4186) via JayShortway (@JayShortway)
* Speed up carthage installation tests (#4184) via Andy Boedo (@aboedo)
* Customer center improvements (#4166) via James Borthwick (@jamesrb1)
* replace `color(from colorInformation:)` global with extension (#4183)
via Andy Boedo (@aboedo)
* Fix tests in main (#4174) via Andy Boedo (@aboedo)
* Enable customer center tests (#4171) via James Borthwick (@jamesrb1)
* [Customer Center] Initial implementation (#3967) via Cesar de la Vega
(@vegaro)

---------

Co-authored-by: Josh Holtz <me@joshholtz.com>
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.

3 participants