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

Return <DOCTYPE!> back and little refactoring of docs/build.js #634

Merged
merged 2 commits into from
May 12, 2015

Conversation

AlexKVal
Copy link
Member

@AlexKVal AlexKVal commented May 9, 2015

Return <DOCTYPE!> to pages.

First time it was introduced here
https://github.com/react-bootstrap/react-bootstrap/blob/120ba6d38a4/Gruntfile.js#L204

Then it was moved here
addc396#diff-31cfa4797a188b7ab32b049b80c82ea6R63

And finally it was lost Root => React easy to overlook
from var RootHTML = Root.renderToString()
to var RootHTML = React.renderToString()
here
804c24a#diff-4cd8a3a29faaf037ff2ec547bf1635abL10
and here
804c24a#diff-ce9d98be04f7a23d325c274fc1a0a089L21

After that Root.renderToString() method (and some others) became unused.

And I've moved out html generating code for all docs - pages into standalone function
for clarity.
Those spagetty drives me crazy.

Less complexity promotes easier entrance for new contributors.

@AlexKVal
Copy link
Member Author

AlexKVal commented May 9, 2015

There are two merge conflicts with Enable Hot-Loader PR.
I'll fix them when #631 is merged.

return new Promise((resolve, reject) => {
Router.run(routes, '/' + fileName, Handler => {
let html = React.renderToString(React.createElement(Handler));
html = '<!doctype html>' + html;
Copy link
Member

Choose a reason for hiding this comment

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

should that be DOCTYPE in caps?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was originally in lowercase

return '<!doctype html>';
, so I just return it back 😃
And I'm looking at source tab in browser, all html code is lowercased.
The same style 😄

Copy link
Member

Choose a reason for hiding this comment

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

@AlexKVal
Copy link
Member Author

Rewritten for merging.

@mtscout6
Copy link
Member

LGTM

mtscout6 added a commit that referenced this pull request May 12, 2015
Return <DOCTYPE!> back and little refactoring of docs/build.js
@mtscout6 mtscout6 merged commit 32ca419 into react-bootstrap:master May 12, 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.

None yet

3 participants