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

Nested editables must have their classes #5067

Closed
Reinmar opened this issue Feb 21, 2017 · 1 comment · Fixed by ckeditor/ckeditor5-image#74
Closed

Nested editables must have their classes #5067

Reinmar opened this issue Feb 21, 2017 · 1 comment · Fixed by ckeditor/ckeditor5-image#74
Labels
package:image type:improvement This issue reports a possible enhancement of an existing feature.
Milestone

Comments

@Reinmar
Copy link
Member

Reinmar commented Feb 21, 2017

In ckeditor/ckeditor5-image#48 we introduced first nested editables, but they are styled as:

 +// Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved.
 +// For licensing, see LICENSE.md or http://ckeditor.com/license
 +
 +@import '~@ckeditor/ckeditor5-theme-lark/theme/helpers/_colors.scss';
 +@import '~@ckeditor/ckeditor5-theme-lark/theme/helpers/_shadow.scss';
 +@import '~@ckeditor/ckeditor5-theme-lark/theme/helpers/_states.scss';
 +
 +.ck-widget.image {
 +	figcaption {
 +		background-color: ck-color( 'foreground' );
 +		padding: 10px;
 +
 +		// The `:focus` styles is applied before `.focused` class inside editables.
 +		// These styles show different border for a blink of an eye.
 +		&:focus {
 +			outline: none;
 +			box-shadow: none;
 +		}
 +
 +		&.focused {
 +			@include ck-focus-ring( 'outline' );
 +			@include ck-box-shadow( $ck-inner-shadow );
 +			background-color: ck-color( 'background' );;
 +		}
 +
 +	}
 +}

This is incorrect – part of these styles should be image styles and part of them should be generic widget styles. In order to style widget's nested editables in a generic way, we need to be able to identify nested editables in the DOM, hence, they need to be marked with classes.

@Reinmar
Copy link
Member Author

Reinmar commented Mar 3, 2017

This is the second part of the code which needs to be generalised as nested editable's functionality:

		editable.on( 'change:isFocused', ( evt, property, is ) => {
			if ( is ) {
				editable.addClass( 'focused' );
			} else {
				editable.removeClass( 'focused' );
			}
		} );

@szymonkups szymonkups self-assigned this Mar 17, 2017
Reinmar referenced this issue in ckeditor/ckeditor5-image Mar 28, 2017
Feature: Introduced `toWidgetEditable()`. Closes #57.

The styling and behavior of image's caption will now be reusable in other widgets.

BREAKING CHANGE: Widget utility function `widgetize()` was renamed to `toWidget()`.
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-image Oct 9, 2019
@mlewand mlewand added this to the iteration 9 milestone Oct 9, 2019
@mlewand mlewand added type:improvement This issue reports a possible enhancement of an existing feature. package:image labels Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:image type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants