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

Move 'Handling Events' example from Codepen to CodeSandbox #2378

Closed
wants to merge 3 commits into from
Closed

Move 'Handling Events' example from Codepen to CodeSandbox #2378

wants to merge 3 commits into from

Conversation

washingtonsoares
Copy link

@washingtonsoares washingtonsoares commented Oct 1, 2019

The main goal of this PR is to move the 'Handling Events' examples page currently in Codepen to Codesandbox! 🎉

Preview changes: https://deploy-preview-2378--reactjs.netlify.com/docs/handling-events.html
image

As can be seen from this PR, there is only one JS file that is imported into handling-events.md and is the same file that will be sent to CodeSandbox. So far so good, and the idea sounds great!

However there is a problem: Comments that are used to highlight/hide lines are also sent to CodeSandbox as shown below.:
imageHighlight/hideline on CodeSandbox.

That seems a little strange to me. "Messing" the code in the examples doesn't look cool.
In that case, would it be interesting for me to adapt the gatsby-remark-code-repls plugin (or some other plugin) to remove comments that highlight/hide the lines before sending to CodeSandbox?

Despite what I did working, there is another approach. Like @cyan33 did in #913, there is a specific file for the markdown, and other specific to be sent to CodeSandbox.
This approach does not have the problem of "messing up" CodeSandbox examples with highlight/hideline comments, however you will need to maintain two files.

This PR brings the approach I found most interesting and would like to hear some opinions on what makes the most sense.

cc @cyan33 @bvaughn @gaearon

@facebook-github-bot
Copy link
Collaborator

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 [email protected]. Thanks!

@reactjs-bot
Copy link

reactjs-bot commented Oct 1, 2019

Deploy preview for reactjs ready!

Built with commit 9e147c2

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

@facebook-github-bot
Copy link
Collaborator

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

@orta
Copy link
Contributor

orta commented Oct 15, 2019

On the tradeoff for the links, comparing the eventual urls from code sandbox in comparison to the links you have in the code:

codesandbox://handling-events/toggle-example,handling-events/toggle-example.css

vs

https://codesandbox.io/api/v1/sandboxes/define?parameters=N4IgZglgNgpgziAXKADgQwMYGs0HMYB0AVnAPYB2SoGFALjObVSACYwoNvkYTzMBOMTE0QgoaenCYAaEIOEBaFqQC2SMRPhMAvrJVoIlUQAs05FlEO4FMAG4NacAPS1SuXLBsAPNCpSxiBG1dEEM2LwJjWhUoZhpGB3UAHhYIWwACCBYAXgAdOVJSWnyAPiSnVNsSkBDTc0tyazsHZ1d3TxgfPwCSOLpE0ScAKnTjLJgFfjN8YABGBQAWbXShp1zyJydRiFxjS13aSemYOYAGaVnZi4WFWYAOaQAmR-11iD9Sflp0gCUhDG-YH4qnSAHJ5ADQQBuN4fL6_f60AAiAHkALLpIEg8GIpSqaGwlCfb6ggguNweCadXz-QgYOBwAnkdYYcQM9IAFQpsHSnXo5jgCOEBAAwqoieQHOlgOt0ul4lJ-ABXAGfAAUKGBKDgAEppbK5ek4EqOPwNVrdTDyIb0rQxnACFJNOlstKIHAue0YCjyIhbcqYNorQa5ZtOfb0gAjMJWTKCyUYeBwND8ACettI6X0WBg6QABnb3Xn0gB3T5YTLWu25jBoKBQSOYLAh232yJmCwwEWWbAu1vu9v1Ls9rAEaPmNWFy0G17M62jDuwbsQbBqvUy-dyqeOmC0ADKtE0aqd9BdJXSao3Nrl7s9lJ9foAhCfCLfud7yNIW9odTqrXLZwNQRzBgM11xbQRaCVfhrTVFs5SSSMlVoVxrQoZdsGyYBtzqTsMKwbQSngw1sLbF8CDfL0fXSAB-MEUQAOVBdI_VBFEADF2NBWdr3ScokJQigiM3dI_xndZAPIP5hFRNECGAtgzQNJI7x5JwSi_edlAwJUVAcAh8FoABRWA9MYAAhVMAEkWDVcFCloUEdXWMTKFqRcrBsexGFad9vBpAJ6QQZAQHifkRBASNSBYdMr0NdAWFSRo_QAVhQLwJJqYIgA

It would seem feasible that whatever pre-processor you use could remove the range and highlight statements before creating the URLs?

@alexkrolick alexkrolick requested a review from gaearon October 15, 2019 19:16
@CompuIves
Copy link
Contributor

CompuIves commented Oct 16, 2019

A sidenote: we have support for highlighting lines in the embed, but only when enabling the codemirror editor. An example here: https://codesandbox.io/embed/new?codemirror=1&highlights=8,9,10,11. We could implement this as well for the default editor (Monaco) if that would be interesting.

Also if there's anything else needed that we can implement, I'm open to suggestions! We're working on the embed right now, so we can include it in the specs.

@washingtonsoares
Copy link
Author

On the tradeoff for the links, comparing the eventual urls from code sandbox in comparison to the links you have in the code:

codesandbox://handling-events/toggle-example,handling-events/toggle-example.css

vs

https://codesandbox.io/api/v1/sandboxes/define?parameters=N4IgZglgNgpgziAXKADgQwMYGs0HMYB0AVnAPYB2SoGFALjObVSACYwoNvkYTzMBOMTE0QgoaenCYAaEIOEBaFqQC2SMRPhMAvrJVoIlUQAs05FlEO4FMAG4NacAPS1SuXLBsAPNCpSxiBG1dEEM2LwJjWhUoZhpGB3UAHhYIWwACCBYAXgAdOVJSWnyAPiSnVNsSkBDTc0tyazsHZ1d3TxgfPwCSOLpE0ScAKnTjLJgFfjN8YABGBQAWbXShp1zyJydRiFxjS13aSemYOYAGaVnZi4WFWYAOaQAmR-11iD9Sflp0gCUhDG-YH4qnSAHJ5ADQQBuN4fL6_f60AAiAHkALLpIEg8GIpSqaGwlCfb6ggguNweCadXz-QgYOBwAnkdYYcQM9IAFQpsHSnXo5jgCOEBAAwqoieQHOlgOt0ul4lJ-ABXAGfAAUKGBKDgAEppbK5ek4EqOPwNVrdTDyIb0rQxnACFJNOlstKIHAue0YCjyIhbcqYNorQa5ZtOfb0gAjMJWTKCyUYeBwND8ACettI6X0WBg6QABnb3Xn0gB3T5YTLWu25jBoKBQSOYLAh232yJmCwwEWWbAu1vu9v1Ls9rAEaPmNWFy0G17M62jDuwbsQbBqvUy-dyqeOmC0ADKtE0aqd9BdJXSao3Nrl7s9lJ9foAhCfCLfud7yNIW9odTqrXLZwNQRzBgM11xbQRaCVfhrTVFs5SSSMlVoVxrQoZdsGyYBtzqTsMKwbQSngw1sLbF8CDfL0fXSAB-MEUQAOVBdI_VBFEADF2NBWdr3ScokJQigiM3dI_xndZAPIP5hFRNECGAtgzQNJI7x5JwSi_edlAwJUVAcAh8FoABRWA9MYAAhVMAEkWDVcFCloUEdXWMTKFqRcrBsexGFad9vBpAJ6QQZAQHifkRBASNSBYdMr0NdAWFSRo_QAVhQLwJJqYIgA

It would seem feasible that whatever pre-processor you use could remove the range and highlight statements before creating the URLs?

I think so. But i don't know how to do that yet.

@bvaughn do you have any ideia ?

@facebook-github-bot facebook-github-bot deleted the branch reactjs:master July 6, 2021 18:08
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