add non-jsx view to interactive code samples #141

criticalbh · 2017-10-11T22:56:00Z

Here is implementation of code interaction view:

and once clicked on VIEW COMPILED JSX we get this:

facebook-github-bot · 2017-10-11T22:56:06Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

facebook-github-bot · 2017-10-11T22:58:38Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

reactjs-bot · 2017-10-11T22:58:41Z

Deploy preview ready!

Built with commit 8a0eda3

https://deploy-preview-141--reactjs.netlify.com

bvaughn

Left some feedback.

Here's a rough prototype I made (demonstrating the feedback I left in the PR) if you're interested:

https://gist.github.com/bvaughn/c65d556693fed3dbc6ab2025efac2c2b

bvaughn · 2017-10-12T16:13:08Z

src/components/CodeEditor/CodeEditor.js

@@ -11,13 +11,19 @@ import ReactDOM from 'react-dom';
 import Remarkable from 'remarkable';
 // TODO: switch back to the upstream version of react-live
 // once https://github.com/FormidableLabs/react-live/issues/37 is fixed.
-import {LiveProvider, LiveEditor} from '@gaearon/react-live';
+import {LiveEditor, LiveProvider} from '@gaearon/react-live';


😆 Love the ninja alpha-sort

bvaughn · 2017-10-12T16:17:02Z

src/components/CodeEditor/CodeEditor.js


 const compile = code =>
  Babel.transform(code, {presets: ['es2015', 'react']}).code; // eslint-disable-line no-undef

+const compileJsxToJS = code => Babel.transform(code, {presets: ['react']}).code; // eslint-disable-line no-undef


How do you feel about these names for improved clarity?

// eslint-disable-next-line no-undef const compileES5 = code => Babel.transform(code, {presets: ['es2015', 'react']}).code; // eslint-disable-next-line no-undef const compileES6 = code => Babel.transform(code, {presets: ['react']}).code;

bvaughn · 2017-10-12T16:26:40Z

src/components/CodeEditor/CodeEditor.js

+                    spellCheck="false"
+                    dangerouslySetInnerHTML={{__html: prism(compiledJsx)}}
+                  />
+                )}


Let's re-use the LiveProvider+LiveEditor to show the non-JSX code too. This way we don't have to use Prism directly. So something like:

<LiveProvider code={showJSX ? code : compiledES6} mountStylesheet={false}>

In this case, compiledES6 would be editable

Ah, good point. I guess this wouldn't quite work as proposed. Maybe we should make the ES6 preview not editable?

bvaughn · 2017-10-12T17:23:40Z

src/components/CodeEditor/CodeEditor.js

+                      Back to JSX Editor
+                    </a>
+                  )}
+                </MetaTitle>


How do you feel about using a checkbox to toggle JSX mode instead of a button? I think it might look better 😄

criticalbh · 2017-10-12T17:30:08Z

@bvaughn Thank you very much, I'll jump on it.

criticalbh · 2017-10-12T21:04:07Z

@bvaughn I have made both ES5 and ES6 editable.

There might be small problem when coming back to JSX view since what we have changed in JS wont be applied to JSX.

If this scenario is not acceptable than we can make JSX compiled view to be preview only.

bvaughn

There might be small problem when coming back to JSX view since what we have changed in JS wont be applied to JSX.

If this scenario is not acceptable than we can make JSX compiled view to be preview only.

That's a good point!

I think it's probably okay. I don't really think that's a common use case or important enough to warrant added complexity.

bvaughn · 2017-10-16T18:35:25Z

src/components/CodeEditor/CodeEditor.js

@@ -282,7 +312,7 @@ class CodeEditor extends Component {
  }

  _onChange = code => {
-    this.setState(this._updateState(code));
+    this.setState(this._updateState(code, this.state.showJSX));


We should change this to:

this.setState(state => this._updateState(code, state.showJSX));

The benefit of using a callback for setState is that it avoids references potentially stale values in this.state. This didn't matter before because we weren't references this.state but now that we are....we should use the callback form. 😄

bvaughn · 2017-10-16T18:37:47Z

src/components/CodeEditor/CodeEditor.js

-      return {
-        compiled: compile(code),
+      let newState = {
+        compiled: compileES5(code),


Should code always go into newState? What's the benefit of only adding it conditionally.

What I thought here is that we want to keep code only if we are in JSX view.

If we mix code from ES5 view with ES6 it wont work any more.

Ah, I see what you mean.

bvaughn

The 1-way edit flow for JSX-to-non-JSX bugs me a bit but I think it's probably not an actual problem and I can't think of a reasonable solution. 😄

Thanks for all of your work on this!

bvaughn · 2017-10-16T21:33:55Z

src/components/CodeEditor/CodeEditor.js

-      return {
-        compiled: compile(code),
+      let newState = {
+        compiled: compileES5(code),


Ah, I see what you mean.

criticalbh · 2017-10-17T05:35:01Z

Yes indeed, it is cost for ES5 being editable. Thank you very much for guidance it was awesome.

criticalbh · 2017-10-17T18:49:52Z

Hello @bvaughn, what are the reasons of this PR being overwritten with 2b44cd3 ?

bvaughn · 2017-10-17T18:51:48Z

It wasn't overridden? I just tweaked the wording slightly when testing the changes to the home template. (Without those changes, a blocked Babel script just caused the home page to show "Loading examples" forever.)

criticalbh · 2017-10-17T18:54:57Z

Yes I saw the reason, also sometimes after the page is started and you scroll down, new refresh happens with editors being in the place.

But regarding is it overridden, it is. Here is raw file on master branch for CodeEditor.js https://raw.githubusercontent.com/reactjs/reactjs.org/master/src/components/CodeEditor/CodeEditor.js and there are no ES5 transpilers.

bvaughn · 2017-10-17T18:57:05Z

Oh whoa! I see what you mean! Looks like something went wrong with the merge. Thank you for pointing that out! I'll fix.

criticalbh · 2017-10-17T18:57:56Z

@bvaughn your welcome, no problems.

bvaughn · 2017-10-17T18:58:51Z

😅 too much context-switching this morning.

Very sorry this happened. I'll get it fixed right away.

criticalbh · 2017-10-17T19:02:14Z

hh I believe, no problems at all, it is good it is noticed on time.

Sync with reactjs.org @ 8803c63

…js#141) * finished translating para 1 and 2 * finished translating para 3 and 4 * altered wording * use Chinese character of question mark * use Chinese character of full stop * Update content/docs/faq-internals.md Co-Authored-By: ChiuMungZitAlexander <zhaomengzhe@qq.com> * Update content/docs/faq-internals.md Co-Authored-By: ChiuMungZitAlexander <zhaomengzhe@qq.com> * Update content/docs/faq-internals.md Co-Authored-By: ChiuMungZitAlexander <zhaomengzhe@qq.com> * Update content/docs/faq-internals.md Co-Authored-By: ChiuMungZitAlexander <zhaomengzhe@qq.com> * Update faq-internals.md * Update content/docs/faq-internals.md Co-Authored-By: ChiuMungZitAlexander <zhaomengzhe@qq.com> * Update content/docs/faq-internals.md Co-Authored-By: ChiuMungZitAlexander <zhaomengzhe@qq.com> * Update content/docs/faq-internals.md Co-Authored-By: ChiuMungZitAlexander <zhaomengzhe@qq.com> * Update content/docs/faq-internals.md Co-Authored-By: ChiuMungZitAlexander <zhaomengzhe@qq.com>

add non-jsx view to to interactive code samples

527f503

facebook-github-bot added the CLA Signed label Oct 11, 2017

99 fix prettier, lint and flow

6bd11c1

criticalbh changed the title ~~add non-jsx view to to interactive code samples~~ add non-jsx view to interactive code samples Oct 11, 2017

bvaughn requested changes Oct 12, 2017

View reviewed changes

99 replace button with checkbox and apply proposed changes

9876647

bvaughn reviewed Oct 16, 2017

View reviewed changes

use callback for setState

8a0eda3

bvaughn approved these changes Oct 16, 2017

View reviewed changes

bvaughn merged commit 361e27c into reactjs:master Oct 16, 2017

bvaughn mentioned this pull request Oct 16, 2017

Interactive code samples should support a non-JSX view #99

Closed

jhonmike pushed a commit to jhonmike/reactjs.org that referenced this pull request Jul 1, 2020

Merge pull request reactjs#141 from reactjs/sync-8803c637

4efc4b7

Sync with reactjs.org @ 8803c63

add non-jsx view to interactive code samples #141

add non-jsx view to interactive code samples #141

Uh oh!

Conversation

criticalbh commented Oct 11, 2017

Uh oh!

facebook-github-bot commented Oct 11, 2017

Uh oh!

facebook-github-bot commented Oct 11, 2017

Uh oh!

reactjs-bot commented Oct 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bvaughn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

criticalbh commented Oct 12, 2017

Uh oh!

criticalbh commented Oct 12, 2017

Uh oh!

bvaughn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

criticalbh Oct 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bvaughn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

criticalbh commented Oct 17, 2017

Uh oh!

criticalbh commented Oct 17, 2017

Uh oh!

bvaughn commented Oct 17, 2017

Uh oh!

criticalbh commented Oct 17, 2017

Uh oh!

bvaughn commented Oct 17, 2017

Uh oh!

criticalbh commented Oct 17, 2017

Uh oh!

bvaughn commented Oct 17, 2017

Uh oh!

criticalbh commented Oct 17, 2017

Uh oh!

Uh oh!

reactjs-bot commented Oct 11, 2017 •

edited

Loading

criticalbh Oct 16, 2017 •

edited

Loading