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

Commit

Permalink
Merge branch 'master' into i/6060-cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
@jodator
jodator committed Apr 21, 2020
2 parents 32e18a4 + 6273527 commit 5991e03
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 157 deletions.
23 changes: 7 additions & 16 deletions src/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

import HighlightStack from './highlightstack';
import IconView from '@ckeditor/ckeditor5-ui/src/icon/iconview';
import env from '@ckeditor/ckeditor5-utils/src/env';

import dragHandleIcon from '../theme/icons/drag-handle.svg';

Expand Down Expand Up @@ -90,11 +89,7 @@ export function isWidget( node ) {
*/
/* eslint-enable max-len */
export function toWidget( element, writer, options = {} ) {
// The selection on Edge behaves better when the whole editor contents is in a single contenteditable element.
// https://github.com/ckeditor/ckeditor5/issues/1079
if ( !env.isEdge ) {
writer.setAttribute( 'contenteditable', 'false', element );
}
writer.setAttribute( 'contenteditable', 'false', element );

writer.addClass( WIDGET_CLASS_NAME, element );
writer.setCustomProperty( 'widget', true, element );
Expand Down Expand Up @@ -220,17 +215,13 @@ export function getLabel( element ) {
export function toWidgetEditable( editable, writer ) {
writer.addClass( [ 'ck-editor__editable', 'ck-editor__nested-editable' ], editable );

// The selection on Edge behaves better when the whole editor contents is in a single contentedible element.
// https://github.com/ckeditor/ckeditor5/issues/1079
if ( !env.isEdge ) {
// Set initial contenteditable value.
writer.setAttribute( 'contenteditable', editable.isReadOnly ? 'false' : 'true', editable );
// Set initial contenteditable value.
writer.setAttribute( 'contenteditable', editable.isReadOnly ? 'false' : 'true', editable );

// Bind the contenteditable property to element#isReadOnly.
editable.on( 'change:isReadOnly', ( evt, property, is ) => {
writer.setAttribute( 'contenteditable', is ? 'false' : 'true', editable );
} );
}
// Bind the contenteditable property to element#isReadOnly.
editable.on( 'change:isReadOnly', ( evt, property, is ) => {
writer.setAttribute( 'contenteditable', is ? 'false' : 'true', editable );
} );

editable.on( 'change:isFocused', ( evt, property, is ) => {
if ( is ) {
Expand Down
68 changes: 1 addition & 67 deletions src/widget.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,11 @@
import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
import MouseObserver from '@ckeditor/ckeditor5-engine/src/view/observer/mouseobserver';
import { getLabel, isWidget, WIDGET_SELECTED_CLASS_NAME } from './utils';
import { getCode, keyCodes, parseKeystroke } from '@ckeditor/ckeditor5-utils/src/keyboard';
import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard';
import env from '@ckeditor/ckeditor5-utils/src/env';

import '../theme/widget.css';

const selectAllKeystrokeCode = parseKeystroke( 'Ctrl+A' );

/**
* The widget plugin. It enables base support for widgets.
*
Expand Down Expand Up @@ -172,8 +170,6 @@ export default class Widget extends Plugin {
// the propagation.
if ( isArrowKeyCode( keyCode ) ) {
wasHandled = this._handleArrowKeys( isForward );
} else if ( isSelectAllKeyCode( domEventData ) ) {
wasHandled = this._selectAllNestedEditableContent() || this._selectAllContent();
} else if ( keyCode === keyCodes.enter ) {
wasHandled = this._handleEnterKey( domEventData.shiftKey );
}
Expand Down Expand Up @@ -306,60 +302,6 @@ export default class Widget extends Plugin {
}
}

/**
* Extends the {@link module:engine/model/selection~Selection document's selection} to span the entire
* content of the nested editable if already anchored in one.
*
* See: {@link module:engine/model/schema~Schema#getLimitElement}.
*
* @private
*/
_selectAllNestedEditableContent() {
const model = this.editor.model;
const documentSelection = model.document.selection;
const limitElement = model.schema.getLimitElement( documentSelection );

if ( documentSelection.getFirstRange().root == limitElement ) {
return false;
}

model.change( writer => {
writer.setSelection( writer.createRangeIn( limitElement ) );
} );

return true;
}

/**
* Handles <kbd>CTRL + A</kbd> when widget is selected.
*
* @private
* @returns {Boolean} Returns true if widget was selected and selecting all was handled by this method.
*/
_selectAllContent() {
const model = this.editor.model;
const editing = this.editor.editing;
const view = editing.view;
const viewDocument = view.document;
const viewSelection = viewDocument.selection;

const selectedElement = viewSelection.getSelectedElement();

// Only widget is selected.
// https://github.com/ckeditor/ckeditor5-widget/issues/23
if ( selectedElement && isWidget( selectedElement ) ) {
const widgetParent = editing.mapper.toModelElement( selectedElement.parent );

model.change( writer => {
writer.setSelection( writer.createRangeIn( widgetParent ) );
} );

return true;
}

return false;
}

/**
* Sets {@link module:engine/model/selection~Selection document's selection} over given element.
*
Expand Down Expand Up @@ -425,14 +367,6 @@ function isArrowKeyCode( keyCode ) {
keyCode == keyCodes.arrowdown;
}

// Returns 'true' if provided (DOM) key event data corresponds with the Ctrl+A keystroke.
//
// @param {module:engine/view/observer/keyobserver~KeyEventData} domEventData
// @returns {Boolean}
function isSelectAllKeyCode( domEventData ) {
return getCode( domEventData ) == selectAllKeystrokeCode;
}

// Returns `true` when element is a nested editable or is placed inside one.
//
// @param {module:engine/view/element~Element}
Expand Down
43 changes: 0 additions & 43 deletions tests/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import {
WIDGET_CLASS_NAME
} from '../src/utils';
import UIElement from '@ckeditor/ckeditor5-engine/src/view/uielement';
import env from '@ckeditor/ckeditor5-utils/src/env';
import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
import Model from '@ckeditor/ckeditor5-engine/src/model/model';
import { setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
Expand All @@ -37,9 +36,6 @@ describe( 'widget utils', () => {
testUtils.createSinonSandbox();

beforeEach( () => {
// Most tests assume non-edge environment but we do not set `contenteditable=false` on Edge so stub `env.isEdge`.
testUtils.sinon.stub( env, 'isEdge' ).get( () => false );

viewDocument = new ViewDocument();
writer = new DowncastWriter( viewDocument );

Expand Down Expand Up @@ -126,19 +122,6 @@ describe( 'widget utils', () => {
expect( icon.classList.contains( 'ck' ) ).to.be.true;
expect( icon.classList.contains( 'ck-icon' ) ).to.be.true;
} );

describe( 'on Edge', () => {
beforeEach( () => {
testUtils.sinon.stub( env, 'isEdge' ).get( () => true );

element = writer.createContainerElement( 'div' );
toWidget( element, writer );
} );

it( 'should not set contenteditable onEdge', () => {
expect( element.getAttribute( 'contenteditable' ) ).to.be.undefined;
} );
} );
} );

describe( 'isWidget()', () => {
Expand Down Expand Up @@ -220,32 +203,6 @@ describe( 'widget utils', () => {
element.isFocused = false;
expect( element.hasClass( 'ck-editor__nested-editable_focused' ) ).to.be.false;
} );

describe( 'on Edge', () => {
beforeEach( () => {
testUtils.sinon.stub( env, 'isEdge' ).get( () => true );

viewDocument = new ViewDocument();
element = new ViewEditableElement( viewDocument, 'div' );
toWidgetEditable( element, writer );
} );

it( 'should add contenteditable attribute when element is read-only - initialization', () => {
const element = new ViewEditableElement( viewDocument, 'div' );
element.isReadOnly = true;
toWidgetEditable( element, writer );

expect( element.getAttribute( 'contenteditable' ) ).to.be.undefined;
} );

it( 'should add contenteditable attribute when element is read-only - when changing', () => {
element.isReadOnly = true;
expect( element.getAttribute( 'contenteditable' ) ).to.be.undefined;

element.isReadOnly = false;
expect( element.getAttribute( 'contenteditable' ) ).to.be.undefined;
} );
} );
} );

describe( 'addHighlightHandling()', () => {
Expand Down
2 changes: 0 additions & 2 deletions tests/widget-integration.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ describe( 'Widget - integration', () => {
testUtils.createSinonSandbox();

beforeEach( () => {
// Most tests assume non-edge environment but we do not set `contenteditable=false` on Edge so stub `env.isEdge`.
testUtils.sinon.stub( env, 'isEdge' ).get( () => false );
testUtils.sinon.stub( env, 'isSafari' ).get( () => true );

editorElement = document.createElement( 'div' );
Expand Down
29 changes: 0 additions & 29 deletions tests/widget.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import DomEventData from '@ckeditor/ckeditor5-engine/src/view/observer/domeventd
import { setData as setModelData, getData as getModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view';
import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard';
import env from '@ckeditor/ckeditor5-utils/src/env';
import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';

/* global document */
Expand All @@ -23,9 +22,6 @@ describe( 'Widget', () => {
testUtils.createSinonSandbox();

beforeEach( () => {
// Most tests assume non-edge environment but we do not set `contenteditable=false` on Edge so stub `env.isEdge`.
testUtils.sinon.stub( env, 'isEdge' ).get( () => false );

return VirtualTestEditor.create( { plugins: [ Widget, Typing ] } )
.then( newEditor => {
editor = newEditor;
Expand Down Expand Up @@ -697,31 +693,6 @@ describe( 'Widget', () => {
} );
} );

describe( 'Ctrl+A', () => {
test(
'should select the entire content of the nested editable',
'<widget><nested>foo[]</nested></widget><paragraph>bar</paragraph>',
{ keyCode: keyCodes.a, ctrlKey: true },
'<widget><nested>[foo]</nested></widget><paragraph>bar</paragraph>'
);

test(
'should not change the selection if outside of the nested editable',
'<widget><nested>foo</nested></widget><paragraph>[]bar</paragraph>',
{ keyCode: keyCodes.a, ctrlKey: true },
'<widget><nested>foo</nested></widget><paragraph>[]bar</paragraph>'
);

test(
'should selected whole content when widget is selected',
'<paragraph>foo</paragraph>[<widget></widget>]<paragraph>bar</paragraph>',
{ keyCode: keyCodes.a, ctrlKey: true },
'<paragraph>[foo</paragraph><widget></widget><paragraph>bar]</paragraph>',
'<p>{foo</p><div class="ck-widget ck-widget_selected" contenteditable="false"><b></b></div><p>bar}</p>'

);
} );

describe( 'enter', () => {
test(
'should insert a paragraph after the selected widget upon Enter',
Expand Down

0 comments on commit 5991e03

Please sign in to comment.