facebookarchive / AsyncDisplayKit

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

[ASLayoutSpec] ASNetworkImageNode with unknown height don't update ASCellNode's height #718

Closed vitalybaev closed 8 years ago

vitalybaev commented 9 years ago

Hi! There is ASCellNode with vertical ASStackLayoutSpec with 3 children:

When image not yet downloaded I'd like to place gray placeholder (horizontal rectangle, like in Twitter app), but when image downloaded it had to be full size.

Here is test project https://github.com/vitalybaev/Test-ASDK of this

When image downloaded, ASCellNode's size doesn't changed. So if image height greater than placeholder's height - there is white gap, if less than placeholder - some content doesn't visible;

Got: screen shot 2015-10-06 at 20 36 53 Expect screen shot 2015-10-06 at 20 36 59

appleguy commented 9 years ago

Quick response — have you tried using preferredFrameSize? Especially if you are using ASLayoutSpec to position everything, this will ensure the image is shown at the desired size even when the placeholder state (e.g. background color, or a default image you set for .placeholderImage) is shown.

On Oct 6, 2015, at 10:38 AM, Vitaly Baev notifications@github.com wrote:

Hi! There is ASCellNode with vertical ASStackLayoutSpec with 3 children:

ASTextNode - Title ASNetworkImageNode - Some internet image with various size ASTextNode - Some text When image not yet downloaded I'd like to place gray placeholder (horizontal rectangle, like in Twitter app), but when image downloaded it had to be full size.

Here is test project https://github.com/vitalybaev/Test-ASDK https://github.com/vitalybaev/Test-ASDK of this

When image downloaded, ASCellNode's size doesn't changed. So if image height greater than placeholder's height - there is white gap, if less than placeholder - some content doesn't visible;

Got: https://cloud.githubusercontent.com/assets/724423/10316840/08a59c16-6c6a-11e5-9f29-0bb27759dbfb.png Expect https://cloud.githubusercontent.com/assets/724423/10316861/1dd8ebc4-6c6a-11e5-879a-5c6ac4014a40.png — Reply to this email directly or view it on GitHub https://github.com/facebook/AsyncDisplayKit/issues/718.

appleguy commented 9 years ago

@vitalybaev ah, so are you asking about animating the size of the cells? I am very happy you provided images as it REALLY helps understand your use case. However, I don't completely understand these because neither of them have an image placeholder state, and the first one also doesn't show the "some text at the bottom". So I'm not sure what the desired behavior is, that you would like it to have, to get from the placeholder state to the final state. Happy to help figure it out once I better understand the intended behavior!

Look at the examples/Kittens app for some examples of cell resizing and relayout. When you tap on a cell, it does a relayout, which involves calling -invalidateCalculatedLayout and calling -reloadRowAtIndexPath:.

@nguyenhuy sorry to ask again, but could you remind me if there were any other method calls required here? The ASTableView /SHOULD/ be able to detect that the cell has an invalidated layout, and requires a synchronous measure pass as part of the row reload, but does it do that right now? Lastly, can you think of an API improvement we can make for this to be a one-line call? Two ideas for that:

vitalybaev commented 9 years ago

@appleguy thanks for answer! About preferedframesize - no, I'm on my way to try it. However, I opened this issue because when I started using ASDK about 2 month ago this case worked ok for me.

vitalybaev commented 9 years ago

@appleguy More about my use case:

1) images not yet downloaded - using ASRatioLayoutSpec with 0.5 ratio - exactly ratio of images on first screenshot 2) but when image downloaded it has to be full sized, so ratio is now 1. In ASNetworkImageNode's delegate I call an setNeedsLayout for cell 3) but until scrolling offscreen and back again - cell height will be old

nguyenhuy commented 9 years ago

@vitalybaev That's probably because the table/collection view doesn't refresh itself until the cell is offscreen. You can easily fix that by triggering an empty transaction on the table view (-beginUpdates followed immediately by -endUpdates) or the collection view (-performBatchUpdates:^(){} completion:nil]). Let me know if this helps.

nguyenhuy commented 9 years ago

@appleguy Yes, I agree that we can improve the API to handle this case more gratefully and robustly. Your later idea seems to the better one, since the new method can be regarded as an edit operation. I will work on it really soon. It shouldn't be hard anyway.

vitalybaev commented 9 years ago

@appleguy @nguyenhuy Thanks! Calling -beginUpdates and -endUpdates works fine now! Thanks for great product!

nguyenhuy commented 9 years ago

You are welcome, @vitalybaev. I will notify you once the new API is available.

robmontesinos commented 9 years ago

I am not clear on how to resize an ASNetworkImageNode after its image has been downloaded to have a correct aspect ratio using ASStackLayoutSpec. I was able to get the image to show using:


        [self.tableView beginUpdates];
        [cellNode setNeedsLayout];
        [self.tableView endUpdates];

but as you can see there is a band above and below the image.

By the way my screen scrolls waaaay faster with ASDK!!! Thank you guys!!!

asdkscreenshot6

nguyenhuy commented 9 years ago

I'm guessing you are using a ratio of 1 fixed ratio for the image node, right? You can do like this to reserve the backing image's ratio:

if let image = imageNode.image { // Make sure image is available/downloaded
  let imageSize = image.size
  let reserveImageRatioSpec = ASRatioLayoutSpec(ratio: imageSize.height / imageSize.width, child: imageNode)
 // Then use reserveImageRatioSpec in the final spec
}

(It's in Swift, but you get the idea. Original code can be found here)

robmontesinos commented 9 years ago

Thank you @nguyenhuy - I understand how to determine the ratio based in image size after download. I don't understand how to apply the ASRatioLayoutSpec to an imageNode that is inside a complex stack arrangement. I see something called ASOverlayLayoutSpec - I don't see where it is implemented - I'll give it a whack - any advice would be appreciated as always. SAlud

nguyenhuy commented 9 years ago

After setting the imageNode as the child of a ASRatioLayoutSpec, you can just add the ratio spec to a stack spec the same way you would do with imageNode. Does this answer your question clearly?

robmontesinos commented 9 years ago

@nguyenhuy You da Man!! Worked like a charm - just in case anyone wants to see an approach as a code sample:

This code is at the end of layoutSpecThatFits:(ASSizeRange) constrainedSize in an ASCellNode

Call Tree goes ASNetworkImageNodeDelegate (imageNode:(ASNetworkImageNode )imageNode didLoadImage:(UIImage )image) --> ASCellNode delegate tells ViewController download complete and passes indexPath which was set as part of init - (id)initWithIndexPath: --> tableView sends cellNode a setNeedsLayout message in between beginUpdates and endUpdates


    ASInsetLayoutSpec *insetSpec = [[ASInsetLayoutSpec alloc] init];
    insetSpec.insets = UIEdgeInsetsMake(kOuterPadding, kOuterPadding, kOuterPadding, kOuterPadding);

    if (self.tweet.tweetImageLink != nil) {
        ASStackLayoutSpec *nodeImageStack = [[ASStackLayoutSpec alloc] init];
        nodeImageStack.direction = ASStackLayoutDirectionVertical;

        UIImage *tweetImage = self.tweetImageNode.image;
        if (tweetImage) {
            ASRatioLayoutSpec *tweetImageRatioSpec = [[ASRatioLayoutSpec alloc] init];
            tweetImageRatioSpec.ratio = tweetImage.size.height / tweetImage.size.width;
            tweetImageRatioSpec.child = self.tweetImageNode;

            nodeImageStack.children = @[topHStack, verticalSpacer, buttonHStack, verticalSpacer, tweetImageRatioSpec];
        } else {
            nodeImageStack.children = @[topHStack, verticalSpacer, buttonHStack, verticalSpacer, self.tweetImageNode];
        }

        nodeImageStack.spacingAfter = kOuterPadding;
        nodeImageStack.spacingBefore = kOuterPadding;

        insetSpec.child = nodeImageStack;
    } else {
        ASStackLayoutSpec *nodeStack = [[ASStackLayoutSpec alloc] init];
        nodeStack.direction = ASStackLayoutDirectionVertical;
        nodeStack.children = @[topHStack, verticalSpacer, buttonHStack];

        insetSpec.child = nodeStack;
    }

    return insetSpec;
macistador commented 8 years ago

Is this an open issue ? As @nguyenhuy's PR is merged this issue seems fixed to me, isn'it ?

bimawa commented 8 years ago

Guys i have a same question for ASCollectionVIew. How refresh item without reload table? After loaded image and sizes of images? Additional info: ADK resized ASCellView after loaded data from network and relayout nodes. But ASCollectionView not change contentSize. ADK 1.9.6 Now i solve this with [self.viewNode.collectionNode.view reloadSections:[[NSIndexSet alloc] initWithIndex:0]];

hannahmbanana commented 8 years ago

@bimawa: I'm a bit confused by the last line of your comment. Were you able to solve your issue?

bimawa commented 8 years ago

@hannahmbanana i think its not problem, but feature request. If i have collection of networkImages and items, will be cool, if resize itself after image loaded ).

appleguy commented 8 years ago

@vitalybaev: Thanks for contributing to the ASDK community by being an early adopter of ASLayout and asking great questions about it! Perhaps you will be an early adopter of our new Layout Transition API?

http://asyncdisplaykit.org/docs/implicit-hierarchy-mgmt.html http://asyncdisplaykit.org/docs/layout-transition-api.html

@bimawa: I don't believe there is a feature request here. If you set the .preferredFrameSize of the image node, Does -setNeedsLayout not work? It should fully notify the table or collectionView that layout needs to be redone, including the size of the cell. Feel free to contact me or @hannah on ASDK's slack if you have any questions.