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

Add reset() method to CSSNode #140

Merged
merged 1 commit into from
Oct 8, 2015
Merged

Add reset() method to CSSNode #140

merged 1 commit into from
Oct 8, 2015

Conversation

lucasr
Copy link
Contributor

@lucasr lucasr commented Oct 5, 2015

This allows users of css-layout in Java to perform faster resets on
CSSNode in cases when you want to recycle instances.

CSSLayout.resetResult() was renamed to simply reset() for consistency.

@lucasr
Copy link
Contributor Author

lucasr commented Oct 8, 2015

@kmagiera @foghina ping?

public CSSAlign alignItems;
public CSSAlign alignSelf;
public CSSPositionType positionType;
public CSSWrap flexWrap;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? I'd venture default values are there for a reason?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, just saw reset() below, ignore.

@foghina
Copy link
Contributor

foghina commented Oct 8, 2015

See my inlines, otherwise

looks good to me

Not a big fan of the breaking API change, but as long as you fix usages, I'll allow it.

@lucasr
Copy link
Contributor Author

lucasr commented Oct 8, 2015

@foghina What API break?

@foghina
Copy link
Contributor

foghina commented Oct 8, 2015

What API break?

resetResult() -> reset().

@lucasr
Copy link
Contributor Author

lucasr commented Oct 8, 2015

@foghina Ah, I thought CSSLayout was package-private. Reverted this change to avoid the API break.

This allows users of css-layout in Java to perform faster resets on
CSSNode in cases when you want to recycle instances.
@lucasr
Copy link
Contributor Author

lucasr commented Oct 8, 2015

Updated the branch:

  • Reverted resetLayout() rename to avoid API break
  • Don't reset measure function on reset to avoid problems in RN
  • Attachment check before resetting in CSSNode as suggested

lucasr added a commit that referenced this pull request Oct 8, 2015
@lucasr lucasr merged commit 4b4cd06 into facebook:master Oct 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants