MessageLabel enabled detectors attributes

[antoinelamy]

General Information

• MessageKit Version: master

• iOS Version(s): all

• Swift Version: all

• Devices/Simulators: all

• Reproducible in ChatExample? (Yes/No): Yes

What happened?

MessageLabel's defaultAttributes assigned to addressAttributes, dateAttributes, phoneNumberAttributes and urlAttributes are assigned before textColor is set so all enabled detectors are always displayed in black.

What did you expect to happen?

Detected links, addresses, etc should display by default in the same color than the regular text. I could be fixed by assigning attributes in textColor didSet closure.

[antoinelamy]

Seems to be fixed on an up to date master branch

[antoinelamy]

Not quite fixed it seems. In MessageLabel line 247.

for detector in detectors {
    guard let rangeTuples = rangesForDetectors[detector] else { return }

should be

for detector in detectors {
    guard let rangeTuples = rangesForDetectors[detector] else { continue }

[antoinelamy]

Even after changing that, custom hyperlinks text color won't work. Please consider subclassing NSLayoutManager because of it's hard coded internal color for hyperlinks. I tested the solution and it works beautifully.

Sources:
https://www.cocoanetics.com/2015/03/customizing-uilabel-hyperlinks/
facebookarchive/AsyncDisplayKit#967

fileprivate class MessageLayoutManager: NSLayoutManager {
    override func showCGGlyphs(_ glyphs: UnsafePointer<CGGlyph>,
                               positions: UnsafePointer<CGPoint>,
                               count glyphCount: Int,
                               font: UIFont,
                               matrix textMatrix: CGAffineTransform,
                               attributes: [NSAttributedStringKey : Any] = [:], in graphicsContext: CGContext) {

        // NSLayoutManager has a hard coded internal color for hyperlinks which ignores
        // NSForegroundColorAttributeName. To get around this, we force the fill color
        // in the current context to match NSForegroundColorAttributeName.
        if let foregroundColor = attributes[.foregroundColor] as? UIColor {
            graphicsContext.setFillColor(foregroundColor.cgColor)
        }

        super.showCGGlyphs(glyphs,
                           positions: positions,
                           count: glyphCount,
                           font: font,
                           matrix: textMatrix,
                           attributes: attributes,
                           in: graphicsContext)
    }
}

[SD10]

@antoinelamy Good catch on the bug. Care to submit a PR?

[zhongwuzw]

@antoinelamy , Did you implement the detectorAttributes(for detector: DetectorType, and message: MessageType, at indexPath: IndexPath) in protocol of MessagesDisplayDelegate?
detectorAttributes set in dataSource phase and not use textColor, we use the NSAttributedStringKey.foregroundColor.

[antoinelamy]

@zhongwuzw yes detectorAttributes(for detector: DetectorType, and message: MessageType, at indexPath: IndexPath) was implemented but the attributes were never applied because if the first detector is not present in the text, every other detectors are skipped because of that return statement that should be a continue.

@SD10 of course I can submit a PR for this.

[SD10]

Cool @antoinelamy. About subclassing NSLayoutManager... are you sure that the attributes for the links are not applied? I just used NSUnderlineStyle & underline color and it worked 😅

[zhongwuzw]

@SD10 ,we don't needs subclass NSLayoutManager, because we used the foregroundColor on url by setting NSAttributedString not UILabel's implementation.

@antoinelamy , break statement is a bug, hope your PR👍.

[antoinelamy]

@SD10 yes you have to subclass NSLayoutManager, it's not all attributes that are ignored but only NSForegroundColorAttributeName. CoreText have an hardcoded foreground color for links and completely ignore the foreground color attribute. Just try to make an link in text appear in green, it won't work.

@zhongwuzw it doesn't matter that your are using NSTextContainer/NSLayoutManager/NSTextStorage directly instead of the UILabel implementation. NSLayoutManager still use CoreText inwards.

[zhongwuzw]

@antoinelamy , We don't use any like NSLinkAttributeName or other, we parse the text by myself to find the link position, then we set attributedString with what we like, that's all, so we can consider that link、address... as ordinary text. I think it's not related to NSLayoutManager except layout.

If you think the problem still existed, hope your Example.:relaxed::relaxed:

[antoinelamy]

In my case, I supply an attributed string with a NSLinkAttributeName in it as a MessageData. I provide it so I could replace the full URL by a custom text. I added this capability to the library since it was not supported. It would be nice to allow this use case.

[antoinelamy]

@zhongwuzw would you like me to add the required code to support NSLinkAttributeName in my PR? It's pretty straightforward, it consist in changing the parse function to the following:

private func parse(text: NSAttributedString) -> [NSTextCheckingResult] {
    guard enabledDetectors.isEmpty == false else { return [] }
    let checkingTypes = enabledDetectors.reduce(0) { $0 | $1.textCheckingType.rawValue }
    let detector = try? NSDataDetector(types: checkingTypes)
    let range = NSRange(location: 0, length: text.length)

    var parseResults: [NSTextCheckingResult] = []
    parseResults.append(contentsOf: detector?.matches(in: text.string, options: [], range: range) ?? [])
    parseResults.append(contentsOf: existingLinkAttributesResults(text: text))

    return parseResults
}

private func existingLinkAttributesResults(text: NSAttributedString) -> [NSTextCheckingResult] {
    var parseResults: [NSTextCheckingResult] = []
    text.enumerateAttributes(in: NSRange(location: 0, length: text.length), options: []) { (attributes, range, stop) in
        if let linkAttributeValue = attributes[NSAttributedStringKey.link] {
            if let url = linkAttributeValue as? URL {
                parseResults.append(NSTextCheckingResult.linkCheckingResult(range: range, url: url))
            } else if let urlString = linkAttributeValue as? String, let url = URL(string: urlString) {
                parseResults.append(NSTextCheckingResult.linkCheckingResult(range: range, url: url))
            }
        }
    }
    return parseResults
}

[zhongwuzw]

@antoinelamy Emm, If we add support to NSLinkAttributeName, I think it break the implication of detector, it just detects url,address....,what's your opinion, @SD10 ?

[antoinelamy]

In my case it works wonderfully.

[SD10]

Sorry for the slow response. I have quite the backlog of things to do here. I'm unfamiliar with NSLinkAttributeName so I can't really offer any input until I test it.

@antoinelamy
So you want to use NSLinkAttributeName with a .attributedText(NSAttributedString)?

Can you just use NSLinkAttributeName as the detector attributes for DetectorType.url?

[SD10]

I'm going to close this because the bug was resolved, feel free to open an issue for NSLinkAttributeName if you believe it is an issue. I will look as soon as I get the chance