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 form element to arguments passed to (private) validate hook #1793

Merged
merged 1 commit into from May 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions addon/components/bs-form.hbs
Expand Up @@ -5,6 +5,7 @@
...attributes
{{on "keypress" this.handleKeyPress}}
{{on "submit" this.handleSubmit}}
{{create-ref "formElement"}}
>
{{yield
(hash
Expand Down
13 changes: 11 additions & 2 deletions addon/components/bs-form.js
Expand Up @@ -9,6 +9,7 @@ import deprecateSubclassing from 'ember-bootstrap/utils/deprecate-subclassing';
import arg from '../utils/decorators/arg';
import { tracked } from '@glimmer/tracking';
import { cached } from 'tracked-toolbox';
import { ref } from 'ember-ref-bucket';

/**
Render a form with the appropriate Bootstrap layout class (see `formLayout`).
Expand Down Expand Up @@ -118,6 +119,13 @@ import { cached } from 'tracked-toolbox';
*/
@deprecateSubclassing
export default class Form extends Component {
/**
* @property _element
* @type null | HTMLFormElement
* @private
*/
@ref('formElement') _element = null;

/**
* Bootstrap form class name (computed)
*
Expand Down Expand Up @@ -332,10 +340,11 @@ export default class Form extends Component {
*
* @method validate
* @param {Object} model
* @param {HTMLFormElement} form
* @return {Promise}
* @public
*/
validate(model) {} // eslint-disable-line no-unused-vars
validate(model, form) {} // eslint-disable-line no-unused-vars

/**
* @property showAllValidations
Expand Down Expand Up @@ -400,7 +409,7 @@ export default class Form extends Component {

return RSVP.resolve()
.then(() => {
return this.hasValidator ? this.validate(model) : null;
return this.hasValidator ? this.validate(model, this._element) : null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somehow my review comment got lost. 😢

I'm wondering if we could pick up the form element from the event instead of tracking it through {{ref}}. The event listener is registered on the form element. Shouldn't the form element be available as event.target in that case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, indeed I had it this way, until I realized some existing test failed. It was one where we submit the form using the yielded submit action, and not a submit (browser) event. In which case we don't have event.target. That's why I also added the second test in this PR, which is replicating this very situation!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. 👏

})
.then(
(record) => {
Expand Down
58 changes: 58 additions & 0 deletions tests/integration/components/bs-form-test.js
Expand Up @@ -152,6 +152,64 @@ module('Integration | Component | bs-form', function (hooks) {
assert.verifySteps(['bubbles']);
});

test('form that supports validation calls validate hook when submitting', async function (assert) {
let model = {};
this.set('model', model);

class ValidatingForm extends Form {
'__ember-bootstrap_subclass' = true;

get hasValidator() {
return true;
}

async validate(modelArg, formArg) {
assert.step('validate');
assert.strictEqual(modelArg, model, 'validate is called with model argument');
assert.ok(formArg instanceof HTMLFormElement, 'validate is called with form argument');
}
}

this.set('form', ensureSafeComponent(ValidatingForm, this));

await render(hbs`
<this.form @model={{this.model}}>Test</this.form>
`);
await triggerEvent('form', 'submit');

assert.verifySteps(['validate'], 'validate has been called');
});

test('form that supports validation calls validate hook when calling submit action', async function (assert) {
let model = {};
this.set('model', model);

class ValidatingForm extends Form {
'__ember-bootstrap_subclass' = true;

get hasValidator() {
return true;
}

async validate(modelArg, formArg) {
assert.step('validate');
assert.strictEqual(modelArg, model, 'validate is called with model argument');
assert.ok(formArg instanceof HTMLFormElement, 'validate is called with form argument');
}
}

this.set('form', ensureSafeComponent(ValidatingForm, this));

await render(hbs`
<this.form @model={{this.model}} as |form|>
<button type="button" {{on "click" form.submit}}>Submit</button>
</this.form>
`);
await click('button');

assert.verifySteps(['validate'], 'validate has been called');
});

test('Submitting the form with valid validation calls onBeforeSubmit and onSubmit action', async function (assert) {
let submit = sinon.spy();
let before = sinon.spy();
Expand Down