Skip to content

Commit

Permalink
Merge pull request #6801 from ckeditor/i/6791-color-normalize
Browse files Browse the repository at this point in the history
Fix (table): When the state is restored or the user enters color value manually, color input is able to provide a matching color label. Closes #6791.
  • Loading branch information
jodator authored May 15, 2020
2 parents 1b42639 + c5c4957 commit f18f4fd
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 4 deletions.
2 changes: 1 addition & 1 deletion packages/ckeditor5-font/tests/ui/colorui.js
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ describe( 'ColorUI', () => {
label: '#000'
},
{
color: 'rgb(255,255,255)',
color: 'rgb(255, 255, 255)',
label: 'Biały'
},
{
Expand Down
20 changes: 19 additions & 1 deletion packages/ckeditor5-table/src/ui/colorinputview.js
Original file line number Diff line number Diff line change
Expand Up @@ -294,8 +294,9 @@ export default class ColorInputView extends View {
*/
_setInputValue( inputValue ) {
if ( !this._stillTyping ) {
const normalizedInputValue = normalizeColor( inputValue );
// Check if the value matches one of our defined colors.
const mappedColor = this.options.colorDefinitions.find( def => inputValue === def.color );
const mappedColor = this.options.colorDefinitions.find( def => normalizedInputValue === normalizeColor( def.color ) );

if ( mappedColor ) {
this._inputView.value = mappedColor.label;
Expand All @@ -305,3 +306,20 @@ export default class ColorInputView extends View {
}
}
}

// Normalizes color value, by stripping extensive whitespace.
// For example., transforms:
// * ` rgb( 25 50 0 )` to `rgb(25 50 0)`,
// * "\t rgb( 25 , 50,0 ) " to `rgb(25 50 0)`.
//
// @param {String} colorString The value to be normalized.
// @returns {String}
function normalizeColor( colorString ) {
return colorString
// Remove any whitespace right after `(` or `,`.
.replace( /([(,])\s+/g, '$1' )
// Remove any whitespace at the beginning or right before the end, `)`, `,`, or another whitespace.
.replace( /^\s+|\s+(?=[),\s]|$)/g, '' )
// Then, replace `,` or whitespace with a single space.
.replace( /,|\s/g, ' ' );
}
20 changes: 20 additions & 0 deletions packages/ckeditor5-table/tests/ui/colorinputview.js
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,26 @@ describe( 'ColorInputView', () => {
expect( inputView.value ).to.equal( 'bar' );
} );

it(
`when the color input value is set to one of defined colors, but with few additional white spaces,
should use its label as the text input value`,
() => {
view.value = 'rgb(0, 255, 0)';
expect( inputView.value ).to.equal( 'Green' );

view.value = ' rgb( 255 0 0) ';
expect( inputView.value ).to.equal( 'Red' );

view.value = ' rgb(0, 0, 255 )';
expect( inputView.value ).to.equal( 'Blue' );

// Blindly stripping spaces may not work.
// rgb(25 50 0) != rgb(255 0 0)
view.value = ' rgb(25 50 0)';
expect( inputView.value ).to.equal( ' rgb(25 50 0)' );
}
);

it( `when the color input value is set to one of defined colors,
should use its label as the text input value`, () => {
view.value = 'rgb(0,255,0)';
Expand Down
7 changes: 5 additions & 2 deletions packages/ckeditor5-ui/src/colorgrid/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,16 @@ export function normalizeColorOptions( options ) {
}

// Creates a normalized color definition from the user-defined configuration.
// The "normalization" means it will create full
// {@link module:ui/colorgrid/colorgrid~ColorDefinition `ColorDefinition-like`}
// object for string values, and add a `view` property, for each definition.
//
// @param {String|module:ui/colorgrid/colorgrid~ColorDefinition}
// @returns {module:ui/colorgrid/colorgrid~ColorDefinition}
export function normalizeSingleColorDefinition( color ) {
if ( typeof color === 'string' ) {
return {
model: color.replace( / /g, '' ),
model: color,
label: color,
hasBorder: false,
view: {
Expand All @@ -82,7 +85,7 @@ export function normalizeSingleColorDefinition( color ) {
};
} else {
return {
model: color.color.replace( / /g, '' ),
model: color.color,
label: color.label || color.color,
hasBorder: color.hasBorder === undefined ? false : color.hasBorder,
view: {
Expand Down

0 comments on commit f18f4fd

Please sign in to comment.