facebookarchive / AsyncDisplayKit

Smooth asynchronous user interfaces for iOS apps.
http://asyncdisplaykit.org
Other
13.4k stars 2.2k forks source link

TextNode with nil attributed text calculates a non-zero height #2555

Closed hannahmbanana closed 8 years ago

hannahmbanana commented 8 years ago

IMO, this seems like incorrect behavior, but correct me if I'm wrong. :)

Repro using example project #2551 (examples/LayoutSpecExamples). These cells contain a title & description in a vertical stack. The textNode description for the 3rd cell has a nil attributedText set on it.

image

cc @Adlai-Holler @maicki (I'm not sure if this is a layout issue or a text node issue)

garrettmoon commented 8 years ago

Just to clarify, is this a nil string or an empty string? UIKit returns non-zero height for empty strings, not sure about nil strings though…

hannahmbanana commented 8 years ago

Both a nil string and an empty string produce the same layout behavior.

maicki commented 8 years ago

@hannahmbanana Do you set any height to the text node or the intrinsic size returns that size? If you set a breakpoint in calculateSizeThatFits: in ASTextNode what's the value of the size it returns?

hannahmbanana commented 8 years ago

@maicki - No height is set in the layout code (copied below). ASTextNode's calculateSizeThatFits returns {0, 13.800000000000001}

- (ASLayoutSpec *)layoutSpecThatFits:(ASSizeRange)constrainedSize
{
    ASStackLayoutSpec *verticalStackSpec = [ASStackLayoutSpec verticalStackLayoutSpec];
    verticalStackSpec.alignItems = ASStackLayoutAlignItemsStart;
    verticalStackSpec.spacing = 5.0;
    verticalStackSpec.children = @[self.titleNode, self.descriptionNode];

    return [ASInsetLayoutSpec insetLayoutSpecWithInsets:UIEdgeInsetsMake(10, 16, 10, 10) child:verticalStackSpec];
}
garrettmoon commented 8 years ago

I think this is actually correct behavior. Even if it returned zero height it would result in a layout that the developer wouldn't want. I.e. it would have the spacing around an empty item. Unless the stack speck removed spacing for empty items…

maicki commented 8 years ago

I will close that issue as @garrettmoon mentioned this is correct behavior. I would suggest if you don't want to have the text node appearing if the string is nil / empty don't add it in layoutSpecThatFits:.