From 0e28dbc0b0d24b806f194f166699c59a01279965 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 9 Nov 2023 09:44:25 -0500 Subject: [PATCH] Refactor FXIOS-7576 [v121] Replace link buttons in FakespotOptInCardView (backport #17132) (#17238) * Refactor FXIOS-7576 [v121] Replace link buttons in FakespotOptInCardView (#17132) * issue-16863 | link button * issue-16863 | removed textalignment * issue-16863 | fix content edge insets warning * issue-16863 | change default insets value * issue-16863 | increased threshold warning count for iOS version bump from 14 to 15 (cherry picked from commit 9aaa4be1ff6b61ff3989d782eae080d881b55663) # Conflicts: # Client/Frontend/Onboarding/ViewControllers/OnboardingCardViewController.swift * Update OnboardingCardViewController.swift * Update generate-metrics.sh --------- Co-authored-by: imsanchit Co-authored-by: Winnie Teichmann <4530+thatswinnie@users.noreply.github.com> --- BrowserKit/Package.swift | 2 +- .../ComponentLibrary/Buttons/LinkButton.swift | 19 ++--- .../Buttons/LinkButtonViewModel.swift | 17 ++++- .../Views/FakespotOptInCardView.swift | 76 ++++++++++--------- .../OnboardingCardViewController.swift | 2 +- test-fixtures/generate-metrics.sh | 4 +- 6 files changed, 67 insertions(+), 53 deletions(-) diff --git a/BrowserKit/Package.swift b/BrowserKit/Package.swift index 2221fadf5af4..eb685a0a6533 100644 --- a/BrowserKit/Package.swift +++ b/BrowserKit/Package.swift @@ -5,7 +5,7 @@ import PackageDescription let package = Package( name: "BrowserKit", platforms: [ - .iOS(.v14) + .iOS(.v15) ], products: [ .library( diff --git a/BrowserKit/Sources/ComponentLibrary/Buttons/LinkButton.swift b/BrowserKit/Sources/ComponentLibrary/Buttons/LinkButton.swift index 8841ec58147e..bec6503b80cf 100644 --- a/BrowserKit/Sources/ComponentLibrary/Buttons/LinkButton.swift +++ b/BrowserKit/Sources/ComponentLibrary/Buttons/LinkButton.swift @@ -8,27 +8,28 @@ import UIKit public class LinkButton: ResizableButton, ThemeApplicable { private struct UX { static let buttonCornerRadius: CGFloat = 13 - static let buttonVerticalInset: CGFloat = 12 - static let buttonHorizontalInset: CGFloat = 16 } override init(frame: CGRect) { super.init(frame: frame) + configuration = UIButton.Configuration.plain() backgroundColor = .clear titleLabel?.adjustsFontForContentSizeCategory = true - contentEdgeInsets = UIEdgeInsets(top: UX.buttonVerticalInset, - left: UX.buttonHorizontalInset, - bottom: UX.buttonVerticalInset, - right: UX.buttonHorizontalInset) } public func configure(viewModel: LinkButtonViewModel) { accessibilityIdentifier = viewModel.a11yIdentifier setTitle(viewModel.title, for: .normal) - titleLabel?.font = DefaultDynamicFontHelper.preferredFont(withTextStyle: .body, - size: viewModel.fontSize) - titleLabel?.textAlignment = viewModel.textAlignment + configuration?.titleTextAttributesTransformer = UIConfigurationTextAttributesTransformer { incoming in + var outgoing = incoming + outgoing.font = DefaultDynamicFontHelper.preferredFont(withTextStyle: .body, + size: viewModel.fontSize) + return outgoing + } + configuration?.contentInsets = viewModel.contentInsets + contentHorizontalAlignment = viewModel.contentHorizontalAlignment + layoutIfNeeded() } required init?(coder aDecoder: NSCoder) { diff --git a/BrowserKit/Sources/ComponentLibrary/Buttons/LinkButtonViewModel.swift b/BrowserKit/Sources/ComponentLibrary/Buttons/LinkButtonViewModel.swift index 21cf25e5020a..783bd9663e48 100644 --- a/BrowserKit/Sources/ComponentLibrary/Buttons/LinkButtonViewModel.swift +++ b/BrowserKit/Sources/ComponentLibrary/Buttons/LinkButtonViewModel.swift @@ -7,18 +7,29 @@ import Foundation /// The view model used to configure a `LinkButton` public struct LinkButtonViewModel { + public struct UX { + public static let verticalInset: CGFloat = 12 + public static let horizontalInset: CGFloat = 16 + } + public let title: String public let a11yIdentifier: String public let fontSize: CGFloat - public let textAlignment: NSTextAlignment + public let contentInsets: NSDirectionalEdgeInsets + public let contentHorizontalAlignment: UIControl.ContentHorizontalAlignment public init(title: String, a11yIdentifier: String, fontSize: CGFloat = 16, - textAlignment: NSTextAlignment = .left) { + contentInsets: NSDirectionalEdgeInsets = NSDirectionalEdgeInsets(top: UX.verticalInset, + leading: UX.horizontalInset, + bottom: UX.verticalInset, + trailing: UX.horizontalInset), + contentHorizontalAlignment: UIControl.ContentHorizontalAlignment = .leading) { self.title = title self.a11yIdentifier = a11yIdentifier self.fontSize = fontSize - self.textAlignment = textAlignment + self.contentInsets = contentInsets + self.contentHorizontalAlignment = contentHorizontalAlignment } } diff --git a/Client/Frontend/Fakespot/Views/FakespotOptInCardView.swift b/Client/Frontend/Fakespot/Views/FakespotOptInCardView.swift index d32b05f294a6..0cb8317a27e9 100644 --- a/Client/Frontend/Fakespot/Views/FakespotOptInCardView.swift +++ b/Client/Frontend/Fakespot/Views/FakespotOptInCardView.swift @@ -15,8 +15,8 @@ final class FakespotOptInCardView: UIView, ThemeApplicable { static let learnMoreButtonFontSize: CGFloat = 15 static let termsOfUseButtonInsets = UIEdgeInsets(top: 0, left: 16, bottom: 0, right: 16) static let disclaimerTextLabelFontSize: CGFloat = 13 - static let disclaimerBlockInsets = UIEdgeInsets(top: 0, left: 16, bottom: 0, right: 16) - static let learnMoreInsets = UIEdgeInsets(top: -10, left: 0, bottom: 10, right: 0) + static let disclaimerBlockInsets = NSDirectionalEdgeInsets(top: 0, leading: 16, bottom: 0, trailing: 16) + static let learnMoreInsets = NSDirectionalEdgeInsets(top: -10, leading: 0, bottom: 10, trailing: 0) static let termsOfUseButtonTitleFontSize: CGFloat = 13 static let privacyPolicyButtonTitleFontSize: CGFloat = 13 static let secondaryButtonFontSize: CGFloat = 13 @@ -72,43 +72,24 @@ final class FakespotOptInCardView: UIView, ThemeApplicable { } // MARK: Buttons - private lazy var learnMoreButton: ResizableButton = .build { button in - button.contentHorizontalAlignment = .leading - button.buttonEdgeSpacing = 0 + private lazy var learnMoreButton: LinkButton = .build { button in button.addTarget(self, action: #selector(self.didTapLearnMore), for: .touchUpInside) - button.titleLabel?.font = DefaultDynamicFontHelper.preferredFont(withTextStyle: .body, - size: UX.learnMoreButtonFontSize) - button.contentEdgeInsets = UX.learnMoreInsets } - private lazy var termsOfUseButton: ResizableButton = .build { button in - button.contentHorizontalAlignment = .leading - button.buttonEdgeSpacing = 0 - button.contentEdgeInsets = UX.disclaimerBlockInsets + private lazy var termsOfUseButton: LinkButton = .build { button in button.addTarget(self, action: #selector(self.didTapTermsOfUse), for: .touchUpInside) - button.titleLabel?.font = DefaultDynamicFontHelper.preferredFont(withTextStyle: .body, - size: UX.termsOfUseButtonTitleFontSize) } - private lazy var privacyPolicyButton: ResizableButton = .build { button in - button.contentHorizontalAlignment = .leading - button.buttonEdgeSpacing = 0 - button.contentEdgeInsets = UX.disclaimerBlockInsets + private lazy var privacyPolicyButton: LinkButton = .build { button in button.addTarget(self, action: #selector(self.didTapPrivacyPolicy), for: .touchUpInside) - button.titleLabel?.font = DefaultDynamicFontHelper.preferredFont(withTextStyle: .body, - size: UX.privacyPolicyButtonTitleFontSize) } private lazy var mainButton: PrimaryRoundedButton = .build { button in button.addTarget(self, action: #selector(self.didTapMainButton), for: .touchUpInside) } - private lazy var secondaryButton: ResizableButton = .build { button in - button.contentHorizontalAlignment = .center - button.buttonEdgeSpacing = 0 + private lazy var secondaryButton: LinkButton = .build { button in button.addTarget(self, action: #selector(self.didTapSecondaryButton), for: .touchUpInside) - button.titleLabel?.font = DefaultDynamicFontHelper.preferredFont(withTextStyle: .body, - size: UX.secondaryButtonFontSize) } override init(frame: CGRect) { @@ -204,14 +185,29 @@ final class FakespotOptInCardView: UIView, ThemeApplicable { disclaimerTextLabel.attributedText = viewModel.disclaimerText disclaimerTextLabel.accessibilityIdentifier = viewModel.disclaimerLabelA11yId - learnMoreButton.setTitle(viewModel.learnMoreButtonText, for: .normal) - learnMoreButton.accessibilityIdentifier = viewModel.learnMoreButtonA11yId + let learnMoreButtonViewModel = LinkButtonViewModel( + title: viewModel.learnMoreButtonText, + a11yIdentifier: viewModel.learnMoreButtonA11yId, + fontSize: UX.learnMoreButtonFontSize, + contentInsets: UX.learnMoreInsets + ) + learnMoreButton.configure(viewModel: learnMoreButtonViewModel) - termsOfUseButton.setTitle(viewModel.termsOfUseButtonText, for: .normal) - termsOfUseButton.accessibilityIdentifier = viewModel.termsOfUseButtonA11yId + let termsOfUseButtonViewModel = LinkButtonViewModel( + title: viewModel.termsOfUseButtonText, + a11yIdentifier: viewModel.termsOfUseButtonA11yId, + fontSize: UX.termsOfUseButtonTitleFontSize, + contentInsets: UX.disclaimerBlockInsets + ) + termsOfUseButton.configure(viewModel: termsOfUseButtonViewModel) - privacyPolicyButton.setTitle(viewModel.privacyPolicyButtonText, for: .normal) - privacyPolicyButton.accessibilityIdentifier = viewModel.privacyPolicyButtonA11yId + let privacyButtonViewModel = LinkButtonViewModel( + title: viewModel.privacyPolicyButtonText, + a11yIdentifier: viewModel.privacyPolicyButtonA11yId, + fontSize: UX.privacyPolicyButtonTitleFontSize, + contentInsets: UX.disclaimerBlockInsets + ) + privacyPolicyButton.configure(viewModel: privacyButtonViewModel) let buttonViewModel = PrimaryRoundedButtonViewModel( title: viewModel.mainButtonText, @@ -219,8 +215,14 @@ final class FakespotOptInCardView: UIView, ThemeApplicable { ) mainButton.configure(viewModel: buttonViewModel) - secondaryButton.setTitle(viewModel.secondaryButtonText, for: .normal) - secondaryButton.accessibilityIdentifier = viewModel.secondaryButtonA11yId + let secondaryButtonViewModel = LinkButtonViewModel( + title: viewModel.secondaryButtonText, + a11yIdentifier: viewModel.secondaryButtonA11yId, + fontSize: UX.secondaryButtonFontSize, + contentInsets: .zero, + contentHorizontalAlignment: .center + ) + secondaryButton.configure(viewModel: secondaryButtonViewModel) let cardModel = ShadowCardViewModel(view: mainView, a11yId: viewModel.cardA11yId) cardContainer.configure(cardModel) @@ -240,10 +242,10 @@ final class FakespotOptInCardView: UIView, ThemeApplicable { headerLabel.textColor = colors.textPrimary bodyLabel.textColor = colors.textPrimary disclaimerTextLabel.textColor = colors.textSecondary - learnMoreButton.setTitleColor(colors.textAccent, for: .normal) - termsOfUseButton.setTitleColor(colors.textAccent, for: .normal) - privacyPolicyButton.setTitleColor(colors.textAccent, for: .normal) + learnMoreButton.applyTheme(theme: theme) + termsOfUseButton.applyTheme(theme: theme) + privacyPolicyButton.applyTheme(theme: theme) mainButton.applyTheme(theme: theme) - secondaryButton.setTitleColor(colors.textAccent, for: .normal) + secondaryButton.applyTheme(theme: theme) } } diff --git a/Client/Frontend/Onboarding/ViewControllers/OnboardingCardViewController.swift b/Client/Frontend/Onboarding/ViewControllers/OnboardingCardViewController.swift index 9f01ea2bd027..d6879f4dc47c 100644 --- a/Client/Frontend/Onboarding/ViewControllers/OnboardingCardViewController.swift +++ b/Client/Frontend/Onboarding/ViewControllers/OnboardingCardViewController.swift @@ -338,7 +338,7 @@ class OnboardingCardViewController: UIViewController, Themeable { title: buttonTitle, a11yIdentifier: "\(self.viewModel.a11yIdRoot)LinkButton", fontSize: UX.buttonFontSize, - textAlignment: .center + contentHorizontalAlignment: .center ) linkButton.configure(viewModel: buttonViewModel) linkButton.applyTheme(theme: themeManager.currentTheme) diff --git a/test-fixtures/generate-metrics.sh b/test-fixtures/generate-metrics.sh index ba4b4b3ce3b2..2a6c3a32a0c6 100755 --- a/test-fixtures/generate-metrics.sh +++ b/test-fixtures/generate-metrics.sh @@ -4,8 +4,8 @@ set -e BUILD_LOG_FILE="$1" TYPE_LOG_FILE="$2" -THRESHOLD_UNIT_TEST=92 -THRESHOLD_XCUITEST=92 +THRESHOLD_UNIT_TEST=108 +THRESHOLD_XCUITEST=108 WARNING_COUNT=$(grep -E -v "SourcePackages/checkouts" "$BUILD_LOG_FILE" | grep -E "(^|:)[0-9]+:[0-9]+:|warning:|ld: warning:|:0: warning:|fatal|===" | uniq | wc -l)