-
Notifications
You must be signed in to change notification settings - Fork 2
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
Display errors to user, don't just console.error them #54
Comments
@justbarreto and I are going to pair on this one! |
Yeah! That's what I had in mind. 👍 |
Do you have Screenhero, @justbarreto? I can hook you up with an invitation if not. |
I do believe I have screenhero. Haven't logged on in a while. @cheshire137 PM'ed you the email. |
omg that screenshot looks amazing. i've been checking my email for 'updates', but havent seen anything. i'm glad i checked here! :) |
Noooo Rob, don't believe it! That's a sample from another site, not our app. |
Damn, rookie mistake |
I'm okay with pushing this to version 2 if you guys want. Up to you. :) |
In
overwatch-team-comps/app/assets/javascripts/components/composition-form.jsx
Line 7 in c4cf612
ErrorMessage
component is the way to go. Here's how I imagine the flow working:console.error
'ing, they should alsothis.setState({ error: 'description of the problem' })
.render
forComponentForm
, we render<ErrorMessage error={this.state.error} />
.ErrorMessage
component is smart enough to rendernull
when theerror
it is given is undefined/blank/null. Otherwise, it renders a<p className="text-attack">{this.props.error}</p>
.<p>
tag ofErrorMessage
that,onClick
, will call some function given to it, likeonDismiss
.ComponentForm
should pass thisonDismiss
prop toErrorMessage
. When it's called,ComponentForm
shouldthis.setState({ error: null })
which will result inErrorMessage
re-rendering to be empty.The text was updated successfully, but these errors were encountered: