Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply color on image template #105

Closed
aleufms opened this issue Feb 3, 2016 · 9 comments
Closed

Apply color on image template #105

aleufms opened this issue Feb 3, 2016 · 9 comments
Assignees
Labels
Milestone

Comments

@aleufms
Copy link

aleufms commented Feb 3, 2016

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
Copy link
Collaborator

@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
Copy link
Collaborator

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
Copy link
Author

aleufms commented Feb 4, 2016

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

@ZevEisenberg
Copy link
Collaborator

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.

@KingOfBrian KingOfBrian modified the milestone: 4.0 Sep 6, 2016
@ZevEisenberg
Copy link
Collaborator

ZevEisenberg commented Sep 23, 2016

Acceptance criteria for BonMot 4.0:

  • Exercise bug somewhere verifiable (demo app or unit test)
  • Decide on re-tinting image or non-breaking space as method of fixing.
    • if re-tinting, port image tinting code to Swift and include it
  • Implement fix (Only template images)

@KingOfBrian KingOfBrian added the P1 label Sep 27, 2016
@KingOfBrian
Copy link
Contributor

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

@ZevEisenberg
Copy link
Collaborator

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

@KingOfBrian
Copy link
Contributor

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
#105 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAG-kg0r38bucwyk7ZcAVfwa0CN8RcHqks5qv_l9gaJpZM4HSkNW
.

@ZevEisenberg
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants