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

Merge adjacent ReactTextComponents in flattenChildren #1989

Closed
wants to merge 2 commits into from
Closed

Merge adjacent ReactTextComponents in flattenChildren #1989

wants to merge 2 commits into from

Conversation

syranide
Copy link
Contributor

@syranide syranide commented Aug 3, 2014

Includes #1988 (so click on the last commit below to view actual changes).

This PR is a super simple isolated implementation for merging adjacent ReactTextComponents just before rendering. So this does not affect this.props.children or cloneWithProps. It's currently not re-entrant, it is really easy to fix simply by storing it in a magic key inside traverseContext instead, but that doesn't seem worth it.

With this and something like #1570 we should be able to get rid of all excess markup.

Massively increased test-coverage for ReactMultiChildText.

@sophiebits
Copy link
Contributor

See #742.

@sebmarkbage
Copy link
Collaborator

I think my main point in #742 was that we didn't want to thrash users until we had a permanent solution (that doesn't use span at all). Instead of having them fix their assumptions of internals twice (for CSS selectors and manual DOM traversal).

@syranide
Copy link
Contributor Author

syranide commented Aug 4, 2014

@sebmarkbage Ah right, that's what you meant by "thrash users". @spicyj's #1871 might make that a reality soon though. :)

@syranide
Copy link
Contributor Author

Rebased

@sebmarkbage
Copy link
Collaborator

Closing out for the benefit of a complete solution that never wraps. That let's us have a simple story: "never wraps in span!" instead of a special case that might prove to be conditional. E.g. you might rely on values being wrapped except when something between them disappears in a conditional.

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

Successfully merging this pull request may close these issues.

4 participants