Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Extracted nested editable styles and logic to widgets. #74

Merged
merged 8 commits into from
Mar 28, 2017
Merged

Extracted nested editable styles and logic to widgets. #74

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b0b4bc7
Extracted nested editable styles and logic to widgets.
szymonkups Mar 17, 2017
a80dff3
Renamed utils functions: widgetize to toWidget and createNestedEditab…
szymonkups Mar 20, 2017
7ef4895
CSS classes name refactoring in widgets.
szymonkups Mar 21, 2017
a426d5a
Widget util function toWidgetEditable now accepts element to have sim…
szymonkups Mar 21, 2017
3be8718
Fixed widget and caption styles.
szymonkups Mar 27, 2017
aeb276b
Added ck-editor__editable prefix to content styles.
szymonkups Mar 28, 2017
c025e49
Fixed indentation in theme file.
szymonkups Mar 28, 2017
d797993
Use data classes to style the content. Removed invalid comment.
Reinmar Mar 28, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/image/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@
* @module image/image/utils
*/

import { widgetize, isWidget } from '../widget/utils';
import { toWidget, isWidget } from '../widget/utils';
import ModelElement from '@ckeditor/ckeditor5-engine/src/model/element';

const imageSymbol = Symbol( 'isImage' );

/**
* Converts given {@link module:engine/view/element~Element} to image widget:
* * adds {@link module:engine/view/element~Element#setCustomProperty custom property} allowing to recognize image widget element,
* * calls {@link module:image/widget/utils~widgetize widgetize} function with proper element's label creator.
* * calls {@link module:image/widget/utils~toWidget toWidget} function with proper element's label creator.
*
* @param {module:engine/view/element~Element} viewElement
* @param {String} label Element's label. It will be concatenated with image's `alt` attribute if one is present.
Expand All @@ -24,7 +24,7 @@ const imageSymbol = Symbol( 'isImage' );
export function toImageWidget( viewElement, label ) {
viewElement.setCustomProperty( imageSymbol, true );

return widgetize( viewElement, { label: labelCreator } );
return toWidget( viewElement, { label: labelCreator } );

function labelCreator() {
const imgElement = viewElement.getChild( 0 );
Expand Down
15 changes: 4 additions & 11 deletions src/imagecaption/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@
* @module image/imagecaption/utils
*/

import ViewEditableElement from '@ckeditor/ckeditor5-engine/src/view/editableelement';
import ModelElement from '@ckeditor/ckeditor5-engine/src/model/element';
import ViewEditableElement from '@ckeditor/ckeditor5-engine/src/view/editableelement';
import { toWidgetEditable } from '../widget/utils';

const captionSymbol = Symbol( 'imageCaption' );

Expand All @@ -20,19 +21,11 @@ const captionSymbol = Symbol( 'imageCaption' );
*/
export function captionElementCreator( viewDocument ) {
return () => {
const editable = new ViewEditableElement( 'figcaption', { contenteditable: true } );
const editable = new ViewEditableElement( 'figcaption' );
editable.document = viewDocument;
editable.setCustomProperty( captionSymbol, true );

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

return editable;
return toWidgetEditable( editable );
};
}

Expand Down
28 changes: 26 additions & 2 deletions src/widget/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export function isWidget( element ) {
}

/**
* "Widgetizes" given {@link module:engine/view/element~Element}:
* Converts given {@link module:engine/view/element~Element} to widget in following way:
* * sets `contenteditable` attribute to `true`,
* * adds custom `getFillerOffset` method returning `null`,
* * adds `ck-widget` CSS class,
Expand All @@ -47,7 +47,7 @@ export function isWidget( element ) {
* a plain string or a function returning a string.
* @returns {module:engine/view/element~Element} Returns same element.
*/
export function widgetize( element, options ) {
export function toWidget( element, options ) {
options = options || {};
element.setAttribute( 'contenteditable', false );
element.getFillerOffset = getFillerOffset;
Expand Down Expand Up @@ -89,6 +89,30 @@ export function getLabel( element ) {
return typeof labelCreator == 'function' ? labelCreator() : labelCreator;
}

/**
* Adds functionality to provided {module:engine/view/editableelement~EditableElement} to act as a widget's editable:
* * sets `contenteditable` attribute to `true`,
* * adds `ck-editable` CSS class,
* * adds `ck-editable_focused` CSS class when editable is focused and removes it when it's blurred.
*
* @param {module:engine/view/editableelement~EditableElement} editable
* @returns {module:engine/view/editableelement~EditableElement} Returns same element that was provided in `editable` param.
*/
export function toWidgetEditable( editable ) {
editable.setAttribute( 'contenteditable', 'true' );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this and the following line should be done via Template.apply(). One of the reasons we decided to have a Template class was that DOM attrs should be touched via unified API.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not true. This isn't UI. This is the engine and the element here isn't a native DOM element. We've been talking about unifying the APIs, but that never happened and I guess won't happen anytime soon.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! I didn't notice that this isn't DOM. You're right, ofc ;-)

editable.addClass( 'ck-editable' );

editable.on( 'change:isFocused', ( evt, property, is ) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

new Template( {
    attributes: {
        contenteditable: true,
        class: [ 
                'ck-editable', 
                bind.if( editable, 'isFocused', 'ck-editable_focused' ) 
        ]
    }
} ).apply( editable );

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above – nope. This is not a native element.

if ( is ) {
editable.addClass( 'ck-editable_focused' );
} else {
editable.removeClass( 'ck-editable_focused' );
}
} );

return editable;
}

// Default filler offset function applied to all widget elements.
//
// @returns {null}
Expand Down
2 changes: 2 additions & 0 deletions src/widget/widget.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import RootEditableElement from '@ckeditor/ckeditor5-engine/src/view/rooteditabl
import { isWidget } from './utils';
import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard';

import '../../theme/widget/theme.scss';

/**
* The widget plugin.
* Adds default {@link module:engine/view/document~Document#event:mousedown mousedown} handling on widget elements.
Expand Down
18 changes: 9 additions & 9 deletions tests/imagecaption/imagecaptionengine.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ describe( 'ImageCaptionEngine', () => {
expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal(
'<figure class="image ck-widget" contenteditable="false">' +
'<img src="img.png"></img>' +
'<figcaption contenteditable="true">Foo bar baz.</figcaption>' +
'<figcaption class="ck-editable" contenteditable="true">Foo bar baz.</figcaption>' +
'</figure>'
);
} );
Expand Down Expand Up @@ -214,7 +214,7 @@ describe( 'ImageCaptionEngine', () => {
expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal(
'<figure class="image ck-widget" contenteditable="false">' +
'<img src=""></img>' +
'<figcaption contenteditable="true">foo bar baz</figcaption>' +
'<figcaption class="ck-editable" contenteditable="true">foo bar baz</figcaption>' +
'</figure>'
);
} );
Expand All @@ -227,7 +227,7 @@ describe( 'ImageCaptionEngine', () => {
expect( getViewData( viewDocument ) ).to.equal(
'[<figure class="image ck-widget ck-widget_selected" contenteditable="false">' +
'<img src=""></img>' +
'<figcaption contenteditable="true"></figcaption>' +
'<figcaption class="ck-editable" contenteditable="true"></figcaption>' +
'</figure>]'
);
} );
Expand All @@ -248,7 +248,7 @@ describe( 'ImageCaptionEngine', () => {
expect( getViewData( viewDocument ) ).to.equal(
'[<figure class="image ck-widget ck-widget_selected" contenteditable="false">' +
'<img src=""></img>' +
'<figcaption contenteditable="true">foo bar</figcaption>' +
'<figcaption class="ck-editable" contenteditable="true">foo bar</figcaption>' +
'</figure>]'
);
} );
Expand Down Expand Up @@ -277,7 +277,7 @@ describe( 'ImageCaptionEngine', () => {
expect( getViewData( viewDocument ) ).to.equal(
'<figure class="image ck-widget" contenteditable="false">' +
'<img src=""></img>' +
'<figcaption contenteditable="true">[]</figcaption>' +
'<figcaption class="ck-editable" contenteditable="true">[]</figcaption>' +
'</figure>'
);
} );
Expand All @@ -294,7 +294,7 @@ describe( 'ImageCaptionEngine', () => {
expect( getViewData( viewDocument ) ).to.equal(
'[<figure class="image ck-widget ck-widget_selected" contenteditable="false">' +
'<img src=""></img>' +
'<figcaption contenteditable="true"></figcaption>' +
'<figcaption class="ck-editable" contenteditable="true"></figcaption>' +
'</figure>]'
);
} );
Expand All @@ -310,11 +310,11 @@ describe( 'ImageCaptionEngine', () => {
expect( getViewData( viewDocument ) ).to.equal(
'<figure class="image ck-widget" contenteditable="false">' +
'<img src=""></img>' +
'<figcaption contenteditable="true">foo bar</figcaption>' +
'<figcaption class="ck-editable" contenteditable="true">foo bar</figcaption>' +
'</figure>' +
'[<figure class="image ck-widget ck-widget_selected" contenteditable="false">' +
'<img src=""></img>' +
'<figcaption contenteditable="true"></figcaption>' +
'<figcaption class="ck-editable" contenteditable="true"></figcaption>' +
'</figure>]'
);
} );
Expand Down Expand Up @@ -346,7 +346,7 @@ describe( 'ImageCaptionEngine', () => {
expect( getViewData( viewDocument ) ).to.equal(
'<figure class="image ck-widget" contenteditable="false">' +
'<img src=""></img>' +
'<figcaption contenteditable="true">{foo bar baz}</figcaption>' +
'<figcaption class="ck-editable" contenteditable="true">{foo bar baz}</figcaption>' +
'</figure>'
);
} );
Expand Down
12 changes: 0 additions & 12 deletions tests/imagecaption/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,6 @@ describe( 'image captioning utils', () => {
expect( element.name ).to.equal( 'figcaption' );
expect( isCaption( element ) ).to.be.true;
} );

it( 'should be created in context of proper document', () => {
expect( element.document ).to.equal( document );
} );

it( 'should add proper class when element is focused', () => {
element.isFocused = true;
expect( element.hasClass( 'focused' ) ).to.be.true;

element.isFocused = false;
expect( element.hasClass( 'focused' ) ).to.be.false;
} );
} );

describe( 'isCaptionEditable', () => {
Expand Down
46 changes: 41 additions & 5 deletions tests/widget/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,26 @@
*/

import ViewElement from '@ckeditor/ckeditor5-engine/src/view/element';
import { widgetize, isWidget, WIDGET_CLASS_NAME, setLabel, getLabel } from '../../src/widget/utils';
import ViewEditableElement from '@ckeditor/ckeditor5-engine/src/view/editableelement';
import ViewDocument from '@ckeditor/ckeditor5-engine/src/view/document';
import {
toWidget,
isWidget,
setLabel,
getLabel,
toWidgetEditable,
WIDGET_CLASS_NAME
} from '../../src/widget/utils';

describe( 'widget utils', () => {
let element;

beforeEach( () => {
element = new ViewElement( 'div' );
widgetize( element );
toWidget( element );
} );

describe( 'widgetize()', () => {
describe( 'toWidget()', () => {
it( 'should set contenteditable to false', () => {
expect( element.getAttribute( 'contenteditable' ) ).to.be.false;
} );
Expand All @@ -30,14 +39,14 @@ describe( 'widget utils', () => {

it( 'should add element\'s label if one is provided', () => {
element = new ViewElement( 'div' );
widgetize( element, { label: 'foo bar baz label' } );
toWidget( element, { label: 'foo bar baz label' } );

expect( getLabel( element ) ).to.equal( 'foo bar baz label' );
} );

it( 'should add element\'s label if one is provided as function', () => {
element = new ViewElement( 'div' );
widgetize( element, { label: () => 'foo bar baz label' } );
toWidget( element, { label: () => 'foo bar baz label' } );

expect( getLabel( element ) ).to.equal( 'foo bar baz label' );
} );
Expand Down Expand Up @@ -77,4 +86,31 @@ describe( 'widget utils', () => {
expect( getLabel( element ) ).to.equal( 'bar' );
} );
} );

describe( 'toWidgetEditable', () => {
let viewDocument, element;

beforeEach( () => {
viewDocument = new ViewDocument();
element = new ViewEditableElement( 'div' );
element.document = viewDocument;
toWidgetEditable( element );
} );

it( 'should be created in context of proper document', () => {
expect( element.document ).to.equal( viewDocument );
} );

it( 'should add proper class', () => {
expect( element.hasClass( 'ck-editable' ) ).to.be.true;
} );

it( 'should add proper class when element is focused', () => {
element.isFocused = true;
expect( element.hasClass( 'ck-editable_focused' ) ).to.be.true;

element.isFocused = false;
expect( element.hasClass( 'ck-editable_focused' ) ).to.be.false;
} );
} );
} );
4 changes: 2 additions & 2 deletions tests/widget/widget.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtest
import Widget from '../../src/widget/widget';
import MouseObserver from '@ckeditor/ckeditor5-engine/src/view/observer/mouseobserver';
import buildModelConverter from '@ckeditor/ckeditor5-engine/src/conversion/buildmodelconverter';
import { widgetize } from '../../src/widget/utils';
import { toWidget } from '../../src/widget/utils';
import ViewContainer from '@ckeditor/ckeditor5-engine/src/view/containerelement';
import ViewEditable from '@ckeditor/ckeditor5-engine/src/view/editableelement';
import DomEventData from '@ckeditor/ckeditor5-engine/src/view/observer/domeventdata';
Expand Down Expand Up @@ -48,7 +48,7 @@ describe( 'Widget', () => {
const b = new AttributeContainer( 'b' );
const div = new ViewContainer( 'div', null, b );

return widgetize( div );
return toWidget( div );
} );

buildModelConverter().for( editor.editing.modelToView )
Expand Down
4 changes: 2 additions & 2 deletions tests/widget/widgetengine.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-util
import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view';
import ViewContainer from '@ckeditor/ckeditor5-engine/src/view/containerelement';
import ViewEditable from '@ckeditor/ckeditor5-engine/src/view/editableelement';
import { widgetize } from '../../src/widget/utils';
import { toWidget } from '../../src/widget/utils';

describe( 'WidgetEngine', () => {
let editor, document, viewDocument;
Expand All @@ -32,7 +32,7 @@ describe( 'WidgetEngine', () => {
buildModelConverter().for( editor.editing.modelToView )
.fromElement( 'widget' )
.toElement( () => {
const element = widgetize( new ViewContainer( 'div' ), { label: 'element label' } );
const element = toWidget( new ViewContainer( 'div' ), { label: 'element label' } );

return element;
} );
Expand Down
29 changes: 9 additions & 20 deletions theme/imagecaption/theme.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,16 @@
// 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';
@import '~@ckeditor/ckeditor5-theme-lark/theme/helpers/_spacing.scss';
@import '~@ckeditor/ckeditor5-theme-lark/theme/helpers/_fonts.scss';

.ck-widget.image {
figcaption {
background-color: ck-color( 'foreground' );
padding: 10px;
font-size: .8em;
color: ck-color( 'text', 40 );

// 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' );;
.ck-editor__editable {
.image {
.ck-editable {
background-color: ck-color( 'foreground' );
padding: ck-spacing();
font-size: ck-font-size( -1 );
color: ck-color( 'text', 40 );
}
}
}
Loading