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

Commit

Permalink
HUED-8605 Test & Fix Potential truncation issue in LabelLayout
Browse files Browse the repository at this point in the history
- Added LabelLayout unit test to verify that size calculation logic considers line break mode while calculating LabelLayout size.
- Added line break mode support in LabelLayout to calculate correct size based on line break mode.
  • Loading branch information
PrkumariLI committed Aug 27, 2018
1 parent dc2ff66 commit 1e91330
Show file tree
Hide file tree
Showing 7 changed files with 207 additions and 19 deletions.
82 changes: 80 additions & 2 deletions LayoutKitTests/LabelLayoutTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ 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 tests are based on these values.
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 +206,75 @@ class LabelLayoutTests: XCTestCase {
let maxSize = CGSize(width: 17, height: .max)
XCTAssertEqual(layout.measurement(within: maxSize).size, label.sizeThatFits(maxSize))
}

func testSizeCalculationWithWordWrapping() {
// Use same line break mode for `LabelLayout` and dummy label and then match LabelLayout's size with dummy label's size calculation.
verifyTextSizeCalculation(with: LabelLayoutTests.sampleText, lineBreakMode: .byWordWrapping)
}

func testSizeCalculationWithCharWrapping() {
// Use same line break mode for `LabelLayout` and dummy label and then match LabelLayout's size with dummy label's size calculation.
verifyTextSizeCalculation(with: LabelLayoutTests.sampleText, lineBreakMode: .byCharWrapping)
}

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, config: { label in
label.text = LabelLayoutTests.sampleText
})
let maxSize = CGSize(width: LabelLayoutTests.labelLayoutMaxWidth, height: .max)
XCTAssertNotEqual(layout.measurement(within: maxSize).size, label.sizeThatFits(maxSize))
}

func testAttributedTextSizeCalculationWithWordWrapping() {
// Use same line break mode for `LabelLayout` and dummy label and then match LabelLayout's size with dummy label's size calculation.
let attributedText = NSAttributedString(string: LabelLayoutTests.sampleText)
verifyAttributedTextSizeCalculation(with: attributedText, lineBreakMode: .byWordWrapping)
}

func testAttributedTextSizeCalculationWithCharWrapping() {
// Use same line break mode for `LabelLayout` and dummy label and then match LabelLayout's size with dummy label's size calculation.
let attributedText = NSAttributedString(string: LabelLayoutTests.sampleText)
verifyAttributedTextSizeCalculation(with: attributedText, lineBreakMode: .byCharWrapping)
}

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, config: { label in
label.attributedText = attributedText
})
let maxSize = CGSize(width: LabelLayoutTests.labelLayoutMaxWidth, height: .max)
XCTAssertNotEqual(layout.measurement(within: maxSize).size, label.sizeThatFits(maxSize))
}

// 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, config: { label in
label.text = text
})
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, config: { label in
label.attributedText = attributedText
})
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 +283,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
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
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
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
8 changes: 8 additions & 0 deletions Sources/ObjCSupport/Builders/LOKLabelLayoutBuilder.m
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 Down Expand Up @@ -87,6 +88,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
72 changes: 72 additions & 0 deletions Sources/ObjCSupport/LOKLabelLayout.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@ 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)?

// TODO: Remove this init once all consumers are switch to `init(attributedString:font:lineBreakMode:lineHeight:numberOfLines:alignment:flexibility:viewReuseId:viewClass:configure:)`
@objc public init(attributedString: NSAttributedString,
font: UIFont?,
lineHeight: CGFloat,
Expand All @@ -29,6 +31,7 @@ import UIKit
configure: ((UILabel) -> Void)?) {
self.attributedString = attributedString
self.font = font ?? UIFont.systemFont(ofSize: UIFont.systemFontSize)
self.lineBreakMode = nil
self.lineHeight = lineHeight
self.numberOfLines = numberOfLines
self.alignment = alignment ?? .topLeading
Expand All @@ -49,6 +52,7 @@ import UIKit
super.init(layout: layout)
}

// TODO: Remove this init once all consumers are switch to `init(string:font:lineBreakMode:lineHeight:numberOfLines:alignment:flexibility:viewReuseId:viewClass:configure:)`
@objc public init(string: String,
font: UIFont?,
lineHeight: CGFloat,
Expand All @@ -60,6 +64,7 @@ import UIKit
configure: ((UILabel) -> Void)?) {
self.string = string
self.font = font ?? UIFont.systemFont(ofSize: UIFont.systemFontSize)
self.lineBreakMode = nil
self.lineHeight = lineHeight
self.numberOfLines = numberOfLines
self.alignment = alignment ?? .topLeading
Expand All @@ -78,4 +83,71 @@ import UIKit
config: self.configure)
super.init(layout: layout)
}

@objc public init(attributedString: NSAttributedString,
font: UIFont?,
lineBreakMode: NSlineBreakMode?,
lineHeight: CGFloat,
numberOfLines: Int,
alignment: LOKAlignment?,
flexibility: LOKFlexibility?,
viewReuseId: String?,
viewClass: UILabel.Type?,
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
self.viewClass = viewClass ?? UILabel.self
self.configure = configure
string = nil
let layout = LabelLayout<UILabel>(
attributedString: attributedString,
font: self.font,
lineHeight: lineHeight > 0 && lineHeight.isFinite ? lineHeight : Optional<CGFloat>.none,
numberOfLines: self.numberOfLines,
lineBreakMode: self.lineBreakMode ?? .byTruncatingTail,
alignment: self.alignment.alignment,
flexibility: flexibility?.flexibility ?? .flexible,
viewReuseId: viewReuseId,
viewClass: self.viewClass,
config: self.configure)

super.init(layout: layout)
}

@objc public init(string: String,
font: UIFont?,
lineBreakMode: NSlineBreakMode?,
lineHeight: CGFloat,
numberOfLines: Int,
alignment: LOKAlignment?,
flexibility: LOKFlexibility?,
viewReuseId: String?,
viewClass: UILabel.Type?,
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
self.viewClass = viewClass ?? UILabel.self
self.configure = configure
attributedString = nil
let layout = LabelLayout<UILabel>(
string: string,
font: self.font,
lineHeight: lineHeight > 0 && lineHeight.isFinite ? lineHeight : Optional<CGFloat>.none,
numberOfLines: self.numberOfLines,
lineBreakMode: self.lineBreakMode ?? .byTruncatingTail,
alignment: self.alignment.alignment,
flexibility: flexibility?.flexibility ?? .flexible,
viewReuseId: viewReuseId,
viewClass: self.viewClass,
config: self.configure)
super.init(layout: layout)
}
}
Loading

0 comments on commit 1e91330

Please sign in to comment.