Skip to content
This repository has been archived by the owner on Dec 1, 2017. It is now read-only.

Potential Mistake ? #3

Open
yrizk opened this issue Jun 13, 2015 · 2 comments
Open

Potential Mistake ? #3

yrizk opened this issue Jun 13, 2015 · 2 comments

Comments

@yrizk
Copy link

yrizk commented Jun 13, 2015

Hey lucas,

First off, thank you for this repo. this is an challenging, but worthwhile endeavor.
I was going through TweetElement.java , when I caught something odd with getMeasuredHeightWithMargins(UIElement uiElement) and getHeightWithMargins(UIElement) . For ease, I have attached the methods below:

 private int getHeightWithMargins(UIElement element) {
        final MarginLayoutParams lp = (MarginLayoutParams) element.getLayoutParams();
        return element.getMeasuredHeight() + lp.topMargin + lp.bottomMargin;
    } 
   private int getMeasuredHeightWithMargins(UIElement element) {
        final MarginLayoutParams lp = (MarginLayoutParams) element.getLayoutParams();
        return element.getMeasuredHeight() + lp.topMargin + lp.bottomMargin;
    }

As you can tell, there is no different between the two. I'm still learning, so I might be wrong, but do you mean to call element.getHeight() in the first method? If so let me know, I'd be happy to submit a PR.

@rayworks
Copy link

rayworks commented Aug 2, 2017

@yrizk
Please note the first method is called in onLayout (). At that time, the MeasuredHeight of UIElement is determined, but not the actual Height.

I agree that the method is kind of duplicated.

@lucasr
Copy link
Owner

lucasr commented Aug 2, 2017

Good points! In case you missed it, I recommend having a look at Litho. It incorporates a lot of these optimizations (and more!) behind a simple declarative API :-)

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

No branches or pull requests

3 participants