Skip to content

Commit

Permalink
Add error managment in places where the user input is needed (#16172)
Browse files Browse the repository at this point in the history
Feature (link): An error message should appear in the link editing form when submitting an empty link.
Feature (media-embed): An error message should appear when submitting an empty URL in the media embed form.
Fix (ui): The color picker should not allow for saving incorrect HEX color values. Added an error message when the color is invalid.

---------

Co-authored-by: CKEditorBot <ckeditor-bot@cksource.com>
Co-authored-by: Aleksander Nowodzinski <a.nowodzinski@cksource.com>
  • Loading branch information
3 people authored Apr 15, 2024
1 parent ba1dedd commit caea11e
Show file tree
Hide file tree
Showing 16 changed files with 391 additions and 53 deletions.
2 changes: 1 addition & 1 deletion packages/ckeditor5-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export { default as Context, type ContextConfig } from './context.js';
export { default as ContextPlugin, type ContextPluginDependencies } from './contextplugin.js';
export { type EditingKeystrokeCallback } from './editingkeystrokehandler.js';

export type { PartialBy, NonEmptyArray } from './typings.js';
export type { PartialBy, NonEmptyArray, HexColor } from './typings.js';

export { default as Editor, type EditorReadyEvent, type EditorDestroyEvent } from './editor/editor.js';
export type {
Expand Down
2 changes: 2 additions & 0 deletions packages/ckeditor5-core/src/typings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,5 @@ export type PartialBy<T, K extends keyof T> = Omit<T, K> & Partial<Pick<T, K>>;
export type NonEmptyArray<A> = Array<A> & {
0: A;
};

export type HexColor<S extends string = string> = `#${ S }`;
1 change: 1 addition & 0 deletions packages/ckeditor5-link/lang/contexts.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"Unlink": "Toolbar button tooltip for the Unlink feature.",
"Link": "Toolbar button tooltip for the Link feature.",
"Link URL": "Label for the URL input in the Link URL editing balloon.",
"Link URL must not be empty.": "An error text displayed when user attempted to enter an empty URL.",
"Link image": "Label for the image link button.",
"Edit link": "Button opening the Link URL editing balloon.",
"Open link in new tab": "Button opening the link in new browser tab.",
Expand Down
49 changes: 35 additions & 14 deletions packages/ckeditor5-link/src/linkui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* @module link/linkui
*/

import { Plugin } from 'ckeditor5/src/core.js';
import { Plugin, type Editor } from 'ckeditor5/src/core.js';
import {
ClickObserver,
type ViewAttributeElement,
Expand All @@ -26,7 +26,7 @@ import {
import type { PositionOptions } from 'ckeditor5/src/utils.js';
import { isWidget } from 'ckeditor5/src/widget.js';

import LinkFormView from './ui/linkformview.js';
import LinkFormView, { type LinkFormValidatorCallback } from './ui/linkformview.js';
import LinkActionsView from './ui/linkactionsview.js';
import type LinkCommand from './linkcommand.js';
import type UnlinkCommand from './unlinkcommand.js';
Expand Down Expand Up @@ -195,28 +195,30 @@ export default class LinkUI extends Plugin {
const editor = this.editor;
const linkCommand: LinkCommand = editor.commands.get( 'link' )!;
const defaultProtocol = editor.config.get( 'link.defaultProtocol' );
const allowCreatingEmptyLinks = editor.config.get( 'link.allowCreatingEmptyLinks' );

const formView = new ( CssTransitionDisablerMixin( LinkFormView ) )( editor.locale, linkCommand );
const formView = new ( CssTransitionDisablerMixin( LinkFormView ) )( editor.locale, linkCommand, getFormValidators( editor ) );

formView.urlInputView.fieldView.bind( 'value' ).to( linkCommand, 'value' );

// Form elements should be read-only when corresponding commands are disabled.
formView.urlInputView.bind( 'isEnabled' ).to( linkCommand, 'isEnabled' );

// Disable the "save" button if the command is disabled or the input is empty despite being required.
formView.saveButtonView.bind( 'isEnabled' ).to(
linkCommand, 'isEnabled',
formView.urlInputView, 'isEmpty',
( isCommandEnabled, isInputEmpty ) => isCommandEnabled && ( allowCreatingEmptyLinks || !isInputEmpty )
);
// Disable the "save" button if the command is disabled.
formView.saveButtonView.bind( 'isEnabled' ).to( linkCommand, 'isEnabled' );

// Execute link command after clicking the "Save" button.
this.listenTo( formView, 'submit', () => {
const { value } = formView.urlInputView.fieldView.element!;
const parsedUrl = addLinkProtocolIfApplicable( value, defaultProtocol );
editor.execute( 'link', parsedUrl, formView.getDecoratorSwitchesState() );
this._closeFormView();
if ( formView.isValid() ) {
const { value } = formView.urlInputView.fieldView.element!;
const parsedUrl = addLinkProtocolIfApplicable( value, defaultProtocol );
editor.execute( 'link', parsedUrl, formView.getDecoratorSwitchesState() );
this._closeFormView();
}
} );

// Update balloon position when form error changes.
this.listenTo( formView.urlInputView, 'change:errorText', () => {
editor.ui.update();
} );

// Hide the panel after clicking the "Cancel" button.
Expand Down Expand Up @@ -384,6 +386,7 @@ export default class LinkUI extends Plugin {
const linkCommand: LinkCommand = editor.commands.get( 'link' )!;

this.formView!.disableCssTransitions();
this.formView!.resetFormStatus();

this._balloon.add( {
view: this.formView!,
Expand Down Expand Up @@ -755,3 +758,21 @@ export default class LinkUI extends Plugin {
function findLinkElementAncestor( position: ViewPosition ): ViewAttributeElement | null {
return position.getAncestors().find( ( ancestor ): ancestor is ViewAttributeElement => isLinkElement( ancestor ) ) || null;
}

/**
* Returns link form validation callbacks.
*
* @param editor Editor instance.
*/
function getFormValidators( editor: Editor ): Array<LinkFormValidatorCallback> {
const t = editor.t;
const allowCreatingEmptyLinks = editor.config.get( 'link.allowCreatingEmptyLinks' );

return [
form => {
if ( !allowCreatingEmptyLinks && !form.url!.length ) {
return t( 'Link URL must not be empty.' );
}
}
];
}
64 changes: 63 additions & 1 deletion packages/ckeditor5-link/src/ui/linkformview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ export default class LinkFormView extends View {
*/
public readonly children: ViewCollection;

/**
* An array of form validators used by {@link #isValid}.
*/
private readonly _validators: Array<LinkFormValidatorCallback>;

/**
* A collection of views that can be focused in the form.
*/
Expand All @@ -95,12 +100,14 @@ export default class LinkFormView extends View {
*
* @param locale The localization services instance.
* @param linkCommand Reference to {@link module:link/linkcommand~LinkCommand}.
* @param validators Form validators used by {@link #isValid}.
*/
constructor( locale: Locale, linkCommand: LinkCommand ) {
constructor( locale: Locale, linkCommand: LinkCommand, validators: Array<LinkFormValidatorCallback> ) {
super( locale );

const t = locale.t;

this._validators = validators;
this.urlInputView = this._createUrlInput();
this.saveButtonView = this._createButton( t( 'Save' ), icons.check, 'ck-button-save' );
this.saveButtonView.type = 'submit';
Expand Down Expand Up @@ -203,6 +210,37 @@ export default class LinkFormView extends View {
this._focusCycler.focusFirst();
}

/**
* Validates the form and returns `false` when some fields are invalid.
*/
public isValid(): boolean {
this.resetFormStatus();

for ( const validator of this._validators ) {
const errorText = validator( this );

// One error per field is enough.
if ( errorText ) {
// Apply updated error.
this.urlInputView.errorText = errorText;

return false;
}
}

return true;
}

/**
* Cleans up the supplementary error and information text of the {@link #urlInputView}
* bringing them back to the state when the form has been displayed for the first time.
*
* See {@link #isValid}.
*/
public resetFormStatus(): void {
this.urlInputView.errorText = null;
}

/**
* Creates a labeled input view.
*
Expand Down Expand Up @@ -328,8 +366,32 @@ export default class LinkFormView extends View {

return children;
}

/**
* The native DOM `value` of the {@link #urlInputView} element.
*
* **Note**: Do not confuse it with the {@link module:ui/inputtext/inputtextview~InputTextView#value}
* which works one way only and may not represent the actual state of the component in the DOM.
*/
public get url(): string | null {
const { element } = this.urlInputView.fieldView;

if ( !element ) {
return null;
}

return element.value.trim();
}
}

/**
* Callback used by {@link ~LinkFormView} to check if passed form value is valid.
*
* * If `undefined` is returned, it is assumed that the form value is correct and there is no error.
* * If string is returned, it is assumed that the form value is incorrect and the returned string is displayed in the error label
*/
export type LinkFormValidatorCallback = ( form: LinkFormView ) => string | undefined;

/**
* Fired when the form view is submitted (when one of the children triggered the submit event),
* for example with a click on {@link ~LinkFormView#saveButtonView}.
Expand Down
77 changes: 76 additions & 1 deletion packages/ckeditor5-link/tests/linkui.js
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,75 @@ describe( 'LinkUI', () => {
expect( balloon.view.pin.lastCall.args[ 0 ].target() ).to.equal( newLinkDomElement );
} );

describe( 'form status', () => {
it( 'should update ui on error due to change ballon position', () => {
const updateSpy = sinon.spy( editor.ui, 'update' );

linkUIFeature._createViews();
formView = linkUIFeature.formView;
actionsView = linkUIFeature.actionsView;
formView.render();

setModelData( editor.model, '<paragraph>[foo]</paragraph>' );

linkUIFeature._showUI();

expect( updateSpy ).not.to.be.called;
formView.fire( 'submit' );
expect( updateSpy ).to.be.calledOnce;
} );

it( 'should show error form status if passed empty link', () => {
linkUIFeature._createViews();
formView = linkUIFeature.formView;
actionsView = linkUIFeature.actionsView;
formView.render();

setModelData( editor.model, '<paragraph>[foo]</paragraph>' );
linkUIFeature._showUI();

formView.fire( 'submit' );

expect( formView.urlInputView.errorText ).to.be.equal( 'Link URL must not be empty.' );
} );

it( 'should reset error form status after filling empty link', () => {
linkUIFeature._createViews();
formView = linkUIFeature.formView;
actionsView = linkUIFeature.actionsView;
formView.render();

setModelData( editor.model, '<paragraph>[foo]</paragraph>' );

linkUIFeature._showUI();

formView.fire( 'submit' );
expect( formView.urlInputView.errorText ).to.be.equal( 'Link URL must not be empty.' );

formView.urlInputView.fieldView.value = 'http://cksource.com';
formView.fire( 'submit' );

expect( formView.urlInputView.errorText ).to.be.null;
} );

it( 'should reset form status on show', () => {
linkUIFeature._createViews();
formView = linkUIFeature.formView;
actionsView = linkUIFeature.actionsView;
formView.render();

setModelData( editor.model, '<paragraph>[foo]</paragraph>' );
linkUIFeature._showUI();

formView.fire( 'submit' );
expect( formView.urlInputView.errorText ).to.be.equal( 'Link URL must not be empty.' );

linkUIFeature._hideUI();
linkUIFeature._showUI();
expect( formView.urlInputView.errorText ).to.be.null;
} );
} );

describe( 'response to ui#update', () => {
let view, viewDocument;

Expand Down Expand Up @@ -1531,8 +1600,12 @@ describe( 'LinkUI', () => {

it( 'should not allow submitting empty form when link is required', () => {
return createEditorWithEmptyLinks( false ).then( ( { editor, formView } ) => {
expect( formView.saveButtonView.isEnabled ).to.be.false;
const executeSpy = sinon.spy( editor, 'execute' );

formView.urlInputView.fieldView.value = '';
formView.fire( 'submit' );

expect( executeSpy ).not.to.be.called;
return editor.destroy();
} );
} );
Expand Down Expand Up @@ -1732,6 +1805,8 @@ describe( 'LinkUI', () => {

it( 'should hide and reveal the #actionsView on formView#submit event', () => {
linkUIFeature._showUI();

formView.urlInputView.fieldView.value = '/test.html';
formView.fire( 'submit' );

expect( balloon.visibleView ).to.equal( actionsView );
Expand Down
53 changes: 53 additions & 0 deletions packages/ckeditor5-link/tests/ui/linkformview.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,45 @@ describe( 'LinkFormView', () => {
} );
} );

describe( 'isValid()', () => {
it( 'should reset error after successful validation', () => {
const view = new LinkFormView( { t: () => {} }, { manualDecorators: [] }, [
() => undefined
] );

expect( view.isValid() ).to.be.true;
expect( view.urlInputView.errorText ).to.be.null;
} );

it( 'should display first error returned from validators list', () => {
const view = new LinkFormView( { t: () => {} }, { manualDecorators: [] }, [
() => undefined,
() => 'Foo bar',
() => 'Another error'
] );

expect( view.isValid() ).to.be.false;
expect( view.urlInputView.errorText ).to.be.equal( 'Foo bar' );
} );

it( 'should pass view reference as argument to validator', () => {
const validatorSpy = sinon.spy();
const view = new LinkFormView( { t: () => {} }, { manualDecorators: [] }, [ validatorSpy ] );

view.isValid();

expect( validatorSpy ).to.be.calledOnceWithExactly( view );
} );
} );

describe( 'resetFormStatus()', () => {
it( 'should clear form input errors', () => {
view.urlInputView.errorText = 'Error';
view.resetFormStatus();
expect( view.urlInputView.errorText ).to.be.null;
} );
} );

describe( 'destroy()', () => {
it( 'should destroy the FocusTracker instance', () => {
const destroySpy = sinon.spy( view.focusTracker, 'destroy' );
Expand Down Expand Up @@ -214,6 +253,20 @@ describe( 'LinkFormView', () => {
} );
} );

describe( 'URL getter', () => {
it( 'null value should be returned in URL getter if element is null', () => {
view.urlInputView.fieldView.element = null;

expect( view.url ).to.be.equal( null );
} );

it( 'trimmed DOM input value should be returned in URL getter', () => {
view.urlInputView.fieldView.element.value = ' https://cksource.com/ ';

expect( view.url ).to.be.equal( 'https://cksource.com/' );
} );
} );

describe( 'manual decorators', () => {
let view, collection, linkCommand;

Expand Down
1 change: 1 addition & 0 deletions packages/ckeditor5-link/theme/linkform.css
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

.ck.ck-link-form {
display: flex;
align-items: flex-start;

& .ck-label {
display: none;
Expand Down
5 changes: 5 additions & 0 deletions packages/ckeditor5-media-embed/src/mediaembedui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,11 @@ export default class MediaEmbedUI extends Plugin {
form.delegate( 'submit', 'cancel' ).to( dropdown );
form.urlInputView.fieldView.bind( 'value' ).to( command, 'value' );

// Update balloon position when form error changes.
form.urlInputView.on( 'change:errorText', () => {
editor.ui.update();
} );

// Form elements should be read-only when corresponding commands are disabled.
form.urlInputView.bind( 'isEnabled' ).to( command, 'isEnabled' );
} );
Expand Down
Loading

0 comments on commit caea11e

Please sign in to comment.