Rightpoint / BonMot

Beautiful, easy attributed strings in Swift
MIT License
3.56k stars 197 forks source link

Apply color on image template #105

Closed aleufms closed 8 years ago

aleufms commented 8 years ago
let chain = BONChain().textColor(UIColor.redColor())
chain.appendLink(BONChain().image(UIImage(named: "image_name")))
chain.appendLink(BONChain().string("TEXT").text, separator: " ")
chain.appendLink(BONChain().image(UIImage(named: "image_name")), separator: " ")

The red color only is applied on the lastimage. The first image on the chain remains black.

ZevEisenberg commented 8 years ago

@aleufms thanks for your report! Would you be interested in submitting a pull request to fix it? Writing a unit test that demonstrates the failure is a good first step, although in this case, determining that the resulting string contains tinted images could be tricky. Let me know if you'd like to try - I'm more than happy to help out if you'd like.

ZevEisenberg commented 8 years ago

Come to think of it, I'm a little confused by what you're seeing. BonMot doesn't currently automatically tint images based on their text color, at least not until someone fixes #21. Could you provide a little more information, and maybe a sample image, of what you're seeing, @aleufms?

aleufms commented 8 years ago

Ok, sorry for the delay. I've been upload an example on https://github.com/aleufms/BonMotImageExample

ZevEisenberg commented 8 years ago

TL;DR you found a bug or secret feature in UIKit. Scroll to the bottom for my recommendation of what you should do.

OK, so, you've stumbled upon what is, as far as I can tell, either a bug or an undocumented feature of NSAttributedString:

let attachment = NSTextAttachment()

// image must be set to Template rendering mode
attachment.image = UIImage(named: "ic_navigate_next")

// Need to use a space, or this bug/feature isn't activated
let attributedString = NSMutableAttributedString(string: " ")

let attachmentString = NSAttributedString(attachment: attachment)

attributedString.appendAttributedString(attachmentString)
attributedString.addAttribute(NSForegroundColorAttributeName, value: UIColor.orangeColor(), range: NSMakeRange(0, 2))
label.attributedText = attributedString

This will result in a colored image:

screen shot 2016-02-09 at 9 57 31 pm

(The original image was black.)

If you do just an image, without a space, you don't get color:

let attachment = NSTextAttachment()
attachment.image = UIImage(named: "ic_navigate_next")

let attachmentString = NSAttributedString(attachment: attachment).mutableCopy() as! NSMutableAttributedString

attachmentString.addAttribute(NSForegroundColorAttributeName, value: UIColor.orangeColor(), range: NSMakeRange(0, 1))
label.attributedText = attachmentString
screen shot 2016-02-09 at 10 01 16 pm

I didn't know about this behavior until you posted this bug, and it's not supported in BonMot - see #21. I wonder if there's a way, in BonMot, to prevent it, so people are not confused in the future? For example, we could force the image to always render in Original mode in -attributedStringLastConcatenant: like this:

- (NSAttributedString *)attributedStringLastConcatenant:(BOOL)lastConcatenant
{
    NSMutableAttributedString *mutableAttributedString = nil;

    if (self.image) {
        NSAssert(!self.string, @"If self.image is non-nil, self.string must be nil");
        NSTextAttachment *attachment = [[NSTextAttachment alloc] init];
        attachment.image = [self.image imageWithRenderingMode:UIImageRenderingModeAlwaysOriginal];

@aleufms for your use case, I recommend pre-tinting your image using -[UIImage bon_tintedImageWithColor:] from UIImage+BonMotUtilities.

ZevEisenberg commented 8 years ago

Acceptance criteria for BonMot 4.0:

KingOfBrian commented 8 years ago

While commenting the method, I thought that we might want to support background color. Does that make sense to you?

ZevEisenberg commented 8 years ago

Background color for the image? Wouldn't NSBackgroundColorAttributeName take care of that?

KingOfBrian commented 8 years ago

Yea, not sure what I was thinking there. Carry on!

On Sunday, October 2, 2016, Zev Eisenberg notifications@github.com wrote:

Background color for the image? Wouldn't NSBackgroundColorAttributeName take care of that?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Raizlabs/BonMot/issues/105#issuecomment-250987087, or mute the thread https://github.com/notifications/unsubscribe-auth/AAG-kg0r38bucwyk7ZcAVfwa0CN8RcHqks5qv_l9gaJpZM4HSkNW .

ZevEisenberg commented 8 years ago

In the upcoming 4.0 release, images set to "automatic" or "always template" will now be tinted if they are styled with a color.