Browse Source

Merge pull request #14 from ckeditor/i/5834

Fix: Pressing "Tab" or "Shift+Tab" should move focus outside the editor in restricted mode if the selection is anchored in the last (or first) exception. Closes ckeditor/ckeditor5#5834.
Aleksander Nowodzinski 6 years ago
parent
commit
b02d84859b

+ 1 - 2
packages/ckeditor5-restricted-editing/src/restrictededitingmodeediting.js

@@ -303,9 +303,8 @@ function getCommandExecuter( editor, commandName ) {
 
 
 		if ( command.isEnabled ) {
 		if ( command.isEnabled ) {
 			editor.execute( commandName );
 			editor.execute( commandName );
+			cancel();
 		}
 		}
-
-		cancel();
 	};
 	};
 }
 }
 
 

+ 38 - 0
packages/ckeditor5-restricted-editing/tests/manual/restrictedediting-focus.html

@@ -0,0 +1,38 @@
+<p>
+	<b>Mode</b>:
+	<input type="radio" id="mode-standard" name="mode" value="standard" checked><label for="mode-standard">Standard</label>
+	<input type="radio" id="mode-restricted" name="mode" value="restricted"><label for="mode-restricted">Restricted</label>
+</p>
+
+<form style="margin: 20px 0;">
+	<input type="text" placeholder="Type your name..." value="Nick" />
+	<input type="text" placeholder="Type your surname..." value="Thomas" disabled />
+</form>
+
+<div id="editor">
+	<h2>Heading 1</h2>
+	<p>Paragraph <span class="restricted-editing-exception">it is editable</span></p>
+	<p>Exception on part of a word: <a href="ckeditor.com">coompi</a><span class="restricted-editing-exception"><a href="ckeditor.com">ter</a> (fix spelling in Firefox).</span></p>
+	<p><strong>Bold</strong> <i>Italic</i> <a href="foo">Link</a></p>
+	<ul>
+		<li>UL List item 1</li>
+		<li>UL List item 2</li>
+	</ul>
+	<ol>
+		<li><span class="restricted-editing-exception">OL List item 1</span></li>
+		<li>OL List item 2</li>
+	</ol>
+	<blockquote>
+		<p>Quote</p>
+		<ul>
+			<li>Quoted UL List item 1</li>
+			<li>Quoted UL List item <span class="restricted-editing-exception">2</span></li>
+		</ul>
+		<p>Quote</p>
+	</blockquote>
+</div>
+
+<form style="margin: 20px 0;">
+	<label><input type="radio" name="test" value="test1" checked />Test 1</label>
+	<label><input type="radio" name="test" value="test2" /> Test 2</label>
+</form>

+ 69 - 0
packages/ckeditor5-restricted-editing/tests/manual/restrictedediting-focus.js

@@ -0,0 +1,69 @@
+/**
+ * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
+ */
+
+/* globals window, document */
+
+import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor';
+import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset';
+import Table from '@ckeditor/ckeditor5-table/src/table';
+
+import StandardEditingMode from '../../src/standardeditingmode';
+import RestrictedEditingMode from '../../src/restrictededitingmode';
+
+const restrictedModeButton = document.getElementById( 'mode-restricted' );
+const standardModeButton = document.getElementById( 'mode-standard' );
+
+restrictedModeButton.addEventListener( 'change', handleModeChange );
+standardModeButton.addEventListener( 'change', handleModeChange );
+
+startMode( document.querySelector( 'input[name="mode"]:checked' ).value );
+
+async function handleModeChange( evt ) {
+	await startMode( evt.target.value );
+}
+
+async function startMode( selectedMode ) {
+	if ( selectedMode === 'standard' ) {
+		await startStandardEditingMode();
+	} else {
+		await startRestrictedEditingMode();
+	}
+}
+
+async function startStandardEditingMode() {
+	await reloadEditor( {
+		plugins: [ ArticlePluginSet, Table, StandardEditingMode ],
+		toolbar: [
+			'heading', '|', 'bold', 'italic', 'link', '|',
+			'bulletedList', 'numberedList', 'blockQuote', 'insertTable', '|',
+			'restrictedEditingException', '|', 'undo', 'redo'
+		],
+		image: {
+			toolbar: [ 'imageStyle:full', 'imageStyle:side', '|', 'imageTextAlternative' ]
+		},
+		table: {
+			contentToolbar: [
+				'tableColumn',
+				'tableRow',
+				'mergeTableCells'
+			]
+		}
+	} );
+}
+
+async function startRestrictedEditingMode() {
+	await reloadEditor( {
+		plugins: [ ArticlePluginSet, Table, RestrictedEditingMode ],
+		toolbar: [ 'bold', 'italic', 'link', '|', 'restrictedEditing', '|', 'undo', 'redo' ]
+	} );
+}
+
+async function reloadEditor( config ) {
+	if ( window.editor ) {
+		await window.editor.destroy();
+	}
+
+	window.editor = await ClassicEditor.create( document.querySelector( '#editor' ), config );
+}

+ 24 - 0
packages/ckeditor5-restricted-editing/tests/manual/restrictedediting-focus.md

@@ -0,0 +1,24 @@
+## Restricted editing - focus cycling in an editor surrounded by forms
+
+## Standard mode
+
+1. Make sure the "Standard" mode is selected.
+2. Focus the "Standard" radio button in the "Mode" form.
+3. Start cycling the focus using the <kbd>Tab</kbd> key.
+
+**Expected behaviour**:
+
+- The focus should cycle across:
+	1. The "Mode" form,
+	2. The editor,
+	3. The radio form below the editor,
+- Pressing <kbd>Shift</kbd>+<kbd>Tab</kbd> should cycle focus in the opposite direction.
+
+## Restricted mode
+
+1. Switch to the "Restricted" mode.
+
+**Expected behaviour**:
+
+- When selection is in the **first** restricted exception, pressing <kbd>Shift</kbd>+<kbd>Tab</kbd> should set focus in the text input with a "value" ("Nick") above the editor
+- When selection is in the **last** restricted exception, pressing <kbd>Tab</kbd> should set focus in the radio input with a "label" ("Test 1") below the editor.

+ 6 - 6
packages/ckeditor5-restricted-editing/tests/restrictededitingmodeediting.js

@@ -1302,7 +1302,7 @@ describe( 'RestrictedEditingModeEditing', () => {
 			sinon.assert.calledOnce( domEvtDataStub.stopPropagation );
 			sinon.assert.calledOnce( domEvtDataStub.stopPropagation );
 		} );
 		} );
 
 
-		it( 'should not move to the closest next exception on tab key when there is none', () => {
+		it( 'should let the focus go outside the editor on tab key when in the last exception', () => {
 			setModelData( model, '<paragraph>foo qux[]</paragraph>' );
 			setModelData( model, '<paragraph>foo qux[]</paragraph>' );
 
 
 			const paragraph = model.document.getRoot().getChild( 0 );
 			const paragraph = model.document.getRoot().getChild( 0 );
@@ -1319,8 +1319,8 @@ describe( 'RestrictedEditingModeEditing', () => {
 			view.document.fire( 'keydown', domEvtDataStub );
 			view.document.fire( 'keydown', domEvtDataStub );
 
 
 			sinon.assert.notCalled( editor.execute );
 			sinon.assert.notCalled( editor.execute );
-			sinon.assert.calledOnce( domEvtDataStub.preventDefault );
-			sinon.assert.calledOnce( domEvtDataStub.stopPropagation );
+			sinon.assert.notCalled( domEvtDataStub.preventDefault );
+			sinon.assert.notCalled( domEvtDataStub.stopPropagation );
 		} );
 		} );
 
 
 		it( 'should move to the closest previous exception on shift+tab key', () => {
 		it( 'should move to the closest previous exception on shift+tab key', () => {
@@ -1355,7 +1355,7 @@ describe( 'RestrictedEditingModeEditing', () => {
 			sinon.assert.calledOnce( domEvtDataStub.stopPropagation );
 			sinon.assert.calledOnce( domEvtDataStub.stopPropagation );
 		} );
 		} );
 
 
-		it( 'should not move to the closest previous exception on shift+tab key when there is none', () => {
+		it( 'should let the focus go outside the editor on shift+tab when in the first exception', () => {
 			setModelData( model, '<paragraph>[]foo qux</paragraph>' );
 			setModelData( model, '<paragraph>[]foo qux</paragraph>' );
 
 
 			const paragraph = model.document.getRoot().getChild( 0 );
 			const paragraph = model.document.getRoot().getChild( 0 );
@@ -1373,8 +1373,8 @@ describe( 'RestrictedEditingModeEditing', () => {
 			view.document.fire( 'keydown', domEvtDataStub );
 			view.document.fire( 'keydown', domEvtDataStub );
 
 
 			sinon.assert.notCalled( editor.execute );
 			sinon.assert.notCalled( editor.execute );
-			sinon.assert.calledOnce( domEvtDataStub.preventDefault );
-			sinon.assert.calledOnce( domEvtDataStub.stopPropagation );
+			sinon.assert.notCalled( domEvtDataStub.preventDefault );
+			sinon.assert.notCalled( domEvtDataStub.stopPropagation );
 		} );
 		} );
 	} );
 	} );