Commit 2a4a732d authored by Nathan Friend's avatar Nathan Friend

Fix throttling issue in form dirty checking

This commit fixes an issue that was causing the "Save changes" button
to be incorrectly enabled or disabled when changes were made to a form.
(Specifically, some of the subsections in the project settings pages.)
parent 913bc964
......@@ -21,10 +21,15 @@ class DirtySubmitForm {
}
registerListeners() {
const throttledUpdateDirtyInput = _.throttle(
event => this.updateDirtyInput(event),
DirtySubmitForm.THROTTLE_DURATION,
const getThrottledHandlerForInput = _.memoize(() =>
_.throttle(event => this.updateDirtyInput(event), DirtySubmitForm.THROTTLE_DURATION),
);
const throttledUpdateDirtyInput = event => {
const throttledHandler = getThrottledHandlerForInput(event.target.name);
throttledHandler(event);
};
this.form.addEventListener('input', throttledUpdateDirtyInput);
this.form.addEventListener('change', throttledUpdateDirtyInput);
$(this.form).on('change.select2', throttledUpdateDirtyInput);
......
---
title: Fix issue that causes "Save changes" button in project settings pages to be
enabled/disabled incorrectly when changes are made to the form
merge_request: 28377
author:
type: fixed
import _ from 'underscore';
import DirtySubmitForm from '~/dirty_submit/dirty_submit_form';
import { getInputValue, setInputValue, createForm } from './helper';
......@@ -13,46 +14,100 @@ function expectToToggleDisableOnDirtyUpdate(submit, input) {
}
describe('DirtySubmitForm', () => {
DirtySubmitForm.THROTTLE_DURATION = 0;
const originalThrottleDuration = DirtySubmitForm.THROTTLE_DURATION;
it('disables submit until there are changes', done => {
const { form, input, submit } = createForm();
describe('submit button tests', () => {
beforeEach(() => {
DirtySubmitForm.THROTTLE_DURATION = 0;
});
new DirtySubmitForm(form); // eslint-disable-line no-new
afterEach(() => {
DirtySubmitForm.THROTTLE_DURATION = originalThrottleDuration;
});
return expectToToggleDisableOnDirtyUpdate(submit, input)
.then(done)
.catch(done.fail);
});
it('disables submit until there are changes', done => {
const { form, input, submit } = createForm();
it('disables submit until there are changes when initializing with a falsy value', done => {
const { form, input, submit } = createForm();
input.value = '';
new DirtySubmitForm(form); // eslint-disable-line no-new
new DirtySubmitForm(form); // eslint-disable-line no-new
return expectToToggleDisableOnDirtyUpdate(submit, input)
.then(done)
.catch(done.fail);
});
return expectToToggleDisableOnDirtyUpdate(submit, input)
.then(done)
.catch(done.fail);
});
it('disables submit until there are changes when initializing with a falsy value', done => {
const { form, input, submit } = createForm();
input.value = '';
new DirtySubmitForm(form); // eslint-disable-line no-new
return expectToToggleDisableOnDirtyUpdate(submit, input)
.then(done)
.catch(done.fail);
});
it('disables submit until there are changes for radio inputs', done => {
const { form, input, submit } = createForm('radio');
it('disables submit until there are changes for radio inputs', done => {
const { form, input, submit } = createForm('radio');
new DirtySubmitForm(form); // eslint-disable-line no-new
new DirtySubmitForm(form); // eslint-disable-line no-new
return expectToToggleDisableOnDirtyUpdate(submit, input)
.then(done)
.catch(done.fail);
return expectToToggleDisableOnDirtyUpdate(submit, input)
.then(done)
.catch(done.fail);
});
it('disables submit until there are changes for checkbox inputs', done => {
const { form, input, submit } = createForm('checkbox');
new DirtySubmitForm(form); // eslint-disable-line no-new
return expectToToggleDisableOnDirtyUpdate(submit, input)
.then(done)
.catch(done.fail);
});
});
it('disables submit until there are changes for checkbox inputs', done => {
const { form, input, submit } = createForm('checkbox');
describe('throttling tests', () => {
beforeEach(() => {
jasmine.clock().install();
DirtySubmitForm.THROTTLE_DURATION = 100;
});
afterEach(() => {
jasmine.clock().uninstall();
DirtySubmitForm.THROTTLE_DURATION = originalThrottleDuration;
});
it('throttles updates when rapid changes are made to a single form element', () => {
const { form, input } = createForm();
const updateDirtyInputSpy = spyOn(new DirtySubmitForm(form), 'updateDirtyInput');
_.range(10).forEach(i => {
setInputValue(input, `change ${i}`, false);
});
jasmine.clock().tick(101);
expect(updateDirtyInputSpy).toHaveBeenCalledTimes(2);
});
it('does not throttle updates when rapid changes are made to different form elements', () => {
const form = document.createElement('form');
const range = _.range(10);
range.forEach(i => {
form.innerHTML += `<input type="text" name="input-${i}" class="js-input-${i}"/>`;
});
const updateDirtyInputSpy = spyOn(new DirtySubmitForm(form), 'updateDirtyInput');
range.forEach(i => {
const input = form.querySelector(`.js-input-${i}`);
setInputValue(input, `change`, false);
});
new DirtySubmitForm(form); // eslint-disable-line no-new
jasmine.clock().tick(101);
return expectToToggleDisableOnDirtyUpdate(submit, input)
.then(done)
.catch(done.fail);
expect(updateDirtyInputSpy).toHaveBeenCalledTimes(range.length);
});
});
});
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment