diff --git a/app/assets/javascripts/dirty_submit/dirty_submit_form.js b/app/assets/javascripts/dirty_submit/dirty_submit_form.js index d8d0fa1fac422e52cc747c3b4a88347b9bd6378e..00e41dd030133756e88bf94fa0040a62dbbd9948 100644 --- a/app/assets/javascripts/dirty_submit/dirty_submit_form.js +++ b/app/assets/javascripts/dirty_submit/dirty_submit_form.js @@ -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(); } diff --git a/changelogs/unreleased/53856-changing-group-visibility-does-not-re-enable-save-button.yml b/changelogs/unreleased/53856-changing-group-visibility-does-not-re-enable-save-button.yml new file mode 100644 index 0000000000000000000000000000000000000000..1daa72fb9c47fafec223f186fb57967fdaf64a4f --- /dev/null +++ b/changelogs/unreleased/53856-changing-group-visibility-does-not-re-enable-save-button.yml @@ -0,0 +1,6 @@ +--- +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 diff --git a/spec/features/groups_spec.rb b/spec/features/groups_spec.rb index d01fc04311a7fded0be225d20c7a0d49ecdaf4b7..00d81b26ce248fd9dba8c216d679a318886bcc15 100644 --- a/spec/features/groups_spec.rb +++ b/spec/features/groups_spec.rb @@ -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 diff --git a/spec/javascripts/dirty_submit/dirty_submit_collection_spec.js b/spec/javascripts/dirty_submit/dirty_submit_collection_spec.js index 08ffc44605fec7bbacfdb998b951778de8626498..47be0b3ce9dc3ffba6c74836dc3ac280738a3cbd 100644 --- a/spec/javascripts/dirty_submit/dirty_submit_collection_spec.js +++ b/spec/javascripts/dirty_submit/dirty_submit_collection_spec.js @@ -1,5 +1,5 @@ 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); }) diff --git a/spec/javascripts/dirty_submit/dirty_submit_form_spec.js b/spec/javascripts/dirty_submit/dirty_submit_form_spec.js index 093fec9795125b90c70cabe6bdd68f4ad5e436a0..ae2a785de5279f6eda99d9e6230aa5958055c6d4 100644 --- a/spec/javascripts/dirty_submit/dirty_submit_form_spec.js +++ b/spec/javascripts/dirty_submit/dirty_submit_form_spec.js @@ -1,14 +1,14 @@ 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); + }); }); diff --git a/spec/javascripts/dirty_submit/helper.js b/spec/javascripts/dirty_submit/helper.js index 6d1e643553cba854360743a622016ab508d07f3c..b51783cb9159d5804287c8de86b67630ef2f780b 100644 --- a/spec/javascripts/dirty_submit/helper.js +++ b/spec/javascripts/dirty_submit/helper.js @@ -1,25 +1,42 @@ 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'); diff --git a/spec/support/shared_examples/dirty_submit_form_shared_examples.rb b/spec/support/shared_examples/dirty_submit_form_shared_examples.rb index ba363593120d6f40f16753d37d7177bee6000cc4..52a2ee494952ce02d58c76662372bcdb6ca8769f 100644 --- a/spec/support/shared_examples/dirty_submit_form_shared_examples.rb +++ b/spec/support/shared_examples/dirty_submit_form_shared_examples.rb @@ -1,24 +1,36 @@ 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