Merge pull request #8267 from ckeditor/i/1098-accessible-input-labels

Feature: Made input labels accessible across the editor UI. Closes #1098. Closes #8242. MAJOR BREAKING CHANGE (theme-lark): The look and behavior of the `LabeledFieldView` UI component (used for displaying fields across the project) have changed. This may require changes in your integration if it customizes the `.ck-labeled-field-view` selector (or its internals).
ckeditor · Nov 5, 2020 · 407a8a7 · 407a8a7
2 parents c7f69dd + fd7a56d
commit 407a8a7
Show file tree

Hide file tree

Showing 53 changed files with 989 additions and 211 deletions.
diff --git a/packages/ckeditor5-image/src/imageinsert/utils.js b/packages/ckeditor5-image/src/imageinsert/utils.js
@@ -70,8 +70,7 @@ export function createLabeledInputView( locale ) {
 	labeledInputView.set( {
 		label: t( 'Insert image via URL' )
 	} );
-	labeledInputView.fieldView.placeholder = 'https://example.com/src/image.png';
-	labeledInputView.infoText = t( 'Paste the image source URL.' );
+	labeledInputView.fieldView.placeholder = 'https://example.com/image.png';
 
 	return labeledInputView;
 }
diff --git a/packages/ckeditor5-image/src/imagetextalternative/imagetextalternativeui.js b/packages/ckeditor5-image/src/imagetextalternative/imagetextalternativeui.js
@@ -165,6 +165,8 @@ export default class ImageTextAlternativeUI extends Plugin {
 		const command = editor.commands.get( 'imageTextAlternative' );
 		const labeledInput = this._form.labeledInput;
 
+		this._form.disableCssTransitions();
+
 		if ( !this._isInBalloon ) {
 			this._balloon.add( {
 				view: this._form,
@@ -180,6 +182,8 @@ export default class ImageTextAlternativeUI extends Plugin {
 		labeledInput.fieldView.value = labeledInput.fieldView.element.value = command.value || '';
 
 		this._form.labeledInput.fieldView.select();
+
+		this._form.enableCssTransitions();
 	}
 
 	/**

diff --git a/packages/ckeditor5-image/src/imagetextalternative/ui/textalternativeformview.js b/packages/ckeditor5-image/src/imagetextalternative/ui/textalternativeformview.js
@@ -14,6 +14,7 @@ import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview';
 
 import LabeledFieldView from '@ckeditor/ckeditor5-ui/src/labeledfield/labeledfieldview';
 import { createLabeledInputText } from '@ckeditor/ckeditor5-ui/src/labeledfield/utils';
+import injectCssTransitionDisabler from '@ckeditor/ckeditor5-ui/src/bindings/injectcsstransitiondisabler';
 
 import submitHandler from '@ckeditor/ckeditor5-ui/src/bindings/submithandler';
 import KeystrokeHandler from '@ckeditor/ckeditor5-utils/src/keystrokehandler';
@@ -126,6 +127,8 @@ export default class TextAlternativeFormView extends View {
 				this.cancelButtonView
 			]
 		} );
+
+		injectCssTransitionDisabler( this );
 	}
 
 	/**
@@ -191,7 +194,6 @@ export default class TextAlternativeFormView extends View {
 		const labeledInput = new LabeledFieldView( this.locale, createLabeledInputText );
 
 		labeledInput.label = t( 'Text alternative' );
-		labeledInput.fieldView.placeholder = t( 'Text alternative' );
 
 		return labeledInput;
 	}

diff --git a/packages/ckeditor5-image/tests/imageinsert/ui/imageinsertpanelview.js b/packages/ckeditor5-image/tests/imageinsert/ui/imageinsertpanelview.js
@@ -161,21 +161,22 @@ describe( 'ImageUploadPanelView', () => {
 		} );
 
 		it( 'should register child views\' #element in #focusTracker with no integrations', () => {
-			const spy = testUtils.sinon.spy( FocusTracker.prototype, 'add' );
-
 			view = new ImageUploadPanelView( { t: () => {} } );
+
+			const spy = testUtils.sinon.spy( view.focusTracker, 'add' );
 			view.render();
 
 			sinon.assert.calledWithExactly( spy.getCall( 0 ), view.insertButtonView.element );
 			sinon.assert.calledWithExactly( spy.getCall( 1 ), view.cancelButtonView.element );
 		} );
 
 		it( 'should register child views\' #element in #focusTracker with "insertImageViaUrl" integration', () => {
-			const spy = testUtils.sinon.spy( FocusTracker.prototype, 'add' );
-
 			view = new ImageUploadPanelView( { t: () => {} }, {
 				'insertImageViaUrl': createLabeledInputView( { t: val => val } )
 			} );
+
+			const spy = testUtils.sinon.spy( view.focusTracker, 'add' );
+
 			view.render();
 
 			sinon.assert.calledWithExactly( spy.getCall( 0 ), view.getIntegration( 'insertImageViaUrl' ).element );

diff --git a/packages/ckeditor5-image/tests/imageinsert/utils.js b/packages/ckeditor5-image/tests/imageinsert/utils.js
@@ -130,12 +130,7 @@ describe( 'Upload utils', () => {
 		describe( 'image URL input view', () => {
 			it( 'should have placeholder', () => {
 				const view = createLabeledInputView( { t: val => val } );
-				expect( view.fieldView.placeholder ).to.equal( 'https://example.com/src/image.png' );
-			} );
-
-			it( 'should have info text', () => {
-				const view = createLabeledInputView( { t: val => val } );
-				expect( view.infoText ).to.match( /^Paste the image source URL/ );
+				expect( view.fieldView.placeholder ).to.equal( 'https://example.com/image.png' );
 			} );
 		} );
 	} );

diff --git a/packages/ckeditor5-image/tests/imagetextalternative/imagetextalternativeui.js b/packages/ckeditor5-image/tests/imagetextalternative/imagetextalternativeui.js
@@ -96,6 +96,19 @@ describe( 'ImageTextAlternativeUI', () => {
 			sinon.assert.calledOnce( spy );
 			expect( form.labeledInput.fieldView.value ).equals( '' );
 		} );
+
+		it( 'should disable CSS transitions before showing the form to avoid unnecessary animations (and then enable them again)', () => {
+			const addSpy = sinon.spy( balloon, 'add' );
+			const disableCssTransitionsSpy = sinon.spy( form, 'disableCssTransitions' );
+			const enableCssTransitionsSpy = sinon.spy( form, 'enableCssTransitions' );
+			const selectSpy = sinon.spy( form.labeledInput.fieldView, 'select' );
+
+			setData( model, '[<image src="" alt="foo bar"></image>]' );
+
+			button.fire( 'execute' );
+
+			sinon.assert.callOrder( disableCssTransitionsSpy, addSpy, selectSpy, enableCssTransitionsSpy );
+		} );
 	} );
 
 	describe( 'balloon panel form', () => {

diff --git a/packages/ckeditor5-image/tests/imagetextalternative/ui/textalternativeformview.js b/packages/ckeditor5-image/tests/imagetextalternative/ui/textalternativeformview.js
@@ -67,6 +67,10 @@ describe( 'TextAlternativeFormView', () => {
 
 			sinon.assert.calledOnce( spy );
 		} );
+
+		it( 'should implement the CSS transition disabling feature', () => {
+			expect( view.disableCssTransitions ).to.be.a( 'function' );
+		} );
 	} );
 
 	describe( 'render()', () => {
@@ -90,7 +94,7 @@ describe( 'TextAlternativeFormView', () => {
 			} );
 
 			it( 'should register child views\' #element in #focusTracker', () => {
-				const spy = testUtils.sinon.spy( FocusTracker.prototype, 'add' );
+				const spy = testUtils.sinon.spy( view.focusTracker, 'add' );
 
 				view.render();
 

diff --git a/packages/ckeditor5-image/theme/imageinsert.css b/packages/ckeditor5-image/theme/imageinsert.css
@@ -4,7 +4,7 @@
  */
 
 .ck.ck-image-insert__panel {
-	padding: var(--ck-spacing-standard);
+	padding: var(--ck-spacing-large);
 }
 
 .ck.ck-image-insert__ck-finder-button {

diff --git a/packages/ckeditor5-link/src/linkui.js b/packages/ckeditor5-link/src/linkui.js
@@ -163,7 +163,7 @@ export default class LinkUI extends Plugin {
 		const linkCommand = editor.commands.get( 'link' );
 		const defaultProtocol = editor.config.get( 'link.defaultProtocol' );
 
-		const formView = new LinkFormView( editor.locale, linkCommand, defaultProtocol );
+		const formView = new LinkFormView( editor.locale, linkCommand );
 
 		formView.urlInputView.fieldView.bind( 'value' ).to( linkCommand, 'value' );
 
@@ -314,6 +314,8 @@ export default class LinkUI extends Plugin {
 		const editor = this.editor;
 		const linkCommand = editor.commands.get( 'link' );
 
+		this.formView.disableCssTransitions();
+
 		this._balloon.add( {
 			view: this.formView,
 			position: this._getBalloonPositionData()
@@ -324,6 +326,8 @@ export default class LinkUI extends Plugin {
 			this.formView.urlInputView.fieldView.select();
 		}
 
+		this.formView.enableCssTransitions();
+
 		// Make sure that each time the panel shows up, the URL field remains in sync with the value of
 		// the command. If the user typed in the input, then canceled the balloon (`urlInputView.fieldView#value` stays
 		// unaltered) and re-opened it without changing the value of the link command (e.g. because they

diff --git a/packages/ckeditor5-link/src/ui/linkformview.js b/packages/ckeditor5-link/src/ui/linkformview.js
@@ -15,6 +15,7 @@ import SwitchButtonView from '@ckeditor/ckeditor5-ui/src/button/switchbuttonview
 
 import LabeledFieldView from '@ckeditor/ckeditor5-ui/src/labeledfield/labeledfieldview';
 import { createLabeledInputText } from '@ckeditor/ckeditor5-ui/src/labeledfield/utils';
+import injectCssTransitionDisabler from '@ckeditor/ckeditor5-ui/src/bindings/injectcsstransitiondisabler';
 
 import submitHandler from '@ckeditor/ckeditor5-ui/src/bindings/submithandler';
 import FocusTracker from '@ckeditor/ckeditor5-utils/src/focustracker';
@@ -43,7 +44,7 @@ export default class LinkFormView extends View {
 	 * @param {module:link/linkcommand~LinkCommand} linkCommand Reference to {@link module:link/linkcommand~LinkCommand}.
 	 * @param {String} [protocol] A value of a protocol to be displayed in the input's placeholder.
 	 */
-	constructor( locale, linkCommand, protocol ) {
+	constructor( locale, linkCommand ) {
 		super( locale );
 
 		const t = locale.t;
@@ -69,7 +70,7 @@ export default class LinkFormView extends View {
 		 *
 		 * @member {module:ui/labeledfield/labeledfieldview~LabeledFieldView}
 		 */
-		this.urlInputView = this._createUrlInput( protocol );
+		this.urlInputView = this._createUrlInput();
 
 		/**
 		 * The Save button view.
@@ -152,6 +153,8 @@ export default class LinkFormView extends View {
 
 			children: this.children
 		} );
+
+		injectCssTransitionDisabler( this );
 	}
 
 	/**
@@ -209,15 +212,13 @@ export default class LinkFormView extends View {
 	 * Creates a labeled input view.
 	 *
 	 * @private
-	 * @param {String} [protocol=http://] A value of a protocol to be displayed in the input's placeholder.
 	 * @returns {module:ui/labeledfield/labeledfieldview~LabeledFieldView} Labeled field view instance.
 	 */
-	_createUrlInput( protocol = 'https://' ) {
+	_createUrlInput() {
 		const t = this.locale.t;
 		const labeledInput = new LabeledFieldView( this.locale, createLabeledInputText );
 
 		labeledInput.label = t( 'Link URL' );
-		labeledInput.fieldView.placeholder = protocol + 'example.com';
 
 		return labeledInput;
 	}

diff --git a/packages/ckeditor5-link/tests/linkui.js b/packages/ckeditor5-link/tests/linkui.js
@@ -1107,6 +1107,18 @@ describe( 'LinkUI', () => {
 				sinon.assert.calledOnce( selectSpy );
 			} );
 
+			it( 'should disable CSS transitions before showing the form to avoid unnecessary animations' +
+				'(and then enable them again)', () => {
+				const addSpy = sinon.spy( balloon, 'add' );
+				const disableCssTransitionsSpy = sinon.spy( formView, 'disableCssTransitions' );
+				const enableCssTransitionsSpy = sinon.spy( formView, 'enableCssTransitions' );
+				const selectSpy = sinon.spy( formView.urlInputView.fieldView, 'select' );
+
+				actionsView.fire( 'edit' );
+
+				sinon.assert.callOrder( disableCssTransitionsSpy, addSpy, selectSpy, enableCssTransitionsSpy );
+			} );
+
 			it( 'should execute unlink command on actionsView#unlink event', () => {
 				const executeSpy = testUtils.sinon.spy( editor, 'execute' );
 

diff --git a/packages/ckeditor5-link/tests/ui/linkformview.js b/packages/ckeditor5-link/tests/ui/linkformview.js
@@ -82,12 +82,6 @@ describe( 'LinkFormView', () => {
 			expect( spy.calledOnce ).to.true;
 		} );
 
-		describe( 'url input view', () => {
-			it( 'has placeholder', () => {
-				expect( view.urlInputView.fieldView.placeholder ).to.equal( 'https://example.com' );
-			} );
-		} );
-
 		describe( 'template', () => {
 			it( 'has url input view', () => {
 				expect( view.template.children[ 0 ].get( 0 ) ).to.equal( view.urlInputView );
@@ -98,6 +92,10 @@ describe( 'LinkFormView', () => {
 				expect( view.template.children[ 0 ].get( 2 ) ).to.equal( view.cancelButtonView );
 			} );
 		} );
+
+		it( 'should implement the CSS transition disabling feature', () => {
+			expect( view.disableCssTransitions ).to.be.a( 'function' );
+		} );
 	} );
 
 	describe( 'render()', () => {
@@ -110,9 +108,10 @@ describe( 'LinkFormView', () => {
 		} );
 
 		it( 'should register child views\' #element in #focusTracker', () => {
-			const spy = testUtils.sinon.spy( FocusTracker.prototype, 'add' );
-
 			view = new LinkFormView( { t: () => {} }, { manualDecorators: [] } );
+
+			const spy = testUtils.sinon.spy( view.focusTracker, 'add' );
+
 			view.render();
 
 			sinon.assert.calledWithExactly( spy.getCall( 0 ), view.urlInputView.element );

diff --git a/packages/ckeditor5-link/theme/linkform.css b/packages/ckeditor5-link/theme/linkform.css
@@ -31,4 +31,15 @@
  */
 .ck.ck-link-form_layout-vertical {
 	display: block;
+
+	/*
+	 * Whether the form is in the responsive mode or not, if there are decorator buttons
+	 * keep the top margin of action buttons medium.
+	 */
+	& .ck-button {
+		&.ck-button-save,
+		&.ck-button-cancel {
+			margin-top: var(--ck-spacing-medium);
+		}
+	}
 }
diff --git a/packages/ckeditor5-media-embed/src/mediaembedui.js b/packages/ckeditor5-media-embed/src/mediaembedui.js
@@ -53,6 +53,12 @@ export default class MediaEmbedUI extends Plugin {
 		} );
 	}
 
+	/**
+	 * @private
+	 * @param {module:ui/dropdown/dropdownview~DropdownView} dropdown
+	 * @param {module:ui/view~View} form
+	 * @param {module:media-embed/mediaembedcommand~MediaEmbedCommand} command
+	 */
 	_setUpDropdown( dropdown, form, command ) {
 		const editor = this.editor;
 		const t = editor.t;
@@ -71,6 +77,8 @@ export default class MediaEmbedUI extends Plugin {
 		// default action of the drop-down is executed (i.e. the panel showed up). Otherwise, the
 		// invisible form/input cannot be focused/selected.
 		button.on( 'open', () => {
+			form.disableCssTransitions();
+
 			// Make sure that each time the panel shows up, the URL field remains in sync with the value of
 			// the command. If the user typed in the input, then canceled (`urlInputView#fieldView#value` stays
 			// unaltered) and re-opened it without changing the value of the media command (e.g. because they
@@ -79,6 +87,7 @@ export default class MediaEmbedUI extends Plugin {
 			form.url = command.value || '';
 			form.urlInputView.fieldView.select();
 			form.focus();
+			form.enableCssTransitions();
 		}, { priority: 'low' } );
 
 		dropdown.on( 'submit', () => {
@@ -97,6 +106,12 @@ export default class MediaEmbedUI extends Plugin {
 		}
 	}
 
+	/**
+	 * @private
+	 * @param {module:ui/dropdown/dropdownview~DropdownView} dropdown
+	 * @param {module:ui/view~View} form
+	 * @param {module:media-embed/mediaembedcommand~MediaEmbedCommand} command
+	 */
 	_setUpForm( dropdown, form, command ) {
 		form.delegate( 'submit', 'cancel' ).to( dropdown );
 		form.urlInputView.bind( 'value' ).to( command, 'value' );

diff --git a/packages/ckeditor5-media-embed/src/ui/mediaformview.js b/packages/ckeditor5-media-embed/src/ui/mediaformview.js
@@ -19,6 +19,7 @@ import submitHandler from '@ckeditor/ckeditor5-ui/src/bindings/submithandler';
 import FocusTracker from '@ckeditor/ckeditor5-utils/src/focustracker';
 import FocusCycler from '@ckeditor/ckeditor5-ui/src/focuscycler';
 import KeystrokeHandler from '@ckeditor/ckeditor5-utils/src/keystrokehandler';
+import injectCssTransitionDisabler from '@ckeditor/ckeditor5-ui/src/bindings/injectcsstransitiondisabler';
 
 import checkIcon from '@ckeditor/ckeditor5-core/theme/icons/check.svg';
 import cancelIcon from '@ckeditor/ckeditor5-core/theme/icons/cancel.svg';
@@ -147,6 +148,8 @@ export default class MediaFormView extends View {
 			]
 		} );
 
+		injectCssTransitionDisabler( this );
+
 		/**
 		 * The default info text for the {@link #urlInputView}.
 		 *
@@ -282,7 +285,6 @@ export default class MediaFormView extends View {
 
 		labeledInput.label = t( 'Media URL' );
 		labeledInput.infoText = this._urlInputViewInfoDefault;
-		inputField.placeholder = 'https://example.com';
 
 		inputField.on( 'input', () => {
 			// Display the tip text only when there's some value. Otherwise fall back to the default info text.

diff --git a/packages/ckeditor5-media-embed/tests/mediaembedui.js b/packages/ckeditor5-media-embed/tests/mediaembedui.js
@@ -127,6 +127,16 @@ describe( 'MediaEmbedUI', () => {
 					button.fire( 'open' );
 					sinon.assert.calledOnce( spy );
 				} );
+
+				it( 'should disable CSS transitions to avoid unnecessary animations (and then enable them again)', () => {
+					const disableCssTransitionsSpy = sinon.spy( form, 'disableCssTransitions' );
+					const enableCssTransitionsSpy = sinon.spy( form, 'enableCssTransitions' );
+					const selectSpy = sinon.spy( form.urlInputView.fieldView, 'select' );
+
+					button.fire( 'open' );
+
+					sinon.assert.callOrder( disableCssTransitionsSpy, selectSpy, enableCssTransitionsSpy );
+				} );
 			} );
 		} );