Added error boundaries to CodeEditor and layout component #96
Conversation
|
Deploy preview ready! Built with commit e2420ff |
There was a problem hiding this comment.
This is a nice start. 😄
I think we need some kind of error UI. Otherwise errors just result in an empty (or mostly empty) page and this is confusing to users and arguably not much better than not handling the error at all.
cc @joecritch in case he has any UI ideas.
| render() { | ||
| console.log(this.state); | ||
| if (this.state.error) { | ||
| alert('Error, try to whitelist the site in AdBlocker/Cookie Whitelist'); |
There was a problem hiding this comment.
I don't think this is the right way to warn people about this. I think a console.error log would be reasonable though. Maybe something like:
If you're using an ad-blocker or have disabled cookies,
Consider adding this website to the whitelist.
We should also keep in mind that not all errors are caused by this so our wording shouldn't be overly assertive that this is definitely the cause/fix. Users might see an error even if they aren't using adblockers and haven't disabled cookies, and that would be confusing.
| @@ -17,6 +17,7 @@ import Remarkable from 'remarkable'; | |||
| import {LiveProvider, LiveEditor} from '@gaearon/react-live'; | |||
| import {colors, media} from 'theme'; | |||
| import MetaTitle from 'templates/components/MetaTitle'; | |||
| import ErrorBoundary from '../ErrorBoundary'; | |||
There was a problem hiding this comment.
nit
import ErrorBoundary from 'components/ErrorBoundary';| @@ -25,6 +25,7 @@ import '../prism-styles'; | |||
| import 'glamor/reset'; | |||
| import 'css/reset.css'; | |||
| import 'css/algolia.css'; | |||
| import ErrorBoundary from '../components/ErrorBoundary'; | |||
There was a problem hiding this comment.
nit
import ErrorBoundary from 'components/ErrorBoundary';| } | ||
|
|
||
| render() { | ||
| console.log(this.state); |
| @@ -82,4 +83,4 @@ class Template extends Component { | |||
| } | |||
| } | |||
|
|
|||
| export default Template; | |||
| export default ErrorBoundary(Template); | |||
There was a problem hiding this comment.
Errors in the template will actually result in a totally empty/white page. This is probably not great. We should show something as a fallback in the browser (in addition to logging error info to the console).
There was a problem hiding this comment.
What do you suggest as the fallback?
Maybe we can use a modal (using react-modal) here.
There was a problem hiding this comment.
We probably need to show a basic error page. It doesn't have to look super pretty. @joecritch or I can pretty it up.
|
It's been a while since there was any movement on this PR, and the code it modifies has been changed by subsequent PRs. I'm going to close this for now- but I'm happy to re-review if the PR is updated. Please just ping me by name if there are more changes pushed! |
* Translating * Finishing traslationof lifting state up * Apply suggestions from code review Translating codepen links Co-Authored-By: gustavobini <[email protected]> * Apply suggestions from code review Fixing grammar errors Co-Authored-By: gustavobini <[email protected]> * Making a few translation corrections * Delete package-lock.json
…eactjs#96) * docs(cn): translate content/docs/reference-react-dom.md into Chinese * change 您 to 你 * modify * modify * Update reference-react-dom.md * Update reference-react-dom.md * Update reference-react-dom.md
Fix issue #3 , facebook/react#11008 by adding error boundaries to CodeEditor and layout components. Nudging (using alert()) users to whitelist the site in AdBlocker in the alternative UI.