Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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
# for free to join this conversation on GitHub. Already have an account? # 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