Skip to content
This repository was archived by the owner on Sep 8, 2025. It is now read-only.

Several enhancements to the submit button state handling #174

Merged
fredck merged 1 commit into from
May 7, 2020
Merged

Several enhancements to the submit button state handling #174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 33 additions & 17 deletions src/app/editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,8 @@ export default class Editor {
} else {
this.rteEditor.setData( this.markdownEditor.getData() );
}

this.fire( 'sync' );
}

/**
Expand Down Expand Up @@ -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();

/**
Expand Down Expand Up @@ -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.
Expand All @@ -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 );
}

/**
Expand Down Expand Up @@ -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();
}
} );
}
Expand All @@ -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;
}
}

Expand Down
92 changes: 79 additions & 13 deletions tests/unit/editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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' );
Expand All @@ -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' );
Expand All @@ -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' );
Expand Down Expand Up @@ -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', '<input value="" required>' );
const other = createElementFromHtml( '<input class="test-el" value="" required>' );
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;
} );
} );

Expand Down Expand Up @@ -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( '<input class="test-el" value="" required>' );
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 );
} );
} );
} );
} );
} );
Expand Down