Skip to content

Convert 2 examples to CodeSandbox #138

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

Closed
wants to merge 15 commits into from

Conversation

CompuIves
Copy link
Contributor

@CompuIves CompuIves commented Oct 11, 2017

Related to #122.

In this PR I converted 2 different examples over to CodeSandbox:

  • Uncontrolled Components
  • Portals (1 & 2)

Uncontrolled Components
Portals 1
Portals 2

For Portals I decided to split the example up in multiple files, since it's one of the more advanced examples. I can always merge it into 1 file if that's desired.

Regarding page size, @gaearon mentioned a good consideration. CodeSandbox has 2 versions: the embed version and the editor version. The embed version is a stripped down version of the editor and built to be small.

I decided to use the embed version for the examples, but it still downloads 1.8MB before render of editor+preview. This is mainly because we use the Monaco editor. To make the bundle size smaller you can give the option to use CodeMirror instead by appending ?codemirror=1 to the url. Then the size before render will be 900KB and it renders quicker, but you'll lose the vscode editor feeling/features. We'll need to decide on that.

Also, the URLs are currently pointing to my branch, because master doesn't contain the examples yet. Maybe it would be cool to automate this so we can generate custom urls based on branch, that way Netlify demos work without effort.

@reactjs-bot
Copy link

reactjs-bot commented Oct 11, 2017

Deploy preview ready!

Built with commit ac676ec

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

@bvaughn
Copy link
Contributor

bvaughn commented Oct 11, 2017

To make the bundle size smaller you can give the option to use CodeMirror instead by appending ?codemirror=1 to the url. Then the size before render will be 900KB and it renders quicker, but you'll lose the vscode editor feeling/features. We'll need to decide on that.

IMO the smaller payload size is better than the vscode polish.

I might be jumping the gun on the preview but it looks like they're all failing at the moment:
screen shot 2017-10-11 at 1 48 38 pm

@CompuIves
Copy link
Contributor Author

Updated the code to use codemirror! Regarding your error, that's really strange. I can't reproduce it on Chrome, Safari and Firefox with fresh cache. It could be related to sw-cache. What does your console say and which browser are you using?

@CompuIves
Copy link
Contributor Author

CompuIves commented Oct 12, 2017

I added the possibility to link to 1 specific file with no boilerplate. In that case it will check if the file is in a create-react-app project and use those settings. So for example in this scenario:

examples/
  rendering-elements/
    package.json
    public/
    src/
      1.js
      2.js
      3.js

You could use these urls (they don't work because the PR has not been updated, I can do that if you like the idea):

https://codesandbox.io/embed/github/CompuIves/reactjs.org/tree/codesandbox/examples/rendering-elements/src/1.js?codemirror=1
https://codesandbox.io/embed/github/CompuIves/reactjs.org/tree/codesandbox/examples/rendering-elements/src/2.js?codemirror=1
https://codesandbox.io/embed/github/CompuIves/reactjs.org/tree/codesandbox/examples/rendering-elements/src/3.js?codemirror=1

And you would get 3 different projects with the entry file assigned to the linked file.

I also played a bit with the embed API, and created this:

screen shot 2017-10-11 at 22 42 12

(Top is inline, bottom is codesandbox embed)

You can actually specify line highlights in the source using comments (like eslint rules) and they work in embeds, this is really cool since your highlights live in your code (you don't have to sync line numbers) and you don't have to keep the inline code of the doc synced with the git repo.

Curious what you think!

@bvaughn
Copy link
Contributor

bvaughn commented Oct 12, 2017

I added the possibility to link to 1 specific file with no boilerplate. In that case it will check if the file is in a create-react-app project and use those settings.

This looks nice 👍 since it would cut down significantly on the boilerplate for multi-step examples.

You can actually specify line highlights in the source using comments (like eslint rules) and they work in embeds, this is really cool since your highlights live in your code (you don't have to sync line numbers) and you don't have to keep the inline code of the doc synced with the git repo.

That is pretty cool 😁

@bvaughn
Copy link
Contributor

bvaughn commented Oct 12, 2017

Updated the code to use codemirror! Regarding your error, that's really strange. I can't reproduce it on Chrome, Safari and Firefox with fresh cache. It could be related to sw-cache. What does your console say and which browser are you using?

Bizarre! I tried the links in an incognito window though and they do work there, just not in my main browser.

I wonder what about these examples would cause them to be broken the first time I load them in an incognito browser? (And if others might encounter this same issue...)

@CompuIves
Copy link
Contributor Author

Right! I'm also exploring more options. I'm currently working on a Gatsby Remark plugin that allows you to add links to the source files in the markdown, these then get converted to the actual prism code highlights with a button to turn it into a CodeSandbox embed. The advantage of this is that you have a single source of truth for the js files, you're also not dependent on an external service just to show code and you automatically have a synced sandbox for every file.

I wonder what about these examples would cause them to be broken the first time I load them in an incognito browser? (And if others might encounter this same issue...)

Most probably it's because of a cached packager response from a couple months ago. Next week we're moving over to the new packager with a max cache time of 24h, so hopefully that will be resolved by then.

@bvaughn
Copy link
Contributor

bvaughn commented Oct 12, 2017

I'm currently working on a Gatsby Remark plugin that allows you to add links to the source files...

Very cool! I had been considering something along those lines as well but haven't gotten around to it yet. Glad to hear you're working on it 😁

Most probably it's because of a cached packager response from a couple months ago...

Okay. Good to hear. Service-worker problems like this are the worst 😅

@CompuIves
Copy link
Contributor Author

CompuIves commented Oct 13, 2017

Okies! I tried something new.

With this version I added a remark plugin that converts:

[Try it on CodeSandbox.](source:examples/uncontrolled-components/src/index.js{11,20})

to the Prism Highlight of that file with line highlights and a generated link to the codesandbox sandbox embed based on the branch/user/file. You can see it here: https://deploy-preview-138--reactjs.netlify.com/docs/uncontrolled-components.html.

The next thing I'm going to try is putting a React Live component in the place of the code, this can be a challenge since we may need 2 plugins in that case. One for converting the markdown and then one for hooking in the components. There are also two considerations:

  1. We lose the line highlighting. I think this is pretty big, since it's valuable to show what we mean in many cases.
  2. We should find a way to allow the import statements to show. I think these statements can be pretty important, as it allows users to directly copy the code locally and it shows the user the recommended way of using React.

Maybe we can open a PR to react-live to add these 2 points.

A second option would be to replace the highlighted code with an embed as soon as you click on edit. This way we keep the line highlights and module imports. This keeps the user on the same page but still allows the user to fiddle. Big downside is that the user first needs to press a button. I think the React Live version would first be better to try out.

Another cool tryout would be the highlight like eslint comments support.

Okay, I'll get back to it this evening!

@EricSimons
Copy link
Contributor

@CompuIves this is awesome! Were you planning on taking a stab at turning the "Try it on X" into a dropdown menu? Or should I make a PR to this for it?

Also, reg react-live not having the import statements — I actually think it's better for the docs to not have them there, as it's just redundant code that isn't critical to explaining the core concepts at hand. What if we did the opposite approach, where we strip out any import statements before putting them in the react-live editor? That way when the user clicks on the "try it on x" dropdown menu, the import statements will be there in the "real" environment on CSB/SB/etc (which makes more sense from the learners perspective)

@CompuIves
Copy link
Contributor Author

CompuIves commented Oct 15, 2017

Pushed a new experimentation. You can now define an example like this:

[Run the example.](source:examples/uncontrolled-components/src/index.js{11,20}?editorsize=70)

The options after the ? are options like how big (in %) you'd like the editor to be.

That line will first show the code as a Prism.js with line highlights. When you press 'Run example' the Prism.js highlighted embed will turn into a CodeSandbox embed with the same lines highlighted. The 'Run example' will turn into an 'Open in CodeSandbox', I'm thinking of turning it into a 'Show original code' or something, so the user can look back on the original code.

It looks like this:

kapture 2017-10-14 at 23 57 17

@EricSimons
Copy link
Contributor

@CompuIves this is cool! Have you started working on the react-live version you mentioned in #122 (comment)? I'd love to help collab on it! 🙌

@CompuIves
Copy link
Contributor Author

Heyo!

@CompuIves this is cool! Have you started working on the react-live version you mentioned in #122 (comment)? I'd love to help collab on it! 🙌

Didn't start it, mostly because I'm not sure if it's smart to embed live preview on every doc page for bundle size and react-live doesn't have line highlights yet.


Okay! I've converted a different page to give a more 'real-life' example of how it could look like. This page: https://deploy-preview-138--reactjs.netlify.com/docs/state-and-lifecycle.html.

All CodePen examples are now linked to a single file in the repo, these files are automatically synced with both the documentation and CodeSandbox. The last example is interesting, because it's referring a single file from a bigger project. If you reference index.js directly, it will use that directory as src folder. This means that you can show a single file of a bigger project, with the existing imports. This fits the last example more, because you want to show the 3 Clocks instead of a whole file.

I also reduced the embed size.

@EricSimons
Copy link
Contributor

EricSimons commented Oct 17, 2017

@bvaughn proposed the solution for the react-live/babel bundle size issue in #122 (comment):

I like the idea of deferring cost until a user chooses to interact with an example (eg by clicking an "try it" link).

^ that's actually the same solution you're using in this PR, right? So we'd just need to swap out the CSB iframe with react-live

react-live doesn't have line highlights yet.

Yes, hence why I asked if you wanted to collab on upgrading it :)

@EricSimons
Copy link
Contributor

EricSimons commented Oct 17, 2017

@CompuIves just checked out your counters example and all of the embeds seem to be broken on my machine; not sure if related to the service worker issue @bvaughn hit earlier in this thread but here's a GIF of it:

screenflow

^ lmk if there's anything I can do to help debug on my end

@CompuIves
Copy link
Contributor Author

CompuIves commented Oct 17, 2017

I added a force refresh for the first 3 examples, because the setIntervals were not cleaned up.

^ that's actually the same solution you're using in this PR, right? So we'd just need to swap out the CSB iframe with react-live

Hmm, yep that could work!

not sure if related to the service worker issue @bvaughn hit earlier

Yap it is 😄 . I finally know exactly what it is! So our packager generates 2 different files: dll.js and manifest.json to map the modules to the code. We recently refreshed the cache, which in itself is no problem. But some users only cached the manifest.json and not the dll.js, because dll.js is bigger. Because of this the wrong mapping was assumed and you get the issue you see there.

I'm deploying the first part of the fix now which should fix it for everyone. The next part will be deployed this evening, which will fix it for future cache refreshes. We're going full on with the new packager this week anyway, which doesn't have this issue at all.

@EricSimons
Copy link
Contributor

Sweet! Okay perfect, sounds like we're on the same page reg react-live — wanna spin up a branch where we could collab on it? Perhaps it would make sense to fork off of this one, since we'll likely want to reuse parts of your Gatsby plugin?

@CompuIves
Copy link
Contributor Author

CompuIves commented Oct 17, 2017

I first want to know what @bvaughn thinks of this solution. I've already put a lot of time in making this solution perfect by building the remark plugin, adding editorsize, new hidenavigation and forcerefresh functionality, adding line highlights, providing IE support, upgrading our git-extractor to handle direct file urls/subprojects, and improving our embed size to be much smaller.

I've also already explained my thoughts on why this could be eg. a good gateway (#122 (comment)), so I'm still in favor of this solution.

@EricSimons
Copy link
Contributor

Ah gotchya — I just asked @bvaughn in #122 (comment) about this + other remaining q's, so I'm hoping his response there will help clear things up and enable us all to work collaboratively 👍

I've already put a lot of time in making this solution perfect

I think we've all put a great deal of time into creating our PR's, debating their merits, and helping figure out the right solution for the React team's needs :)

@bvaughn bvaughn self-requested a review October 21, 2017 19:08
@bvaughn
Copy link
Contributor

bvaughn commented Oct 21, 2017

Hm. Fresh install (rm -rf .cache && rm -rf node_modules && yarn install && yarn dev) fails with:
screen shot 2017-10-21 at 1 08 47 pm

Edit Looks like it failed because my local Git repo was detached (b'c I checked out CompuIves/codesandbox).

Copy link
Contributor

@bvaughn bvaughn left a comment

Choose a reason for hiding this comment

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

This is really cool stuff 👍

I am concerned about the complexity it adds to both the build and live/runtime environments though.

I want to think on it a little.

@@ -46,7 +46,7 @@ A typical use case for portals is when a parent component has an `overflow: hidd
>
> It is important to remember, when working with portals, you'll need to make sure to follow the proper accessibility guidelines.

[Try it on CodePen.](https://codepen.io/gaearon/pen/yzMaBd)
[Try it on CodeSandbox.](https://codesandbox.io/embed/github/CompuIves/reactjs.org/tree/codesandbox/examples/portals-1?codemirror=1)
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be nice to bundle this up as a custom remark transform. That way we could keep the query params and stuff in one place, and just write Markdown like:

[Try it on CodeSandbox](codesandbox=examples/portals-1)

Edit Looks like this would be similar to the "Run Example" markdown below 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

I mentioned this because I was wondering if we could, for example, auto-expand the left side menu on multi-file examples b'c it's not obvious there is more than one file when you open a new sandbox- and it would be nice to not have to find-and-replace a bunch of links to do this.

Copy link
Contributor

Choose a reason for hiding this comment

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

This could also have the same issue I mentioned below though, of being difficult to test initially (at least for a reviewer).

<button>Click</button>
</div>
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The portals examples still have a lot of boilerplate. I see that this adds more flexibility for customization, but I suspect it would also end up being super redundant in the case of most of our examples. 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, we can move this already to a version that has less boilerplate (same how state-and-lifecycle-7 works).

```

[Try it on CodePen.](http://codepen.io/gaearon/pen/gwoJZk?editors=0010)
[Run example.](source:examples/single-file-examples/src/state-and-lifecycle/state-and-lifecycle-1.js{11}?editorsize=70&forcerefresh=1)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm. Just tried this one locally and it failed:

Failed to load resource: the server responded with a status of 422 ()
GET https://codesandbox.io/api/v1/sandboxes/github/reactjs/reactjs.org/tree/codesandbox/examples/single-file-examples/src/state-and-lifecycle/state-and-lifecycle-1.js

I think that's because the code it's trying to open only exists on your fork of the repo.

This default behavior would make things hard to test for people adding new runnable examples though?

css={[sharedStyles.markdown]}
dangerouslySetInnerHTML={{__html: markdownRemark.html}}
/>
codeSandboxLinks.forEach(link => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could do this with a custom remark plugin and remove the runtime document querying for better perf.

css={[sharedStyles.markdown]}
dangerouslySetInnerHTML={{__html: markdownRemark.html}}
/>
codeSandboxLinks.forEach(link => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This breaks IE. ☹️

querySelectorAll returns a NodeList rather than an Array, and the NodeList doesn't have a forEach method in IE.

Changing to a for-in statement fixes it, or...a custom remark plugin would be even better.

@CompuIves
Copy link
Contributor Author

Really good feedback, thanks! I'll do some thinking on this tonight 😄

@bvaughn
Copy link
Contributor

bvaughn commented Oct 21, 2017

Happy to chat about this on Twitter or Slack if you'd like to run ideas by someone Ives.

@bvaughn bvaughn mentioned this pull request Oct 25, 2017
@bvaughn
Copy link
Contributor

bvaughn commented Nov 6, 2017

Closing this PR for now in favor of a more incremental solutions (#245). Let's keep the conversation going here though, either on GitHub or on Twitter directly. I think the ideas shown here are exciting.

@bvaughn bvaughn closed this Nov 6, 2017
jhonmike pushed a commit to jhonmike/reactjs.org that referenced this pull request Jul 1, 2020
* translate how-to-contribute page

* fix charset

* fix typo

* Update content/docs/how-to-contribute.md

Co-Authored-By: lhcgoncalves <luiz.goncalves@ufpr.br>

* Update content/docs/how-to-contribute.md

Co-Authored-By: lhcgoncalves <luiz.goncalves@ufpr.br>

* Update content/docs/how-to-contribute.md

Co-Authored-By: lhcgoncalves <luiz.goncalves@ufpr.br>

* Update content/docs/how-to-contribute.md

Co-Authored-By: lhcgoncalves <luiz.goncalves@ufpr.br>

* Update content/docs/how-to-contribute.md

Co-Authored-By: lhcgoncalves <luiz.goncalves@ufpr.br>

* Update content/docs/how-to-contribute.md

Co-Authored-By: lhcgoncalves <luiz.goncalves@ufpr.br>

* Update content/docs/how-to-contribute.md

Co-Authored-By: lhcgoncalves <luiz.goncalves@ufpr.br>

* Update content/docs/how-to-contribute.md

Co-Authored-By: lhcgoncalves <luiz.goncalves@ufpr.br>

* Update content/docs/how-to-contribute.md

Co-Authored-By: lhcgoncalves <luiz.goncalves@ufpr.br>

* Update content/docs/how-to-contribute.md

Co-Authored-By: lhcgoncalves <luiz.goncalves@ufpr.br>

* change to male speech
BetterZxx pushed a commit to BetterZxx/react.dev that referenced this pull request Mar 21, 2023
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.

5 participants