Skip to content

Commit

Permalink
do not show errors before user interaction
Browse files Browse the repository at this point in the history
Errors aren't shown before a user interaction. If user interacts with a field
errors for that field are shown. If user submits the form, errors for all field
of the form are shown.

What is treated as an user interaction differs by field. Only if an interaction
could be seen as finished, it's causing errors to be shown. All focus out events
are treated as finished user interactions. A change event on an input field isn't
to prevent errors from popping up, while user inserts a valid string (e.g. if a
minimum length is required). On the other hand an change on select and checkbox
is.

This topic was discussed intensively in Emerson#15. There were many different ideas how
it could be implemented. This PR tries to reflect the last state of the debate.

In opposite to branch feature/delayed-validations this is not optional. I don't
think there are use cases where you want to show errors for untouched fields to
the user. If there are use cases one could pass `true` to `showErrors` property
of fields.
There shouldn't be any changes for people using server-side validations, since
there these ones aren't present before a form is submitted.
  • Loading branch information
jelhan committed Nov 20, 2015
1 parent 7030429 commit 36d691d
Show file tree
Hide file tree
Showing 19 changed files with 338 additions and 45 deletions.
25 changes: 22 additions & 3 deletions addon/components/fm-checkbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,28 @@ export default Ember.Component.extend({
classNameBindings: ['checkboxWrapperClass', 'errorClass'],
fmConfig: Ember.inject.service('fm-config'),
checkboxWrapperClass: Ember.computed.reads('fmConfig.checkboxWrapperClass'),
errorClass: Ember.computed('errors', 'fmConfig.errorClass', function() {
if(!Ember.isEmpty(this.get('errors'))) {
errorClass: Ember.computed('showErrors', 'fmConfig.errorClass', function() {
if(this.get('showErrors')) {
return this.get('fmConfig.errorClass');
}
})
}),

shouldShowErrors: false,
showErrors: Ember.computed('shouldShowErrors', 'errors', function() {
return this.get('shouldShowErrors') && !Ember.isEmpty(this.get('errors'));
}),

change() {
this.send('userInteraction');
},

focusOut() {
this.send('userInteraction');
},

actions: {
userInteraction() {
this.set('shouldShowErrors', true);
}
}
});
15 changes: 12 additions & 3 deletions addon/components/fm-field.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ export default Ember.Component.extend({
placeholder: null,
label: null,
classNameBindings: ['wrapperClass', 'errorClass'],
errorClass: Ember.computed('errors', 'fmConfig.errorClass', function() {
if(!Ember.isEmpty(this.get('errors'))) {
errorClass: Ember.computed('showErrors', 'fmConfig.errorClass', function() {
if (this.get('showErrors')) {
return this.get('fmConfig.errorClass');
}
}),
Expand Down Expand Up @@ -67,6 +67,15 @@ export default Ember.Component.extend({
} else {
this.set('value', value);
}
},

userInteraction() {
this.set('shouldShowErrors', true);
}
}
},

shouldShowErrors: false,
showErrors: Ember.computed('shouldShowErrors', 'errors', function() {
return this.get('shouldShowErrors') && !Ember.isEmpty(this.get('errors'));
})
});
5 changes: 5 additions & 0 deletions addon/components/fm-form.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ export default Ember.Component.extend({
'for': null,
submit: function(e) {
e.preventDefault();
this.get('childViews').forEach((chieldView) => {
if (chieldView.get('shouldShowErrors') === false) {
chieldView.set('shouldShowErrors', true);
}
});
this.sendAction('action', this.get('for'));
}
});
6 changes: 5 additions & 1 deletion addon/components/fm-input.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ import DataAttributesSupport from '../mixins/data-attribute-support';

export default Ember.TextField.extend(DataAttributesSupport, {

focusOut() {
this.sendAction('onUserInteraction');
},

init: function() {
if(this.get('parentView.forAttribute')) {
this.set('elementId', this.get('parentView.forAttribute'));
Expand All @@ -11,4 +15,4 @@ export default Ember.TextField.extend(DataAttributesSupport, {
this.setDataAttributes();
}

});
});
17 changes: 14 additions & 3 deletions addon/components/fm-radio-group.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,22 @@ export default Ember.Component.extend({
layout: layout,
classNameBindings: ['radioGroupWrapperClass', 'errorClass'],
fmConfig: Ember.inject.service('fm-config'),
errorClass: Ember.computed('errors', 'fmConfig.errorClass', function() {
if(!Ember.isEmpty(this.get('errors'))) {
errorClass: Ember.computed('showErrors', 'fmConfig.errorClass', function() {
if(this.get('showErrors')) {
return this.get('fmConfig.errorClass');
}
}),
radioGroupWrapperClass: Ember.computed.reads('fmConfig.radioGroupWrapperClass'),
labelClass: Ember.computed.reads('fmConfig.labelClass')
labelClass: Ember.computed.reads('fmConfig.labelClass'),

shouldShowErrors: false,
showErrors: Ember.computed('errors', 'shouldShowErrors', function() {
return this.get('shouldShowErrors') && !Ember.isEmpty(this.get('errors'));
}),

actions: {
userInteraction() {
this.set('shouldShowErrors', true);
}
}
});
4 changes: 4 additions & 0 deletions addon/components/fm-radio.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,9 @@ export default Ember.Component.extend({
}),
change: function() {
this.set('parentView.value', this.get('value'));
this.sendAction('onUserInteraction');
},
focusOut() {
this.sendAction('onUserInteraction');
}
});
6 changes: 5 additions & 1 deletion addon/components/fm-select.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export default Ember.Component.extend({

change: function() {
this.send('change');
this.sendAction('onUserInteraction');
// console.log('changing');
},

Expand Down Expand Up @@ -63,6 +64,9 @@ export default Ember.Component.extend({
const value = (path.length > 0)? Ember.get(selection, path) : selection;
this.attrs.action(value);
}
}
},

focusOut() {
this.sendAction('onUserInteraction');
}
});
5 changes: 4 additions & 1 deletion addon/components/fm-textarea.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ import Ember from 'ember';
import DataAttributesSupport from 'ember-form-master-2000/mixins/data-attribute-support';

export default Ember.TextArea.extend(DataAttributesSupport, {
focusOut() {
this.sendAction('onUserInteraction');
},

init: function() {
if(this.get('parentView.forAttribute')) {
Expand All @@ -11,4 +14,4 @@ export default Ember.TextArea.extend(DataAttributesSupport, {
this.setDataAttributes();
}

});
});
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
{{label}}
</label>

{{fm-errortext errors=errors}}
{{#if showErrors}}
{{fm-errortext errors=errors}}
{{/if}}

</div>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
value=value
classNameBindings='errorClass inputClass'
maxlength=maxlength
placeholder=placeholder}}
placeholder=placeholder
onUserInteraction='userInteraction'
}}
{{/if}}

{{#if isSelect}}
Expand All @@ -19,6 +21,7 @@
prompt=prompt
value=value
action=(action 'selectAction')
onUserInteraction='userInteraction'
}}
{{/if}}

Expand All @@ -32,10 +35,11 @@
maxlength=maxlength
spellcheck=spellcheck
disabled=disabled
onUserInteraction='userInteraction'
}}
{{/if}}

{{#if errors}}
{{#if showErrors}}
{{fm-errortext errors=errors}}
{{/if}}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@

{{#each content as |option|}}

{{fm-radio content=option}}
{{fm-radio content=option onUserInteraction='userInteraction'}}

{{/each}}

{{fm-errortext errors=errors}}
{{#if showErrors}}
{{fm-errortext errors=errors}}
{{/if}}
59 changes: 48 additions & 11 deletions tests/integration/components/fm-checkbox-test.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
// import { moduleForComponent, test } from 'ember-qunit';
// import hbs from 'htmlbars-inline-precompile';
import { moduleForComponent, test } from 'ember-qunit';
import hbs from 'htmlbars-inline-precompile';
// import {initialize} from 'ember-form-master-2000/initializers/fm-initialize';
//
// moduleForComponent('fm-checkbox', 'Integration | Component | fm-checkbox', {
// integration: true,
// setup: function() {
// this.container.inject = this.container.injection;
// initialize(null, this.container);
// }
// });
//

moduleForComponent('fm-checkbox', 'Integration | Component | fm-checkbox', {
integration: true,
setup: function() {
this.container.inject = this.container.injection;
// initialize(null, this.container);
}
});

// test('fm-checkbox renders properly', function(assert) {
// assert.expect(2);
// this.render(hbs `{{fm-checkbox}}`);
Expand All @@ -24,3 +24,40 @@
//
// assert.equal(this.$('label').text().trim(), 'This is the label', 'the fm-checkbox label matches');
// });

test('errors are shown after user interaction but not before', function(assert) {
this.set('errors', ['error message']);
this.render(hbs `{{fm-checkbox errors=errors}}`);
assert.ok(
this.$('.help-block').length === 0,
'error message is not shown before user interaction'
);
assert.notOk(
this.$('div').hasClass('has-error'),
'there is no errorClass before user interaction'
);
this.$('input').trigger('focusout');
assert.equal(
this.$('.help-block').text().trim(), 'error message',
'error message is shown after user interaction'
);
assert.ok(
this.$('div').hasClass('has-error'),
'errorClass is added after user interaction'
);
this.set('errors', []);
assert.notOk(
this.$('div').hasClass('has-error'),
'errorClass is removed when errors array got empty'
);
});

test('change event is treated as userInteraction', function(assert) {
this.set('errors', ['error message']);
this.render(hbs `{{fm-checkbox errors=errors}}`);
this.$('input').change();
assert.ok(
this.$('div').hasClass('has-error'),
'errorClass is added after change event'
);
});
82 changes: 82 additions & 0 deletions tests/integration/components/fm-field-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,85 @@ test('action is passed down to select component', function(assert) {
// assert.equal(this.$('label').attr('for'), 'example-id');
// assert.equal(this.$('input').attr('id'), 'example-id');
// });

test('errors are not shown after user interaction but not before', function(assert) {
this.set('errors', ['error message']);
this.render(hbs `{{fm-field errors=errors}}`);
assert.ok(
this.$('.help-block').length === 0,
'error message is not shown before user interaction'
);
assert.notOk(
this.$('div').hasClass('has-error'),
'errorClass is not there before user interaction'
);
this.$('input').trigger('focusout');
assert.ok(
this.$('.help-block').text().trim(), 'error message',
'error message is shown after user interaction'
);
assert.ok(
this.$('div').hasClass('has-error'),
'errorClass is added after user interaction'
);
this.set('errors', []);
assert.notOk(
this.$('div').hasClass('has-error'),
'errorClass is removed when errors empty got empty'
);
});

test('errors are not shown after user interaction but not before (textarea)', function(assert) {
this.set('errors', ['error message']);
this.render(hbs `{{fm-field type='textarea' errors=errors}}`);
assert.ok(
this.$('.help-block').length === 0,
'error message is not shown before user interaction'
);
assert.notOk(
this.$('div').hasClass('has-error'),
'errorClass is not there before user interaction'
);
this.$('textarea').trigger('focusout');
assert.ok(
this.$('.help-block').text().trim(), 'error message',
'error message is shown after user interaction'
);
assert.ok(
this.$('div').hasClass('has-error'),
'errorClass is added after user interaction'
);
this.set('errors', []);
assert.notOk(
this.$('div').hasClass('has-error'),
'errorClass is removed when errors empty got empty'
);
});

test('errors are not shown after user interaction but not before (select)', function(assert) {
this.set('errors', ['error message']);
this.render(hbs `{{fm-field type='select' errors=errors}}`);
assert.ok(
this.$('.help-block').length === 0,
'error message is not shown before user interaction'
);
assert.notOk(
this.$('div').hasClass('has-error'),
'errorClass is not there before user interaction'
);
this.$('select').trigger('focusout');
assert.ok(
this.$('.help-block').text().trim(), 'error message',
'error message is shown after user interaction'
);
assert.ok(
this.$('div').hasClass('has-error'),
'errorClass is added after user interaction'
);
this.set('errors', []);
assert.notOk(
this.$('div').hasClass('has-error'),
'errorClass is removed when errors empty got empty'
);
});

Loading

0 comments on commit 36d691d

Please sign in to comment.