-
-
Notifications
You must be signed in to change notification settings - Fork 10.6k
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
Add inline errors #5399
Add inline errors #5399
Conversation
@ErisDS Turns out this was a lot simpler than I thought. DS.Errors can be applied to any object, meaning it is fairly trivial to add it to validation and to templates for things. Currently this has been wired into the setup/two form, although the validation engine still shows notifications for the moment. |
I know this is still marked as WIP, but just checking the on-the-fly part of this hadn't been forgotten? I realise I didn't say explicitly in the spec that this should happen on de-focus, have updated that now. |
@ErisDS I think I fixed the on-the-fly part of it. Once I get a response on this comment: #5336 (comment) , and clean up a bit of code, I should be able to finish this up. |
I don't think there was any particular weird ember things that I did with this one, but can @jaswilli and @novaugust take a look at it sometime? |
I've got a comment just on naming: I like not abbreviating things when possible, |
Need to give this a closer look but initially, I have some minor concerns about co-opting DS.Errors for general purpose error handling and then being on the hook when it changes out from underneath us. Something to consider... |
IMO, the way DS.Errors works currently is pretty good. If it doesn't use DS.Errors explicitly, it should likely use something that works pretty much exactly like DS.Errors works at the moment. So I could copy the code out of DS.Errors into its own object, or we could leave it and then if DS.Errors changes in the future, we could change it then. The other thing too is that DS.Errors works really well with server side responses, meaning server-side errors are easier to handle. |
866c19a
to
135f027
Compare
@@ -31,10 +31,13 @@ export default Ember.Controller.extend(ValidationEngine, { | |||
$('#login').find('input').trigger('change'); | |||
|
|||
this.validate({format: false}).then(function () { | |||
console.log('testing'); |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
d8fb33b
to
bf2c4a7
Compare
<label for="password">Password</label> | ||
<span class="input-icon icon-lock"> | ||
{{input type="password" name="password" placeholder="At least 8 characters" class="gh-input" autofocus="autofocus" autocorrect="off" value=password }} | ||
{{gh-input type="password" name="password" placeholder="At least 7 characters" class="gh-input" autofocus="autofocus" autocorrect="off" value=password focusOut=(action "validate" "password")}} |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
Is the last task on this one still TODO as well? |
@ErisDS yep. I'll have time later today to finish it and fix the thing that @kevinansfield pointed out |
@acburdine can you ping me in slack when this is wrapped up please? Thanks! |
closes TryGhost#5336 - creates gh-form-group component to handle form group status - refactors current validation methods to work on a per-property basis - adds gh-error-message component to render error message - removes (comments out) tests that pertain to the old notifications until the new inline validation is added
refs #5336
TODOS: