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

[Fiber] Normalize className passed to dom from fiber #8247

Closed
wants to merge 1 commit into from

Conversation

sdougbrown
Copy link

Nobody wants to see class="null". :)

Apologies for this being so small, but I wanted to confirm that this is the approach that you guys wanted to take wrt to helpers - I was going to start working on reconciling styles, but that's a hairy beast and I don't want to make a mess of things without verifying that component-level reconciliation is intended to be inlined into this file.

Is the DOMRenderer supposed to get huge as more features are built, or should each chunk of logic live in another module?

Thanks for your time! 😄

Nobody wants to see `class="null"`. :)
@facebook-github-bot
Copy link

Thank you for your pull request. We require contributors to sign our Contributor License Agreement, and yours has expired.

Before we can review or merge your code, we need you to email [email protected] with your details so we can update your status.

@gaearon
Copy link
Collaborator

gaearon commented Nov 9, 2016

Thanks for the PR! I think for now we don’t plan to take changes related to DOM attributes or events because @sebmarkbage is already working on porting the existing code to implement those.

@sdougbrown
Copy link
Author

Yeah the more I looked at it, it seemed like that would probably end up being the correct approach.

I was working on an assumption from your comment in #7925 that everything was open season. I probably misread your intent when you spoke to implementing attributes making things easier.

Thanks for responding quickly - perhaps I'll pick on something a bit more abstract instead. 😄

@sdougbrown sdougbrown closed this Nov 9, 2016
@sdougbrown sdougbrown deleted the db/fiber-className branch November 9, 2016 18:22
@gaearon
Copy link
Collaborator

gaearon commented Nov 10, 2016

Sorry I should've been more clear. The OP post contains all items that somebody is already working on: #7925 (comment). If there's a name in front of a todo, it's taken.

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.

3 participants