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.

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

Jump to bottom

Fixes UISegmentedControl extension crash and improves efficiency #398

Merged
merged 7 commits into from
Jun 30, 2021
Merged

Fixes UISegmentedControl extension crash and improves efficiency #398

Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b297775
Fixed UISegmentedControl extension crash and improved efficiency.
Dec 17, 2020
a3f88b4
Update AdaptableTextContainer.swift
rzulkoski Dec 17, 2020
620f623
Reverted change to public signature of bon_titleTextAttributes(for:).
Dec 17, 2020
ee18bc4
Merge remote-tracking branch 'refs/remotes/origin/bugfix/segmented-co…
Dec 17, 2020
c0fe6bb
Fixed error in UISegmentedControl.bon_titleTextAttributes(for:) and a…
Dec 18, 2020
e8ef86a
Reverting back to returning optional StyleAttributes from UISegmented…
Dec 18, 2020
6753df1
Disabled social_media_url in podspec to fix builds
rzulkoski Jan 24, 2021
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
28 changes: 14 additions & 14 deletions Sources/UIKit/AdaptableTextContainer.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,20 +146,20 @@ extension UISegmentedControl {

// `UISegmentedControl` has terrible generics ([NSObject: AnyObject]? or [AnyHashable: Any]?) on
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this comment need to be updated at all in light of these changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fixed an error inside that method. When I changed the return type back to being non-optional I forgot to default to an empty dictionary when nil. I also added a test for segmented control similar to the other UIKit tests.

As for the comment, I can't speak to whether it is still appropriate or not, although I can say in Swift 5 titleTextAttributes(for:) returns [NSAttributedFont.Key: Any]?.

/// `titleTextAttributes`, so use a helper in Swift 3+
@nonobjc final func bon_titleTextAttributes(for state: UIControl.State) -> StyleAttributes {
let attributes = titleTextAttributes(for: state) ?? [:]
var result: StyleAttributes = [:]
for value in attributes {
#if swift(>=4.2)
result[value.key] = value
#else
guard let string = value.key as? StyleAttributes.Key else {
fatalError("Can not convert key \(value.key) to String")
@nonobjc final func bon_titleTextAttributes(for state: UIControl.State) -> StyleAttributes? {
Copy link
Contributor

Choose a reason for hiding this comment

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

This changes the public API, would it be possible keep this the same?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I have reverted the changes to the signature of that method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just realized that method is actually internal, so if we wanted to keep that change it wouldn't affect the public API. I'm fine with either direction.

Copy link
Contributor Author

@rzulkoski rzulkoski Dec 18, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Also it looks like the tests are failing because Zev's twitter account is unreachable.

 ** BUILD SUCCEEDED **
    
   Testing with `xcodebuild`. 
 -> BonMot (5.6.0)
    - WARN  | url: The URL (https://twitter.com/ZevEisenberg) is not reachable.
    - NOTE  | xcodebuild:  note: Using new build system
    - NOTE  | xcodebuild:  note: Building targets in parallel
    - NOTE  | xcodebuild:  note: Using codesigning identity override: -
    - NOTE  | xcodebuild:  note: Planning build
    - NOTE  | xcodebuild:  note: Constructing build description
    - NOTE  | xcodebuild:  warning: Capabilities for Signing & Capabilities may not function correctly because its entitlements use a placeholder team ID. To resolve this, select a development team in the App editor. (in target 'App' from project 'App')
    - NOTE  | xcodebuild:  warning: Skipping code signing because the target does not have an Info.plist file and one is not being generated automatically. (in target 'App' from project 'App')
    - NOTE  | xcodebuild:  note: Using codesigning identity override: 

[!] BonMot did not pass validation, due to 1 warning (but you can use `--allow-warnings` to ignore it).
You can use the `--no-clean` option to inspect any issue.

Exited with code exit status 1
CircleCI received exit code 1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Might be related to #373 @ZevEisenberg

guard let attributes = titleTextAttributes(for: state) else { return nil }
#if swift(>=4.2)
return attributes
#else
var result: StyleAttributes = [:]
for attribute in attributes {
guard let string = attribute.key as? StyleAttributes.Key else {
fatalError("Can not convert key \(attribute.key) to String")
}
result[string] = value
#endif
}
return result
result[string] = attribute.value
}
return result
#endif
}

/// Adapt `attributedTitle`, for all control states, to the specified trait collection.
Expand All @@ -168,7 +168,7 @@ extension UISegmentedControl {
@objc(bon_updateTextForTraitCollection:)
public func adaptText(forTraitCollection traitCollection: UITraitCollection) {
for state in UIControl.State.commonStates {
let attributes = bon_titleTextAttributes(for: state)
guard let attributes = bon_titleTextAttributes(for: state) else { continue }
let newAttributes = NSAttributedString.adapt(attributes: attributes, to: traitCollection)
setTitleTextAttributes(newAttributes, for: state)
}
Expand Down