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

Commit

Permalink
Fixed Potential truncation issue in LabelLayout
Browse files Browse the repository at this point in the history 
- Added line break mode support in LabelLayout to calculate correct size based on line break mode.
- Added LabelLayout unit test to verify that size calculation logic considers line break mode while calculating LabelLayout size.
  • Loading branch information
@p-jadhav @pa1-linkedin
p-jadhav authored and pa1-linkedin committed Jun 17, 2019
1 parent c874993 commit 34abc8e
Show file tree
Hide file tree
Showing 7 changed files with 163 additions and 19 deletions.
87 changes: 85 additions & 2 deletions LayoutKitTests/LabelLayoutTests.swift
View file
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
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 = [NSAttributedString.Key.font: font]
let attributedTextWithFont = NSMutableAttributedString(string: string, attributes: fontAttribute)
return with(additionalAttributes: [NSAttributedString.Key.font: font])
}

/// Returns a new NSAttributedString with previous as well as additional attributes.
func with(additionalAttributes: [NSAttributedString.Key : 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
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@ open class LabelLayout<Label: UILabel>: BaseLayout<Label>, ConfigurableLayout {
public let font: UIFont
public let numberOfLines: Int
public let lineHeight: CGFloat
public 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
}

5 changes: 5 additions & 0 deletions Sources/ObjCSupport/Builders/LOKLabelLayoutBuilder.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@
*/
@property (nonatomic, nonnull, readonly) LOKLabelLayoutBuilder * _Nonnull(^numberOfLines)(NSInteger);

/**
@c LOKLabelLayoutBuilder block for setting line break mode of the label.
*/
@property (nonatomic, nonnull, readonly) LOKLabelLayoutBuilder * _Nonnull(^lineBreakMode)(NSLineBreakMode);

/**
@c LOKLabelLayoutBuilder block for setting line height of the label.
*/
Expand Down
12 changes: 12 additions & 0 deletions Sources/ObjCSupport/Builders/LOKLabelLayoutBuilder.m
View file
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
11 changes: 11 additions & 0 deletions Sources/ObjCSupport/LOKLabelLayout.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ import UIKit
*/
@objc public let font: UIFont

/**
Line break mode for label.
*/
@objc public let lineBreakMode: NSLineBreakMode

/**
Number of lines the label can have.
*/
Expand All @@ -55,6 +60,7 @@ import UIKit

@objc public init(attributedString: NSAttributedString,
font: UIFont?,
lineBreakMode: NSLineBreakMode,
lineHeight: CGFloat,
numberOfLines: Int,
alignment: LOKAlignment?,
Expand All @@ -64,6 +70,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 @@ -75,6 +82,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 @@ -86,6 +94,7 @@ import UIKit

@objc public init(string: String,
font: UIFont?,
lineBreakMode: NSLineBreakMode,
lineHeight: CGFloat,
numberOfLines: Int,
alignment: LOKAlignment?,
Expand All @@ -95,6 +104,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 @@ -106,6 +116,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

0 comments on commit 34abc8e

Please sign in to comment.