zacwest / ZSWTappableLabel

UILabel subclass for links which are tappable, long-pressable, 3D Touchable, and VoiceOverable.
MIT License
169 stars 37 forks source link

Tapping on link propagates link style to whole attributed string #5

Closed mergesort closed 9 years ago

mergesort commented 9 years ago

I'm having an issue where I tap on a link, and it takes a string that was bolded only on the a attribute, and makes the entire label bold, and can't quite track it down.

I've subclassed ZSWTappableLabel so I can create methods that take in a string and convert it to an attributed string with my own custom attributes.

The results look like this.

This is before I tapped on it. @victoriasj is bold because it is the link, and visited is not bold, because it is not part of the link.

This is after I tapped on it. (In this case I tapped on each label to demonstrate what happens, not tapping on one influencing the other cells). Note that @victoriasj is bold as it should be, but visited is as well, which it shouldn't be.

After tapping (on each label individually in this case)

The relevant parts of the subclass looks like this:

- (void)setDataDetectingText:(NSString *)text linkColor:(UIColor *)color highlightedColor:(UIColor *)highlightedColor linkFont:(UIFont *)font
{
    self.userInteractionEnabled = YES;

    NSString *htmlText = [self anchorTaggedString:text withDataDetectors:DataDetectorTypeAll];

    ZSWTaggedStringOptions *options = [ZSWTaggedStringOptions options];
    [options setDynamicAttributes:^NSDictionary *(NSString *tagName, NSDictionary *tagAttributes, NSDictionary *existingStringAttributes) {
        NSString *href = tagAttributes[@"href"];
        NSURL *URL = href.length > 0 ? [NSURL URLWithString:href] : nil;

        if (!URL)
        {
            return nil;
        }
        else
        {
            return @{
                        ZSWTappableLabelTappableRegionAttributeName : @YES,
                        NSUnderlineStyleAttributeName : @(NSUnderlineStyleNone),
                        ZSWTappableLabelHighlightedBackgroundAttributeName : [UIColor clearColor],
                        ZSWTappableLabelHighlightedForegroundAttributeName : highlightedColor,
                        NSForegroundColorAttributeName : color,
                        NSFontAttributeName : font,
                        URLMetadataKey : URL
                    };

        }
    } forTagName:@"a"];

    self.attributedText = [[ZSWTaggedString stringWithString:htmlText] attributedStringWithOptions:options];
}

- (void)setDataDetectingText:(NSString *)text
{
    [self setDataDetectingText:text linkColor:self.textColor highlightedColor:[UIColor lightGrayColor] linkFont:[UIFont boldSystemFontOfSize:self.font.pointSize]];
}

////////////////////////////////////////////////////////////////////////////////
#pragma mark - Delegation - ZSWTappableLabelDelegate

- (void)tappableLabel:(ZSWTappableLabel *)tappableLabel tappedAtIndex:(NSInteger)idx withAttributes:(NSDictionary *)attributes
{
    NSURL *URL = attributes[URLMetadataKey];

    if ([UsernameMetadataDetector URLContainsMetadata:URL])
    {
        if (self.tappedMetadata)
        {
            self.tappedMetadata([UsernameMetadataDetector metadataFromURL:URL]);
        }
    }
    else if ([[UIApplication sharedApplication] canOpenURL:URL])
    {
        [[UIApplication sharedApplication] openURL:URL];
    }
}

////////////////////////////////////////////////////////////////////////////////
#pragma mark - Private

- (NSString *)anchorTaggedString:(NSString *)string withDataDetectors:(DataDetectorType)type
{
    NSString *htmlText = nil;

    if (string.length > 0)
    {
        NSString * (^anchorForUsername) (NSString *) = ^(NSString *string){
            NSString *template = nil;
            template = [NSString stringWithFormat:@"<a href=\"%@\">%@</a>", [UsernameMetadataDetector URLStringFromMetadataString:@"$0"], [UsernameMetadataDetector unprefixedUsernameString:@"$0"]];
            NSRegularExpression *regex = [UsernameMetadataDetector usernameRegularExpression];
            NSString *replacedString = [regex stringByReplacingMatchesInString:string options:0 range:NSMakeRange(0, string.length) withTemplate:template];
            return replacedString;
        };

        if (type == DataDetectorTypeAll)
        {
            htmlText = anchorForUsername(string);
            htmlText = anchorForList(string);
        }
        else if (type == DataDetectorTypeUsername)
        {
            htmlText = anchorForUsername(string);
        }
    }

    return htmlText;
}

Other things to note: Even if I disable the tapDelegate (which is self), the issue still occurs.

Thanks a lot for the great work! This issue aside, using ZSWTappableLabel has been terrific.

zacwest commented 9 years ago

Can you add a breakpoints to:

You may need to throw a -[UILabel setAttributedText:] in if these aren't useful since it invokes super to handle highlight behavior.

Since you're only seeing it after tapping and not highlighting it could be one of two things that come to mind:

  1. Something is calling -setText: and it's resetting the attributed version. This will take the font UILabel chooses (e.g., index 0) and apply it to the full string as the new attributed version.
  2. Something is broken with ZSWTappableLabel storing the original attributed version, which might be more obvious if we can see how the highlight removal isn't working.
mergesort commented 9 years ago

I made a mistake, I didn't think about the distinction of tapping and highlighting. It's actually happening on highlighting, not after tapping.

When I highlight (or tap), -setText: and -setAttributedText: do not get called at all. (Which I think makes sense).

self.unmodifiedAttributedText appears to remain unchanged throughout the process, even after watching it highlight. Going through -applyHighlightAtIndex:, the resulting attributedString doesn't appear to ever differ from the unmodifiedAttributedText.

The end result being:

@puck{
    NSColor = "UIDeviceRGBColorSpace 0.25451 0.293333 0.353726 1";
    NSFont = "<UICTFont: 0x7fe1e62f7030> font-family: \"Avenir-Heavy\"; font-weight: bold; font-style: normal; font-size: 13.00pt";
    NSUnderline = 0;
    URL = "picks://username?=@puck";
    ZSWTappableLabelHighlightedForegroundAttributeName = "UIDeviceRGBColorSpace 0.00392157 0.494118 0.8 1";
    ZSWTappableLabelTappableRegionAttributeName = 1;
} gave this ★★★★★{
    NSFont = "<UICTFont: 0x7fe1e62f7030> font-family: \"Avenir-Heavy\"; font-weight: bold; font-style: normal; font-size: 13.00pt";
}

It appears above that it's setting the font of the part that shouldn't be highlighted to be the same as the part that should be highlighted (Avenir-Heavy instead of Avenir-Book). I tried setting self.font explicitly before calling setDynamicAttributes: forTagName: and it still ends up adding the heavy font to the rest of the string. Despite setting the font to be heavy, it's very visibly not a heavy font in the UI, so maybe something's happening in performWithLayoutManager? I'm a bit out of my element not having worked much with NSLayoutManager.

mergesort commented 9 years ago

I just went through the readme again, and noticed that I had never called -setBaseAttributes: on the ZSWTaggedStringOptions. Once I did, the highlighting issue went away.

I'm going to close this issue since it's effectively fixed, but leave this suggestion, maybe for an enhancement?

Have baseAttributes should pull from the label by default if it goes unset?

zacwest commented 9 years ago

In your example, could you print out what the [super font] value is before and after doing -setAttributedText: on the label?

You're running into, I think, UILabel being weird about how it sets self.font - I believe it behaves like so:

  1. Font is normal
  2. String is set, where unknown values are filled with normal
  3. Font is updated to new string, first index is bold
  4. ZSWTappableLabel saves the string, filling in blanks with bold

Visually, until ZSWTappableLabel executes, it's using the value saved in (2), but when the label goes to update due to highlight changes, it's using the value saved in (4).

I think this is a bug in this class, but it's pretty nuanced. I can picture a few solutions:

  1. We re-apply the string from (4) so it's obvious what's happening.
  2. We throw an exception if any parts of the string lack fonts, or at least log to console.
  3. We grab the font before setting super, so (4) matches (2). Note that self.font would be "wrong" still, since UILabel is using idx0 after setting.
  4. We grab the font before setting super, and re-apply it after (4).

I'll have to think about this some more, but I'm glad you were able to figure it out.

mergesort commented 9 years ago

That sounds plausible, since the font does seem to change from before and after -setAttributedText:.

Before: <UICTFont: 0x7f83be1d8350> font-family: "Avenir-Book"; font-weight: normal; font-style: normal; font-size: 13.00pt

After: <UICTFont: 0x7f83be50c5e0> font-family: "Avenir-Heavy"; font-weight: bold; font-style: normal; font-size: 13.00pt