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

Background Color button does not recognize active styles using custom class and colorName #4031

Closed
f1ames opened this issue May 7, 2020 · 2 comments · Fixed by #4063
Closed
Assignees
Labels
plugin:colorbutton The plugin which probably causes the issue. status:confirmed An issue confirmed by the development team. type:bug A bug.

Comments

@f1ames
Copy link
Contributor

f1ames commented May 7, 2020

Type of report

Bug

Provide detailed reproduction steps (if any)

Follow-up of #3940 (and #3977 PR).

  1. Go to tests/plugins/colorbutton/manual/customclassbackground.html.
  2. Follow the scenario to apply custom background color.
  3. Focus text where background color was applied.

Expected result

Background Color button changes its state to active.

Actual result

Background Color button stays inactive.

Other details

It seems to be related to specific config:

colorButton_backStyle: {
    element: 'span',
    attributes: { 'class': '#(colorName)' },
    overrides: [ {
        element: 'span',
        attributes: {
            'class': /text/
        }
    } ]
}

and the fact that background color transformations doesn't take such styling into account probably:

if ( normalizeBackground === undefined || normalizeBackground ) {
// If background contains only color, then we want to convert it into background-color so that it's
// correctly picked by colorbutton plugin.
contentTransformations = [
[
{
// Transform span that specify background with color only to background-color.
element: 'span',
left: function( element ) {
var tools = CKEDITOR.tools;
if ( element.name != 'span' || !element.styles || !element.styles.background ) {
return false;
}
var background = tools.style.parse.background( element.styles.background );
// We return true only if background specifies **only** color property, and there's only one background directive.
return background.color && tools.object.keys( background ).length === 1;
},
right: function( element ) {
var style = new CKEDITOR.style( editor.config.colorButton_backStyle, {
color: element.styles.background
} ),
definition = style.getDefinition();
// Align the output object with the template used in config.
element.name = definition.element;
element.styles = definition.styles;
element.attributes = definition.attributes || {};
return element;
}
}
]
];
}

@f1ames f1ames added type:bug A bug. plugin:colorbutton The plugin which probably causes the issue. status:confirmed An issue confirmed by the development team. workload:medium labels May 7, 2020
@hub33k hub33k self-assigned this May 18, 2020
@hub33k
Copy link
Contributor

hub33k commented May 19, 2020

@f1ames, after doing some research I think that the problem is in colorButton_backStyle option in overrides.attributes property. When we remove overrides.attributes or change regex /text/ to eg. /bg-/ it works good.

Here is config we use in customclassbackground.html:

CKEDITOR.addCss( '.bg-red { background-color: #F00; } .bg-blue { background-color: #00F; }' );
CKEDITOR.replace( 'editor', {
	colorButton_colors: 'bg-red/F00,bg-blue/00F',
	colorButton_backStyle: {
		element: 'span',
		attributes: { 'class': '#(colorName)' },
		overrides: [ {
			element: 'span',
			attributes: {
				'class': /text/
			}
		} ]
	},
	// Config necessary due to #3994 and #3996
	extraAllowedContent: 'span(*)',
	removeButtons: 'TextColor'
} );

@f1ames
Copy link
Contributor Author

f1ames commented May 19, 2020

@hub33k ok, so it means this is just misconfiguration here? Could you prepare a PR with updated https://github.com/ckeditor/ckeditor4/blob/major/tests/plugins/colorbutton/manual/customclassbackground.html manual test with proper configuration?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin:colorbutton The plugin which probably causes the issue. status:confirmed An issue confirmed by the development team. type:bug A bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants