Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#1098: Made input labels accessible across the editor UI #8267

Merged
merged 45 commits into from
Nov 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
a356bd3
Materialized labeled and text inputs PoC.
oleq Jul 16, 2019
06ad0ad
More improvements to LabeledInputView and InputTextView.
oleq Jul 24, 2019
62e04b4
Ported the PoC to the monorepo.
oleq Aug 26, 2020
b47b9f0
Adjustments to the styles.
oleq Aug 27, 2020
ddd0237
Code refactoring.
oleq Aug 27, 2020
5d283ac
Code refactoring.
oleq Aug 27, 2020
e86069f
Code refactoring.
oleq Aug 27, 2020
ab1b9d0
Updated the classic build to showcase the feature.
oleq Aug 27, 2020
d8e365e
Merge branch 'master' into i/1098-accessible-input-labels
oleq Oct 9, 2020
a8bbd62
Merge branch 'master' into i/1098-accessible-input-labels
oleq Oct 13, 2020
f9ffc1b
Docs.
oleq Oct 13, 2020
50f3e61
Removed info text from the image insertion via URL field.
oleq Oct 13, 2020
716a1ba
Refactored tabe forms. Code refactoring and refining.
oleq Oct 13, 2020
b2925d3
Reverted changes made to the classic build sample.
oleq Oct 13, 2020
4b98c1d
Aligned image tests to the new LabeledInputView API.
oleq Oct 13, 2020
b8d9b18
Aligned LinkFormView to the new LabeledInputView API.
oleq Oct 13, 2020
7fa1257
Aligned MediaFormView to the new LabeledInputView API.
oleq Oct 13, 2020
ff716fe
Created ColorInputView #isFocused and #isEmpty observable properties.
oleq Oct 15, 2020
de997b7
Ensured the TableCellPropertiesView is filled with date before being …
oleq Oct 15, 2020
38fe929
Tests: Ensured the TablePropertiesView is filled with date before bei…
oleq Oct 15, 2020
cb2cdf2
Added tests for getLabeledColorInputCreator().
oleq Oct 15, 2020
c3c6adf
Tests: Updated table property form view tests to integrate with the n…
oleq Oct 15, 2020
fe9e874
Removed wrong docs from the LabeledInputView class.
oleq Oct 15, 2020
d37119e
Reverted wrong changes to the LabeledInputView class.
oleq Oct 15, 2020
0ef3d13
Tests: Added tests for LabeledFieldView.
oleq Oct 15, 2020
e57fd79
Tests and code refactoring in the InputTextView class.
oleq Oct 15, 2020
d276f14
Tests: Removed obsolete only().
oleq Oct 15, 2020
baaac13
InputTextView should update #isEmpty on DOM change event.
oleq Oct 15, 2020
df1038c
Code refactoring in MediaFormView.
oleq Oct 15, 2020
80e333f
Merge branch 'master' into i/1098-accessible-input-labels
oleq Oct 15, 2020
61732ae
Docs: Fixed broken docs in LabeledFieldView.
oleq Oct 15, 2020
5c95ff4
Created the injectCSSTransitionDisabling() View decorator. Disabled C…
oleq Oct 20, 2020
76a7bcc
Fixed border row styles in table forms when editor renders in German.
oleq Oct 20, 2020
6597408
Code refactoring.
oleq Oct 20, 2020
eed6700
Merge branch 'master' into i/1098-accessible-input-labels
oleq Oct 20, 2020
4ec95d3
Removed obsolete file.
oleq Oct 20, 2020
228f573
Integration with .ck-responsive-form.
oleq Oct 20, 2020
670bc95
Tests: Added missing tests for createLabeledInputText().
oleq Oct 20, 2020
9902136
Merge branch 'master' into i/1098-accessible-input-labels
Reinmar Nov 2, 2020
34d3a80
Brought RTL support to LabeledFieldView styles.
oleq Nov 4, 2020
ec85982
Brought support for RTL languages in the ColorInput styles.
oleq Nov 4, 2020
243b0f5
Tests: Updated theme-lark manual test to the latest API of LabeledField.
oleq Nov 4, 2020
9fad801
Increased the contrast between the color of the empty LabeledFieldVie…
oleq Nov 4, 2020
5b7f2d9
Merge branch 'master' into i/1098-accessible-input-labels
Reinmar Nov 5, 2020
fd7a56d
Renamed CSS to Css when used inside a code name.
Reinmar Nov 5, 2020
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
3 changes: 1 addition & 2 deletions packages/ckeditor5-image/src/imageinsert/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -126,6 +127,8 @@ export default class TextAlternativeFormView extends View {
this.cancelButtonView
]
} );

injectCssTransitionDisabler( this );
}

/**
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand Down
7 changes: 1 addition & 6 deletions packages/ckeditor5-image/tests/imageinsert/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' );
} );
} );
} );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()', () => {
Expand All @@ -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();

Expand Down
2 changes: 1 addition & 1 deletion packages/ckeditor5-image/theme/imageinsert.css
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 5 additions & 1 deletion packages/ckeditor5-link/src/linkui.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' );

Expand Down Expand Up @@ -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()
Expand All @@ -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
Expand Down
11 changes: 6 additions & 5 deletions packages/ckeditor5-link/src/ui/linkformview.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -152,6 +153,8 @@ export default class LinkFormView extends View {

children: this.children
} );

injectCssTransitionDisabler( this );
}

/**
Expand Down Expand Up @@ -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;
}
Expand Down
12 changes: 12 additions & 0 deletions packages/ckeditor5-link/tests/linkui.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' );

Expand Down
15 changes: 7 additions & 8 deletions packages/ckeditor5-link/tests/ui/linkformview.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand All @@ -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()', () => {
Expand All @@ -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 );
Expand Down
11 changes: 11 additions & 0 deletions packages/ckeditor5-link/theme/linkform.css
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
15 changes: 15 additions & 0 deletions packages/ckeditor5-media-embed/src/mediaembedui.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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', () => {
Expand All @@ -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' );
Expand Down
4 changes: 3 additions & 1 deletion packages/ckeditor5-media-embed/src/ui/mediaformview.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -147,6 +148,8 @@ export default class MediaFormView extends View {
]
} );

injectCssTransitionDisabler( this );

/**
* The default info text for the {@link #urlInputView}.
*
Expand Down Expand Up @@ -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.
Expand Down
10 changes: 10 additions & 0 deletions packages/ckeditor5-media-embed/tests/mediaembedui.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
} );
} );
} );

Expand Down
Loading