Skip to content
This repository was archived by the owner on Feb 2, 2023. It is now read-only.

Make textContainerInset configurable for ASEditableTextNode - #802

Merged
appleguy merged 1 commit into
facebookarchive:masterfrom
samhsiung:add_textContainerInset
Nov 1, 2015
Merged

Make textContainerInset configurable for ASEditableTextNode#802
appleguy merged 1 commit into
facebookarchive:masterfrom
samhsiung:add_textContainerInset

Conversation

@samhsiung

Copy link
Copy Markdown
Contributor

Previously, it was only possible to configure the textContainerInset of the typed textView by accessing the textView property on didLoad. This would cause the placeholder and typed textViews to become desynced however, so in this commit we add the ability to configure both.

…er and typed textViews

Previously, it was only possible to configure the textContainerInset of the typed textView by accessing the textView property on didLoad. This would cause the placeholder and typed textViews to become desynced however, so in this commit we add the ability to configure both.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In the future I'd still prefer we be more explicit about this — a comment is fine if it is logically certain that an if (nodeLoaded) check is not needed. As a future reader / author in this code, though, it isn't immediately obvious that these latter two lines are intended to be no-ops and should not trigger the views to be created.

appleguy added a commit that referenced this pull request Nov 1, 2015
Make textContainerInset configurable for ASEditableTextNode
@appleguy
appleguy merged commit a641a6e into facebookarchive:master Nov 1, 2015
@appleguy

appleguy commented Nov 1, 2015

Copy link
Copy Markdown
Contributor

Thanks again for the nice refinement!

aimalygin pushed a commit to aimalygin/AsyncDisplayKit that referenced this pull request Feb 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants