diff --git a/src/app/editor.js b/src/app/editor.js index 8d4ab5a..aec35ca 100644 --- a/src/app/editor.js +++ b/src/app/editor.js @@ -292,6 +292,8 @@ export default class Editor { } else { this.rteEditor.setData( this.markdownEditor.getData() ); } + + this.fire( 'sync' ); } /** @@ -467,6 +469,15 @@ export default class Editor { } ); } + // Unlock the form whenever the textarea gets sync'ed. + { + this.on( 'sync', () => { + if ( this.getMode() === Editor.modes.RTE ) { + unlockForm( this ); + } + } ); + } + listenToData(); /** @@ -504,9 +515,10 @@ export default class Editor { */ function unlockForm() { if ( textarea.hasAttribute( 'data-github-writer-was-required' ) ) { - textarea.required = textarea.getAttribute( 'data-github-writer-was-required' ); + textarea.required = ( textarea.getAttribute( 'data-github-writer-was-required' ) === 'true' ); textarea.removeAttribute( 'data-github-writer-was-required' ); } + textarea.setCustomValidity( '' ); // Restore the "formnovalidate" attribute on submit buttons. @@ -529,9 +541,6 @@ export default class Editor { if ( editor.getMode() === Editor.modes.RTE ) { editor.syncEditors(); } - - // Since the textarea is sync`ed, the form is good to go. - unlockForm( editor ); } /** @@ -660,15 +669,16 @@ export default class Editor { const textarea = this.markdownEditor.dom.textarea; const form = textarea.form; - form.querySelectorAll( '[required]' ).forEach( element => { + disabled = Array.from( form.querySelectorAll( '[required]' ) ).some( element => { + // Instead of checking the markdown textarea, we check the RTE editor. if ( element === textarea ) { - // Instead of checking the markdown textarea, we check the RTE editor. - disabled = disabled || ckeditor.isEmpty; + // We may have changed the "required" attribute of the textarea. We want the original value. + const required = textarea.getAttribute( 'data-github-writer-was-required' ) !== 'false'; + + return required && ckeditor.isEmpty; } else if ( !isWiki ) { // For other elements, we just use DOM checks. - if ( !element.checkValidity() ) { - disabled = true; - } + return !element.checkValidity(); } } ); } @@ -688,16 +698,22 @@ export default class Editor { connectSubmitButtonObserver.call( this ); function connectSubmitButtonObserver() { - const button = this.dom.getSubmitBtn(); + this._submitButtonObserver = new MutationObserver( () => { + // noinspection JSPotentiallyInvalidUsageOfClassThis + this._setSubmitStatus(); + } ); - if ( button ) { - this._submitButtonObserver = new MutationObserver( () => { - // noinspection JSPotentiallyInvalidUsageOfClassThis - this._setSubmitStatus(); - } ); - this._submitButtonObserver.observe( button, { attributes: true, attributeFilter: [ 'disabled' ] } ); + // GH messes up with submit buttons as its will, so instead of observe a specific button, we observe + // anything in the form that had the "disabled" attributed changed (usually submit buttons). + this._submitButtonObserver.observe( this.dom.root, { + attributes: true, + attributeFilter: [ 'disabled' ], + subtree: true + } ); + if ( !this.domManipulator._hasSubmitBtnObRollback ) { this.domManipulator.addRollbackOperation( () => disconnectSubmitButtonObserver.call( this ) ); + this.domManipulator._hasSubmitBtnObRollback = true; } } diff --git a/tests/unit/editor.js b/tests/unit/editor.js index 51d3faf..0598b0c 100644 --- a/tests/unit/editor.js +++ b/tests/unit/editor.js @@ -620,6 +620,22 @@ describe( 'Editor', () => { } ); } ); + it( 'should unlock the form on sync', () => { + const editor = new Editor( GitHubPage.appendRoot( { text: 'test' } ) ); + const textarea = editor.dom.root.querySelector( 'textarea' ); + + return editor.create() + .then( () => { + expect( textarea.validity.customError ).to.be.false; + editor.rteEditor.setData( 'Changed data' ); + expect( textarea.validity.customError ).to.be.true; + + editor.syncEditors(); + + expect( textarea.validity.customError ).to.be.false; + } ); + } ); + it( 'should unlock the form during submit', () => { const editor = new Editor( GitHubPage.appendRoot( { text: 'test' } ) ); const textarea = editor.dom.root.querySelector( 'textarea' ); @@ -636,11 +652,55 @@ describe( 'Editor', () => { } ); } ); + it( 'should lock/unlock the form when switching modes', () => { + const editor = new Editor( GitHubPage.appendRoot( { text: 'test' } ) ); + const textarea = editor.dom.root.querySelector( 'textarea' ); + + return editor.create() + .then( () => { + editor.rteEditor.setData( 'Changed data' ); + expect( textarea.validity.customError ).to.be.true; + + editor.setMode( Editor.modes.MARKDOWN ); + expect( textarea.validity.customError ).to.be.false; + + editor.setMode( Editor.modes.RTE ); + expect( textarea.validity.customError ).to.be.true; + } ); + } ); + + it( 'should clean the textarea on form unlock', () => { + const editor = new Editor( GitHubPage.appendRoot() ); + const textarea = editor.dom.root.querySelector( 'textarea' ); + textarea.required = false; + + return editor.create() + .then( () => { + editor.rteEditor.setData( 'Changed data' ); + expect( textarea.validity.customError ).to.be.true; + expect( textarea.required ).to.be.true; + + editor.syncEditors(); + + expect( textarea.validity.customError ).to.be.false; + expect( textarea.required ).to.be.false; + + editor.rteEditor.setData( 'Changed data again' ); + expect( textarea.validity.customError ).to.be.true; + expect( textarea.required ).to.be.true; + + editor.syncEditors(); + + expect( textarea.validity.customError ).to.be.false; + expect( textarea.required ).to.be.false; + } ); + } ); + it( 'should remove "formnovalidate" from buttons on form lock', () => { const editor = new Editor( GitHubPage.appendRoot( { text: 'test', submitAlternative: true } ) ); const button = editor.dom.root.querySelector( '.js-quick-submit-alternative' ); - editor.create() + return editor.create() .then( () => { expect( button.hasAttribute( 'formnovalidate' ) ).to.be.true; editor.rteEditor.setData( 'Changed data' ); @@ -652,7 +712,7 @@ describe( 'Editor', () => { const editor = new Editor( GitHubPage.appendRoot( { text: 'test', submitAlternative: true } ) ); const button = editor.dom.root.querySelector( '.js-quick-submit-alternative' ); - editor.create() + return editor.create() .then( () => { expect( button.hasAttribute( 'formnovalidate' ) ).to.be.true; editor.rteEditor.setData( 'Changed data' ); @@ -820,11 +880,19 @@ describe( 'Editor', () => { it( 'should check other required elements (empty)', () => { const editor = new Editor( GitHubPage.appendRoot( { text: 'Test' } ) ); - editor.dom.root.insertAdjacentHTML( 'afterbegin', '' ); + const other = createElementFromHtml( '' ); + editor.dom.root.insertAdjacentElement( 'afterbegin', other ); return editor.create() .then( () => { expect( editor.dom.getSubmitBtn().disabled ).to.be.true; + + other.value = 'Something'; + editor.rteEditor.ckeditor.fire( 'change:isEmpty' ); + expect( editor.dom.getSubmitBtn().disabled ).to.be.false; + + editor.rteEditor.setData( '' ); + expect( editor.dom.getSubmitBtn().disabled ).to.be.true; } ); } ); @@ -884,35 +952,33 @@ describe( 'Editor', () => { } ); it( 'should observe external changes to submit.disabled', done => { + // The root textarea has "required". const editor = new Editor( GitHubPage.appendRoot() ); const otherInput = createElementFromHtml( '' ); editor.dom.root.insertAdjacentElement( 'afterbegin', otherInput ); editor.create() .then( () => { - expect( editor.dom.getSubmitBtn().disabled ).to.be.true; + expect( editor.dom.getSubmitBtn().disabled, 'disabled start' ).to.be.true; otherInput.value = 'Testing'; - expect( editor.dom.getSubmitBtn().disabled ).to.be.true; - editor.dom.getSubmitBtn().disabled = false; // Mutation observers are asynchronous, so we should still not see the editor fixup here. - expect( editor.dom.getSubmitBtn().disabled ).to.be.false; + expect( editor.dom.getSubmitBtn().disabled, 'disabled after mutation' ).to.be.false; // So we use a timout to give a chance to the observer to be invoked. setTimeout( () => { - expect( editor.dom.getSubmitBtn().disabled ).to.be.true; + expect( editor.dom.getSubmitBtn().disabled, 'not disabled after mutation' ).to.be.true; otherInput.value = ''; - expect( editor.dom.getSubmitBtn().disabled ).to.be.true; - editor.dom.getSubmitBtn().disabled = true; + setTimeout( () => { - expect( editor.dom.getSubmitBtn().disabled ).to.be.true; + expect( editor.dom.getSubmitBtn().disabled, 'stay disabled' ).to.be.true; done(); - }, 0 ); - }, 0 ); + } ); + } ); } ); } ); } );