Skip to content

Commit

Permalink
Fixed list properties input behaviour when invalid values typed (#15851)
Browse files Browse the repository at this point in the history
Fix (list): An error message will be displayed when numbered list start index input field has an incorrect value. Closes #14939.
Internal (theme-lark): Input field label will no longer be displayed like a "placeholder value" if the input displays an error message.
  • Loading branch information
map-r committed Feb 21, 2024
1 parent 048a6ce commit 6fa4a52
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 2 deletions.
1 change: 1 addition & 0 deletions packages/ckeditor5-list/lang/contexts.json
Expand Up @@ -24,6 +24,7 @@
"Upper-latin": "The tooltip text of the button that toggles the \"upper–latin\" list style.",
"List properties": "The label of the button that toggles the visibility of additional numbered list property UI fields.",
"Start at": "The label of the input allowing to change the start index of a numbered list.",
"Invalid start index value.": "The error message displayed when the numbered list start index input value is not a valid number.",
"Start index must be greater than 0.": "The error message displayed when the numbered list start index input value is invalid.",
"Reversed order": "The label of the switch button that reverses the order of the numbered list."
}
Expand Up @@ -366,6 +366,10 @@ export default class ListPropertiesView extends View {
const startIndex = inputElement.valueAsNumber;

if ( Number.isNaN( startIndex ) ) {
// Number inputs allow for the entry of characters that may result in NaN,
// such as 'e', '+', '123e', '2-'.
startIndexFieldView.errorText = t( 'Invalid start index value.' );

return;
}

Expand Down
Expand Up @@ -698,7 +698,7 @@ describe( 'ListPropertiesView', () => {
sinon.assert.notCalled( spy );
} );

it( 'should not fire #listStart upon #input but display an errir if the field is invalid', () => {
it( 'should not fire #listStart upon #input but display an error if the field is invalid', () => {
const spy = sinon.spy();
view.on( 'listStart', spy );

Expand All @@ -708,6 +708,36 @@ describe( 'ListPropertiesView', () => {
sinon.assert.notCalled( spy );
expect( view.startIndexFieldView.errorText ).to.equal( 'Start index must be greater than 0.' );
} );

it( 'should not fire #listStart upon #input but display an error if the numeric value is NaN', () => {
const spy = sinon.spy();
view.on( 'listStart', spy );

view.startIndexFieldView.fieldView.value = '3e';
view.startIndexFieldView.fieldView.fire( 'input' );

sinon.assert.notCalled( spy );
expect( view.startIndexFieldView.errorText ).to.equal( 'Invalid start index value.' );
} );

it( 'should hide an error and proceed to fire #listStart when previously invalid value gets corrected', () => {
const spy = sinon.spy();
view.on( 'listStart', spy );

// Check for error.
view.startIndexFieldView.fieldView.value = '3e';
view.startIndexFieldView.fieldView.fire( 'input' );

sinon.assert.notCalled( spy );
expect( view.startIndexFieldView.errorText ).to.equal( 'Invalid start index value.' );

// And revert to valid state (clear error).
view.startIndexFieldView.fieldView.value = '32';
view.startIndexFieldView.fieldView.fire( 'input' );

sinon.assert.calledOnce( spy );
expect( view.startIndexFieldView.errorText ).to.be.null;
} );
} );

describe( '#reversedSwitchButtonView', () => {
Expand Down
Expand Up @@ -88,7 +88,7 @@
/* Fields that are disabled or not focused and without a placeholder should have full-sized labels. */
/* stylelint-disable-next-line no-descending-specificity */
&.ck-disabled.ck-labeled-field-view_empty:not(.ck-labeled-field-view_placeholder) > .ck.ck-labeled-field-view__input-wrapper > .ck.ck-label,
&.ck-labeled-field-view_empty:not(.ck-labeled-field-view_focused):not(.ck-labeled-field-view_placeholder) > .ck.ck-labeled-field-view__input-wrapper > .ck.ck-label {
&.ck-labeled-field-view_empty:not(.ck-labeled-field-view_focused):not(.ck-labeled-field-view_placeholder):not(.ck-error) > .ck.ck-labeled-field-view__input-wrapper > .ck.ck-label {
@mixin ck-dir ltr {
transform: translate(var(--ck-labeled-field-label-default-position-x), var(--ck-labeled-field-label-default-position-y)) scale(1);
}
Expand Down

0 comments on commit 6fa4a52

Please sign in to comment.