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

♿️ Disable Dynamic Type on buttons #806

Merged
merged 14 commits into from
Aug 20, 2019
11 changes: 1 addition & 10 deletions Kickstarter-iOS/Views/Controllers/LoginToutViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ internal final class LoginToutViewController: UIViewController, MFMailComposeVie
|> fbLoginButtonStyle

_ = self.disclaimerButton
|> baseMultiLineButtonStyle
|> multiLineButtonStyle
|> disclaimerButtonStyle

_ = self.loginButton
Expand Down Expand Up @@ -421,12 +421,3 @@ private let baseLabelStyle: LabelStyle = { label in
|> \.lineBreakMode .~ NSLineBreakMode.byWordWrapping
|> \.numberOfLines .~ 0
}

private let baseMultiLineButtonStyle: ButtonStyle = { button in
_ = button.titleLabel
?|> \.textAlignment .~ NSTextAlignment.center
?|> \.lineBreakMode .~ NSLineBreakMode.byWordWrapping
?|> \.numberOfLines .~ 0

return button
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import Prelude
final class PledgeContinueViewController: UIViewController {
// MARK: - Properties

private let continueButton = MultiLineButton(type: .custom)
private let continueButton = UIButton(type: .custom)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We no longer need to use MultiLineButton here...in fact since we've previously added trimming we haven't been using it for a while (so this is just a cleanup).

Copy link
Contributor

Choose a reason for hiding this comment

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

I see we're only using this in one place, not time to remove MultiLineButton yet?

private let viewModel = PledgeContinueViewModel()

// MARK: - Lifecycle
Expand All @@ -25,10 +25,7 @@ final class PledgeContinueViewController: UIViewController {
|> checkoutBackgroundStyle

_ = self.continueButton
|> greenButtonStyle
|> UIButton.lens.title(for: .normal) %~ { _ in
Strings.Continue()
}
|> continueButtonStyle
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moving to // MARK: - Styles for consistency

Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking that we can start renaming the Styles pragma mark above bindStyles() to Styles Binding or Bind Styles, so it differs from the Styles on the bottom of the file. what do you think?

}

// MARK: - View model
Expand Down Expand Up @@ -74,3 +71,11 @@ final class PledgeContinueViewController: UIViewController {
self.presentViewControllerWithSheetOverlay(navigationController, offset: navigationBarHeight)
}
}

// MARK: - Styles

private let continueButtonStyle: ButtonStyle = { button in
button
|> greenButtonStyle
|> UIButton.lens.title(for: .normal) %~ { _ in Strings.Continue() }
}
28 changes: 3 additions & 25 deletions Kickstarter-iOS/Views/PledgeCTAContainerView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,6 @@ final class PledgeCTAContainerView: UIView {
|> \.isLayoutMarginsRelativeArrangement .~ true
|> \.spacing .~ Styles.gridHalf(1)

_ = self.pledgeCTAButton
|> pledgeCTAButtonStyle(
isAccessibilityCategory,
amountAndRewardTitleStackViewIsHidden: self.titleAndSubtitleStackView.isHidden
)

_ = self.rootStackView
|> adaptableStackViewStyle(isAccessibilityCategory)
|> \.isLayoutMarginsRelativeArrangement .~ true
Expand Down Expand Up @@ -107,7 +101,8 @@ final class PledgeCTAContainerView: UIView {
self.viewModel.outputs.buttonStyleType
.observeForUI()
.observeValues { [weak self] buttonStyleType in
_ = self?.pledgeCTAButton ?|> buttonStyleType.style
_ = self?.pledgeCTAButton
?|> buttonStyleType.style
}

self.viewModel.outputs.rootStackViewAnimateIsHidden
Expand Down Expand Up @@ -169,27 +164,10 @@ final class PledgeCTAContainerView: UIView {

private func adaptableStackViewStyle(_ isAccessibilityCategory: Bool) -> (StackViewStyle) {
return { (stackView: UIStackView) in
let axis: NSLayoutConstraint.Axis = (isAccessibilityCategory ? .vertical : .horizontal)
let spacing: CGFloat = (isAccessibilityCategory ? Styles.grid(1) : 0)

return stackView
|> \.axis .~ axis
|> \.axis .~ NSLayoutConstraint.Axis.horizontal
Copy link
Contributor

Choose a reason for hiding this comment

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

Was the compiler giving you trouble here? 😄 I think that |> \.axis .~ .horizontal is more readable and more used on the codebase. Also, we have more styling going on inside the bindStyles() function. To keep consistency and avoid doing the same thing in multiple places, I would either move this code to the bindStyles function (not recommended) or do a small refactor bringing the other styles to the bottom. (recommended, since that's how we are approaching styling).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not that the compiler would give us trouble but we've been seeing a lot of warnings lately related to Xcode's type checker...therefore at some point we've decided that for styles it would make more sense to be more explicit.

I'll make the styling more consistent by extracting bindStyles here.

|> \.spacing .~ spacing
}
}

private func pledgeCTAButtonStyle(
_ isAccessibilityCategory: Bool, amountAndRewardTitleStackViewIsHidden: Bool
) -> (ButtonStyle) {
return { (button: UIButton) in
let lineBreakMode: NSLineBreakMode = isAccessibilityCategory || amountAndRewardTitleStackViewIsHidden
? NSLineBreakMode.byWordWrapping : NSLineBreakMode.byTruncatingTail

return button
|> roundedStyle(cornerRadius: 12)
|> UIButton.lens.titleLabel.font .~ UIFont.ksr_headline(size: 15)
|> UIButton.lens.layer.borderWidth .~ 0
|> (UIButton.lens.titleLabel .. UILabel.lens.textAlignment) .~ NSTextAlignment.center
|> (UIButton.lens.titleLabel .. UILabel.lens.lineBreakMode) .~ lineBreakMode
}
}
11 changes: 4 additions & 7 deletions Kickstarter-iOS/Views/RewardCardContainerView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ public final class RewardCardContainerView: UIView {
|> \.translatesAutoresizingMaskIntoConstraints .~ false
}()

private let pledgeButton: MultiLineButton = {
MultiLineButton(type: .custom)
private let pledgeButton: UIButton = {
UIButton(type: .custom)
|> \.translatesAutoresizingMaskIntoConstraints .~ false
}()

Expand Down Expand Up @@ -65,9 +65,6 @@ public final class RewardCardContainerView: UIView {
(UIColor.white.withAlphaComponent(1.0), 1)
]
self.gradientView.setGradient(gradient)

_ = self.pledgeButton.titleLabel
?|> \.lineBreakMode .~ .byTruncatingTail
}

public override func bindViewModel() {
Expand Down Expand Up @@ -104,8 +101,8 @@ public final class RewardCardContainerView: UIView {
self.viewModel.outputs.pledgeButtonStyleType
.observeForUI()
.observeValues { [weak self] styleType in
guard let self = self else { return }
_ = self.pledgeButton |> styleType.style
_ = self?.pledgeButton
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using optional chaining for consistency between files

?|> styleType.style
}
}

Expand Down
15 changes: 12 additions & 3 deletions Library/Styles/ButtonStyles.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ public let applePayButtonStyle: ButtonStyle = { button in
public let baseButtonStyle: ButtonStyle = { button in
button
|> roundedStyle(cornerRadius: Styles.grid(2))
|> UIButton.lens.titleLabel.font .~ UIFont.ksr_callout().bolded
|> (UIButton.lens.titleLabel .. UILabel.lens.textAlignment) .~ NSTextAlignment.center
|> UIButton.lens.contentEdgeInsets .~ .init(all: Styles.grid(2))
|> UIButton.lens.titleLabel .. UILabel.lens.font .~ UIFont.boldSystemFont(ofSize: 16)
Copy link
Contributor

Choose a reason for hiding this comment

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

Lenses give us access to the font property of a UIButton. So this line could simply be |> UIButton.lens.titleLabel.font .~ UIFont.boldSystemFont(ofSize: 16) (this syntax is the most commonly used on our codebase)

Copy link
Contributor Author

@dusi dusi Aug 20, 2019

Choose a reason for hiding this comment

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

Had to update Prelude so if you can also check it out and approve if everything looks fine - kickstarter/Kickstarter-Prelude#95

|> UIButton.lens.titleLabel .. UILabel.lens.lineBreakMode .~ NSLineBreakMode.byTruncatingMiddle
|> UIButton.lens.titleLabel .. UILabel.lens.textAlignment .~ NSTextAlignment.center
|> UIButton.lens.contentEdgeInsets .~ .init(all: Styles.grid(2))
|> UIButton.lens.titleLabel .. UILabel.lens.numberOfLines .~ 1
|> UIButton.lens.adjustsImageWhenDisabled .~ false
|> UIButton.lens.adjustsImageWhenHighlighted .~ false
Expand Down Expand Up @@ -89,6 +89,15 @@ public let facebookButtonStyle = baseButtonStyle

<> UIButton.lens.image(for: .normal) %~ { _ in image(named: "fb-logo-white") }

// MARK: - Multiline

public let multiLineButtonStyle: ButtonStyle = { button in
button
|> UIButton.lens.titleLabel .. UILabel.lens.lineBreakMode .~ NSLineBreakMode.byWordWrapping
|> UIButton.lens.titleLabel .. UILabel.lens.numberOfLines .~ 0
|> UIButton.lens.titleLabel .. UILabel.lens.textAlignment .~ NSTextAlignment.center
}

// MARK: - Red

public let redButtonStyle = baseButtonStyle
Expand Down
1 change: 0 additions & 1 deletion Library/Styles/LoginStyles.swift
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ public func emailFieldAutoFillStyle(_ textField: UITextField) -> UITextField {
}

public let fbLoginButtonStyle = facebookButtonStyle
<> baseButtonStyle
<> UIButton.lens.title(for: .normal) %~ { _ in
Strings.login_tout_buttons_log_in_with_facebook()
}
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.