Skip to content
This repository has been archived by the owner on Feb 17, 2021. It is now read-only.

Fixed Potential truncation issue in LabelLayout #229

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fixed Potential truncation issue in LabelLayout #229

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
87 changes: 85 additions & 2 deletions LayoutKitTests/LabelLayoutTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@ import LayoutKit

class LabelLayoutTests: XCTestCase {

// For the defined `sampleText` and `labelLayoutMaxWidth` combination, `LabelLayout` requires 2 line of
// text for char-wrapping and 3 lines for word-wrapping/truncating-tail.
// So don't change this combination as the following tests are based on these values:
// - testSizeCalculationWithDifferentLineBreakMode
// - testAttributedTextSizeCalculationWithDifferentLineBreakMode
private static let sampleText = "Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Aenean comm"
private static let labelLayoutMaxWidth = 305

func testNeedsView() {
let l = LabelLayout(text: "hi").arrangement().makeViews()
XCTAssertNotNil(l as? UILabel)
Expand Down Expand Up @@ -200,10 +208,78 @@ class LabelLayoutTests: XCTestCase {
let maxSize = CGSize(width: 17, height: .max)
XCTAssertEqual(layout.measurement(within: maxSize).size, label.sizeThatFits(maxSize))
}

func testSizeCalculationWithDifferentLineBreakMode() {
// Use different line break mode for `LabelLayout` and dummy label.
// So in this case, size calculation should not match with dummy label's size calculation.
let label = UILabel(text: LabelLayoutTests.sampleText, numberOfLines: 0, lineBreakMode: .byCharWrapping)
let layout = LabelLayout(text: LabelLayoutTests.sampleText)
let maxSize = CGSize(width: LabelLayoutTests.labelLayoutMaxWidth, height: .max)
XCTAssertNotEqual(layout.measurement(within: maxSize).size, label.sizeThatFits(maxSize))
}

func testAttributedTextSizeCalculationWithDifferentLineBreakMode() {
// Use different line break mode for `LabelLayout` and dummy label.
// So in this case, size calculation should not match with dummy label's size calculation.
let attributedText = NSAttributedString(string: LabelLayoutTests.sampleText)
let label = UILabel(attributedText: attributedText, numberOfLines: 0, lineBreakMode: .byCharWrapping)
let layout = LabelLayout(attributedText: attributedText)
let maxSize = CGSize(width: LabelLayoutTests.labelLayoutMaxWidth, height: .max)
XCTAssertNotEqual(layout.measurement(within: maxSize).size, label.sizeThatFits(maxSize))
}

func testTextSizeCalculationWithSameLineBreakMode() {
// Use same line break mode for `LabelLayout` and dummy label and then match LabelLayout's size with dummy label's size calculation.
let lineBreakingModes: [NSLineBreakMode] = [
.byWordWrapping,
.byCharWrapping,
.byClipping,
.byTruncatingHead,
.byTruncatingTail,
.byTruncatingMiddle
]

lineBreakingModes.forEach { (lineBreakMode) in
verifyTextSizeCalculation(with: LabelLayoutTests.sampleText, lineBreakMode: lineBreakMode)
}
}

func testAttributedTextSizeCalculationWithSameLineBreakMode() {
// Use same line break mode for `LabelLayout` and dummy label and then match LabelLayout's size with dummy label's size calculation.
let lineBreakingModes: [NSLineBreakMode] = [
.byWordWrapping,
.byCharWrapping,
.byClipping,
.byTruncatingHead,
.byTruncatingTail,
.byTruncatingMiddle
]

let attributedText = NSAttributedString(string: LabelLayoutTests.sampleText)
lineBreakingModes.forEach { (lineBreakMode) in
verifyAttributedTextSizeCalculation(with: attributedText, lineBreakMode: lineBreakMode)
}
}

// MARK: Private Helpers

private func verifyTextSizeCalculation(with text: String, lineBreakMode: NSLineBreakMode) {
let label = UILabel(text: text, numberOfLines: 0, lineBreakMode: lineBreakMode)
let layout = LabelLayout(text: text, lineBreakMode: lineBreakMode)
let maxSize = CGSize(width: LabelLayoutTests.labelLayoutMaxWidth, height: .max)
XCTAssertEqual(layout.measurement(within: maxSize).size, label.sizeThatFits(maxSize))
}

private func verifyAttributedTextSizeCalculation(with attributedText: NSAttributedString, lineBreakMode: NSLineBreakMode) {
let label = UILabel(attributedText: attributedText, numberOfLines: 0, lineBreakMode: lineBreakMode)
let layout = LabelLayout(attributedText: attributedText, lineBreakMode: lineBreakMode)
let maxSize = CGSize(width: LabelLayoutTests.labelLayoutMaxWidth, height: .max)
XCTAssertEqual(layout.measurement(within: maxSize).size, label.sizeThatFits(maxSize))
}
}

extension UILabel {
convenience init(text: String, font: UIFont? = nil, numberOfLines: Int? = nil) {
convenience init(text: String, font: UIFont? = nil, numberOfLines: Int? = nil, lineBreakMode: NSLineBreakMode? = nil) {
self.init()
self.text = text
if let font = font {
Expand All @@ -212,16 +288,23 @@ extension UILabel {
if let numberOfLines = numberOfLines {
self.numberOfLines = numberOfLines
}
if let lineBreakMode = lineBreakMode {
self.lineBreakMode = lineBreakMode
}
}

convenience init(attributedText: NSAttributedString, font: UIFont? = nil, numberOfLines: Int? = nil) {
convenience init(attributedText: NSAttributedString, font: UIFont? = nil, numberOfLines: Int? = nil, lineBreakMode: NSLineBreakMode? = nil) {
self.init()
if let font = font {
self.font = font
}
if let numberOfLines = numberOfLines {
self.numberOfLines = numberOfLines
}
if let lineBreakMode = lineBreakMode {
self.lineBreakMode = lineBreakMode
}

// Want to set attributed text AFTER font it set, otherwise the font seems to take precedence.
self.attributedText = attributedText
}
Expand Down
18 changes: 10 additions & 8 deletions Sources/Internal/NSAttributedStringExtension.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,18 @@ extension NSAttributedString {

/// Returns a new NSAttributedString with a given font and the same attributes.
func with(font: UIFont) -> NSAttributedString {
let fontAttribute = [NSAttributedStringKey.font: font]
let attributedTextWithFont = NSMutableAttributedString(string: string, attributes: fontAttribute)
return with(additionalAttributes: [NSAttributedStringKey.font: font])
}

/// Returns a new NSAttributedString with previous as well as additional attributes.
func with(additionalAttributes: [NSAttributedStringKey : Any]?) -> NSAttributedString {
let attributedTextWithAdditionalAttributes = NSMutableAttributedString(string: string, attributes: additionalAttributes)
let fullRange = NSMakeRange(0, (string as NSString).length)
attributedTextWithFont.beginEditing()
attributedTextWithAdditionalAttributes.beginEditing()
self.enumerateAttributes(in: fullRange, options: .longestEffectiveRangeNotRequired, using: { (attributes, range, _) in
attributedTextWithFont.addAttributes(attributes, range: range)
attributedTextWithAdditionalAttributes.addAttributes(attributes, range: range)
})
attributedTextWithFont.endEditing()

return attributedTextWithFont
attributedTextWithAdditionalAttributes.endEditing()
return attributedTextWithAdditionalAttributes
}

}
15 changes: 14 additions & 1 deletion Sources/Layouts/LabelLayout.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@ open class LabelLayout<Label: UILabel>: BaseLayout<Label>, ConfigurableLayout {
open let font: UIFont
open let numberOfLines: Int
open let lineHeight: CGFloat
open let lineBreakMode: NSLineBreakMode

public init(text: Text,
font: UIFont = LabelLayoutDefaults.defaultFont,
lineHeight: CGFloat? = nil,
numberOfLines: Int = LabelLayoutDefaults.defaultNumberOfLines,
lineBreakMode: NSLineBreakMode = LabelLayoutDefaults.defaultLineBreakMode,
alignment: Alignment = LabelLayoutDefaults.defaultAlignment,
flexibility: Flexibility = LabelLayoutDefaults.defaultFlexibility,
viewReuseId: String? = nil,
Expand All @@ -31,13 +33,15 @@ open class LabelLayout<Label: UILabel>: BaseLayout<Label>, ConfigurableLayout {
self.numberOfLines = numberOfLines
self.font = font
self.lineHeight = lineHeight ?? font.lineHeight
self.lineBreakMode = lineBreakMode
super.init(alignment: alignment, flexibility: flexibility, viewReuseId: viewReuseId, config: config)
}

init(attributedString: NSAttributedString,
font: UIFont = LabelLayoutDefaults.defaultFont,
lineHeight: CGFloat? = nil,
numberOfLines: Int = LabelLayoutDefaults.defaultNumberOfLines,
lineBreakMode: NSLineBreakMode = LabelLayoutDefaults.defaultLineBreakMode,
alignment: Alignment = LabelLayoutDefaults.defaultAlignment,
flexibility: Flexibility = LabelLayoutDefaults.defaultFlexibility,
viewReuseId: String? = nil,
Expand All @@ -48,13 +52,15 @@ open class LabelLayout<Label: UILabel>: BaseLayout<Label>, ConfigurableLayout {
self.numberOfLines = numberOfLines
self.font = font
self.lineHeight = lineHeight ?? font.lineHeight
self.lineBreakMode = lineBreakMode
super.init(alignment: alignment, flexibility: flexibility, viewReuseId: viewReuseId, viewClass: viewClass ?? Label.self, config: config)
}

init(string: String,
font: UIFont = LabelLayoutDefaults.defaultFont,
lineHeight: CGFloat? = nil,
numberOfLines: Int = LabelLayoutDefaults.defaultNumberOfLines,
lineBreakMode: NSLineBreakMode = LabelLayoutDefaults.defaultLineBreakMode,
alignment: Alignment = LabelLayoutDefaults.defaultAlignment,
flexibility: Flexibility = LabelLayoutDefaults.defaultFlexibility,
viewReuseId: String? = nil,
Expand All @@ -65,6 +71,7 @@ open class LabelLayout<Label: UILabel>: BaseLayout<Label>, ConfigurableLayout {
self.numberOfLines = numberOfLines
self.font = font
self.lineHeight = lineHeight ?? font.lineHeight
self.lineBreakMode = lineBreakMode
super.init(alignment: alignment, flexibility: flexibility, viewReuseId: viewReuseId, viewClass: viewClass ?? Label.self, config: config)
}

Expand All @@ -74,6 +81,7 @@ open class LabelLayout<Label: UILabel>: BaseLayout<Label>, ConfigurableLayout {
font: UIFont = LabelLayoutDefaults.defaultFont,
lineHeight: CGFloat? = nil,
numberOfLines: Int = LabelLayoutDefaults.defaultNumberOfLines,
lineBreakMode: NSLineBreakMode = LabelLayoutDefaults.defaultLineBreakMode,
alignment: Alignment = LabelLayoutDefaults.defaultAlignment,
flexibility: Flexibility = LabelLayoutDefaults.defaultFlexibility,
viewReuseId: String? = nil,
Expand All @@ -83,6 +91,7 @@ open class LabelLayout<Label: UILabel>: BaseLayout<Label>, ConfigurableLayout {
font: font,
lineHeight: lineHeight,
numberOfLines: numberOfLines,
lineBreakMode: lineBreakMode,
alignment: alignment,
flexibility: flexibility,
viewReuseId: viewReuseId,
Expand All @@ -93,6 +102,7 @@ open class LabelLayout<Label: UILabel>: BaseLayout<Label>, ConfigurableLayout {
font: UIFont = LabelLayoutDefaults.defaultFont,
lineHeight: CGFloat? = nil,
numberOfLines: Int = LabelLayoutDefaults.defaultNumberOfLines,
lineBreakMode: NSLineBreakMode = LabelLayoutDefaults.defaultLineBreakMode,
alignment: Alignment = LabelLayoutDefaults.defaultAlignment,
flexibility: Flexibility = LabelLayoutDefaults.defaultFlexibility,
viewReuseId: String? = nil,
Expand All @@ -102,6 +112,7 @@ open class LabelLayout<Label: UILabel>: BaseLayout<Label>, ConfigurableLayout {
font: font,
lineHeight: lineHeight,
numberOfLines: numberOfLines,
lineBreakMode: lineBreakMode,
alignment: alignment,
flexibility: flexibility,
viewReuseId: viewReuseId,
Expand All @@ -116,7 +127,7 @@ open class LabelLayout<Label: UILabel>: BaseLayout<Label>, ConfigurableLayout {
}

private func textSize(within maxSize: CGSize) -> CGSize {
var size = text.textSize(within: maxSize, font: font)
var size = text.textSize(within: maxSize, font: font, lineBreakMode: lineBreakMode)
if numberOfLines > 0 {
let maxHeight = (CGFloat(numberOfLines) * lineHeight).roundedUpToFractionalPoint
if size.height > maxHeight {
Expand All @@ -134,6 +145,7 @@ open class LabelLayout<Label: UILabel>: BaseLayout<Label>, ConfigurableLayout {
open override func configure(view label: Label) {
config?(label)
label.numberOfLines = numberOfLines
label.lineBreakMode = lineBreakMode
label.font = font
switch text {
case .unattributed(let text):
Expand All @@ -155,6 +167,7 @@ public class LabelLayoutDefaults {
public static let defaultNumberOfLines = 0
public static let defaultFont = UILabel().font ?? UIFont.systemFont(ofSize: 17)
public static let defaultAlignment = Alignment.topLeading
public static let defaultLineBreakMode = NSLineBreakMode.byTruncatingTail
public static let defaultFlexibility = Flexibility.flexible
}

1 change: 1 addition & 0 deletions Sources/ObjCSupport/Builders/LOKLabelLayoutBuilder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

@property (nonatomic, nonnull, readonly) LOKLabelLayoutBuilder * _Nonnull(^font)(UIFont * _Nullable);
@property (nonatomic, nonnull, readonly) LOKLabelLayoutBuilder * _Nonnull(^numberOfLines)(NSInteger);
@property (nonatomic, nonnull, readonly) LOKLabelLayoutBuilder * _Nonnull(^lineBreakMode)(NSLineBreakMode);
@property (nonatomic, nonnull, readonly) LOKLabelLayoutBuilder * _Nonnull(^lineHeight)(CGFloat);

@property (nonatomic, nonnull, readonly) LOKLabelLayoutBuilder * _Nonnull(^alignment)(LOKAlignment * _Nullable);
Expand Down
12 changes: 12 additions & 0 deletions Sources/ObjCSupport/Builders/LOKLabelLayoutBuilder.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ @interface LOKLabelLayoutBuilder ()
@property (nonatomic, nullable) NSAttributedString *privateAttributedString;
@property (nonatomic, nullable) UIFont *privateFont;
@property (nonatomic) NSInteger privateNumberOfLines;
@property (nonatomic) NSLineBreakMode privateLineBreakMode;
@property (nonatomic) CGFloat privateLineHeight;
@property (nonatomic, nullable) void (^ privateConfigure)(UILabel * _Nonnull);

Expand All @@ -31,12 +32,14 @@ @implementation LOKLabelLayoutBuilder
- (instancetype)initWithString:(NSString *)string {
self = [super init];
_privateString = string;
_privateLineBreakMode = NSLineBreakByTruncatingTail;
return self;
}

- (instancetype)initWithAttributedString:(NSAttributedString *)attributedString {
self = [super init];
_privateAttributedString = attributedString;
_privateLineBreakMode = NSLineBreakByTruncatingTail;
return self;
}

Expand All @@ -53,6 +56,7 @@ - (nonnull LOKLabelLayout *)layout {
if (self.privateAttributedString) {
return [[LOKLabelLayout alloc] initWithAttributedString:self.privateAttributedString
font:self.privateFont
lineBreakMode:self.privateLineBreakMode
lineHeight:self.privateLineHeight
numberOfLines:self.privateNumberOfLines
alignment:self.privateAlignment
Expand All @@ -63,6 +67,7 @@ - (nonnull LOKLabelLayout *)layout {
} else {
return [[LOKLabelLayout alloc] initWithString:self.privateString
font:self.privateFont
lineBreakMode:self.privateLineBreakMode
lineHeight:self.privateLineHeight
numberOfLines:self.privateNumberOfLines
alignment:self.privateAlignment
Expand All @@ -87,6 +92,13 @@ - (nonnull LOKLabelLayout *)layout {
};
}

- (LOKLabelLayoutBuilder * _Nonnull (^)(NSLineBreakMode))lineBreakMode {
return ^LOKLabelLayoutBuilder *(NSLineBreakMode lineBreakMode){
self.privateLineBreakMode = lineBreakMode;
return self;
};
}

- (LOKLabelLayoutBuilder * _Nonnull (^)(CGFloat))lineHeight {
return ^LOKLabelLayoutBuilder *(CGFloat lineHeight){
self.privateLineHeight = lineHeight;
Expand Down
7 changes: 7 additions & 0 deletions Sources/ObjCSupport/LOKLabelLayout.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,15 @@ import UIKit
@objc public let string: String?
@objc public let lineHeight: CGFloat
@objc public let font: UIFont
@objc public let lineBreakMode: NSLineBreakMode
@objc public let numberOfLines: Int
@objc public let alignment: LOKAlignment
@objc public let viewClass: UILabel.Type
@objc public let configure: ((UILabel) -> Void)?

@objc public init(attributedString: NSAttributedString,
font: UIFont?,
lineBreakMode: NSLineBreakMode,
lineHeight: CGFloat,
numberOfLines: Int,
alignment: LOKAlignment?,
Expand All @@ -29,6 +31,7 @@ import UIKit
configure: ((UILabel) -> Void)?) {
self.attributedString = attributedString
self.font = font ?? UIFont.systemFont(ofSize: UIFont.systemFontSize)
self.lineBreakMode = lineBreakMode
self.lineHeight = lineHeight
self.numberOfLines = numberOfLines
self.alignment = alignment ?? .topLeading
Expand All @@ -40,6 +43,7 @@ import UIKit
font: self.font,
lineHeight: lineHeight > 0 && lineHeight.isFinite ? lineHeight : Optional<CGFloat>.none,
numberOfLines: self.numberOfLines,
lineBreakMode: self.lineBreakMode,
alignment: self.alignment.alignment,
flexibility: flexibility?.flexibility ?? .flexible,
viewReuseId: viewReuseId,
Expand All @@ -51,6 +55,7 @@ import UIKit

@objc public init(string: String,
font: UIFont?,
lineBreakMode: NSLineBreakMode,
lineHeight: CGFloat,
numberOfLines: Int,
alignment: LOKAlignment?,
Expand All @@ -60,6 +65,7 @@ import UIKit
configure: ((UILabel) -> Void)?) {
self.string = string
self.font = font ?? UIFont.systemFont(ofSize: UIFont.systemFontSize)
self.lineBreakMode = lineBreakMode
self.lineHeight = lineHeight
self.numberOfLines = numberOfLines
self.alignment = alignment ?? .topLeading
Expand All @@ -71,6 +77,7 @@ import UIKit
font: self.font,
lineHeight: lineHeight > 0 && lineHeight.isFinite ? lineHeight : Optional<CGFloat>.none,
numberOfLines: self.numberOfLines,
lineBreakMode: self.lineBreakMode,
alignment: self.alignment.alignment,
flexibility: flexibility?.flexibility ?? .flexible,
viewReuseId: viewReuseId,
Expand Down
Loading