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

Add inline errors #5399

Merged
merged 1 commit into from
Jul 5, 2015
Merged

Add inline errors #5399

merged 1 commit into from
Jul 5, 2015

Conversation

acburdine
Copy link
Member

refs #5336

  • adds inline errors to setup form
  • creates gh-form-group component to handle form group status

TODOS:

  • add ability for errors to be validated per-property
  • reconfigure errors in parts of the application that need it
  • fix tests
  • cleanup code
  • squash
  • add 'dem comments

@acburdine
Copy link
Member Author

@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.

@ErisDS
Copy link
Member

ErisDS commented Jun 8, 2015

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.

@acburdine
Copy link
Member Author

@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.

@acburdine
Copy link
Member Author

@ErisDS this is almost done, although I may need some help with the casper tests (cc @cobbspur?)

@acburdine
Copy link
Member Author

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?

@novaugust
Copy link
Contributor

I've got a comment just on naming: I like not abbreviating things when possible, prop -> property

@jaswilli
Copy link
Contributor

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...

@acburdine
Copy link
Member Author

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.

@kevinansfield kevinansfield mentioned this pull request Jun 26, 2015
22 tasks
@acburdine acburdine force-pushed the inline-errors branch 4 times, most recently from 866c19a to 135f027 Compare June 30, 2015 01:39
@@ -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.

@acburdine acburdine force-pushed the inline-errors branch 3 times, most recently from d8fb33b to bf2c4a7 Compare July 1, 2015 04:35
@acburdine acburdine changed the title [WIP] Add inline errors Add inline errors Jul 1, 2015
@acburdine acburdine mentioned this pull request Jul 1, 2015
6 tasks
<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.

@ErisDS
Copy link
Member

ErisDS commented Jul 3, 2015

Is the last task on this one still TODO as well?

@acburdine
Copy link
Member Author

@ErisDS yep. I'll have time later today to finish it and fix the thing that @kevinansfield pointed out

@ErisDS
Copy link
Member

ErisDS commented Jul 5, 2015

@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
ErisDS added a commit that referenced this pull request Jul 5, 2015
@ErisDS ErisDS merged commit 82467c8 into TryGhost:master Jul 5, 2015
@acburdine acburdine deleted the inline-errors branch July 7, 2015 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants