Commit ed1da730 authored by Filipa Lacerda's avatar Filipa Lacerda

Merge branch '53856-changing-group-visibility-does-not-re-enable-save-button' into 'master'

Resolve "Changing group visibility does not re-enable Save button"

Closes #53856

See merge request gitlab-org/gitlab-ce!23022
parents 03def93b 88ce491f
......@@ -25,15 +25,16 @@ class DirtySubmitForm {
DirtySubmitForm.THROTTLE_DURATION,
);
this.form.addEventListener('input', throttledUpdateDirtyInput);
this.form.addEventListener('change', throttledUpdateDirtyInput);
this.form.addEventListener('submit', event => this.formSubmit(event));
}
updateDirtyInput(event) {
const input = event.target;
const { target } = event;
if (!input.dataset.isDirtySubmitInput) return;
if (!target.dataset.isDirtySubmitInput) return;
this.updateDirtyInputs(input);
this.updateDirtyInputs(target);
this.toggleSubmission();
}
......
---
title: Fix suboptimal handling of checkbox and radio input events causing
group general settings submit button to stay disabled after changing its visibility
merge_request: 23022
author:
type: fixed
......@@ -154,7 +154,7 @@ describe 'Group' do
end
describe 'group edit', :js do
let(:group) { create(:group) }
let(:group) { create(:group, :public) }
let(:path) { edit_group_path(group) }
let(:new_name) { 'new-name' }
......@@ -163,6 +163,8 @@ describe 'Group' do
end
it_behaves_like 'dirty submit form', [{ form: '.js-general-settings-form', input: 'input[name="group[name]"]' },
{ form: '.js-general-settings-form', input: '#group_visibility_level_0' },
{ form: '.js-general-permissions-form', input: '#group_request_access_enabled' },
{ form: '.js-general-permissions-form', input: 'input[name="group[two_factor_grace_period]"]' }]
it 'saves new settings' do
......
import DirtySubmitCollection from '~/dirty_submit/dirty_submit_collection';
import { setInput, createForm } from './helper';
import { setInputValue, createForm } from './helper';
describe('DirtySubmitCollection', () => {
it('disables submits until there are changes', done => {
......@@ -14,11 +14,11 @@ describe('DirtySubmitCollection', () => {
expect(submit.disabled).toBe(true);
return setInput(input, `${originalValue} changes`)
return setInputValue(input, `${originalValue} changes`)
.then(() => {
expect(submit.disabled).toBe(false);
})
.then(() => setInput(input, originalValue))
.then(() => setInputValue(input, originalValue))
.then(() => {
expect(submit.disabled).toBe(true);
})
......
import DirtySubmitForm from '~/dirty_submit/dirty_submit_form';
import { setInput, createForm } from './helper';
import { getInputValue, setInputValue, createForm } from './helper';
function expectToToggleDisableOnDirtyUpdate(submit, input) {
const originalValue = input.value;
const originalValue = getInputValue(input);
expect(submit.disabled).toBe(true);
return setInput(input, `${originalValue} changes`)
return setInputValue(input, `${originalValue} changes`)
.then(() => expect(submit.disabled).toBe(false))
.then(() => setInput(input, originalValue))
.then(() => setInputValue(input, originalValue))
.then(() => expect(submit.disabled).toBe(true));
}
......@@ -33,4 +33,24 @@ describe('DirtySubmitForm', () => {
.then(done)
.catch(done.fail);
});
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
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);
});
});
import DirtySubmitForm from '~/dirty_submit/dirty_submit_form';
import setTimeoutPromiseHelper from '../helpers/set_timeout_promise_helper';
export function setInput(element, value) {
element.value = value;
function isCheckableType(type) {
return /^(radio|checkbox)$/.test(type);
}
export function setInputValue(element, value) {
const { type } = element;
let eventType;
if (isCheckableType(type)) {
element.checked = !element.checked;
eventType = 'change';
} else {
element.value = value;
eventType = 'input';
}
element.dispatchEvent(
new Event('input', {
new Event(eventType, {
bubbles: true,
cancelable: true,
}),
);
return setTimeoutPromiseHelper(DirtySubmitForm.THROTTLE_DURATION);
}
export function createForm() {
export function getInputValue(input) {
return isCheckableType(input.type) ? input.checked : input.value;
}
export function createForm(type = 'text') {
const form = document.createElement('form');
form.innerHTML = `
<input type="text" value="original" class="js-input" name="input" />
<input type="${type}" name="${type}" class="js-input"/>
<button type="submit" class="js-dirty-submit"></button>
`;
const input = form.querySelector('.js-input');
const submit = form.querySelector('.js-dirty-submit');
......
shared_examples 'dirty submit form' do |selector_args|
selectors = selector_args.is_a?(Array) ? selector_args : [selector_args]
def expect_disabled_state(form, submit, is_disabled = true)
disabled_selector = is_disabled == true ? '[disabled]' : ':not([disabled])'
form.find(".js-dirty-submit#{disabled_selector}", match: :first)
expect(submit.disabled?).to be is_disabled
end
selectors.each do |selector|
it "disables #{selector[:form]} submit until there are changes", :js do
it "disables #{selector[:form]} submit until there are changes on #{selector[:input]}", :js do
form = find(selector[:form])
submit = form.first('.js-dirty-submit')
input = form.first(selector[:input])
is_radio = input[:type] == 'radio'
is_checkbox = input[:type] == 'checkbox'
is_checkable = is_radio || is_checkbox
original_value = input.value
original_checkable = form.find("input[name='#{input[:name]}'][checked]") if is_radio
original_checkable = input if is_checkbox
expect(submit.disabled?).to be true
expect(input.checked?).to be false
input.set("#{original_value} changes")
is_checkable ? input.click : input.set("#{original_value} changes")
form.find('.js-dirty-submit:not([disabled])', match: :first)
expect(submit.disabled?).to be false
expect_disabled_state(form, submit, false)
input.set(original_value)
is_checkable ? original_checkable.click : input.set(original_value)
form.find('.js-dirty-submit[disabled]', match: :first)
expect(submit.disabled?).to be true
expect_disabled_state(form, submit)
end
end
end
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