Skip to content

Commit

Permalink
Internal: Dropdown will move focus to the button after an option is p…
Browse files Browse the repository at this point in the history
…icked from the list. Closes #12125. Closes #12121.
  • Loading branch information
mlewand authored Jul 28, 2022
2 parents 9d58eb2 + eccc841 commit 6fb6303
Show file tree
Hide file tree
Showing 10 changed files with 89 additions and 12 deletions.
28 changes: 25 additions & 3 deletions docs/updating/migration-to-35.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ Listed below are the most important changes that require your attention when upg

### The source element is not updated automatically after the editor destroy

The last version of CKEditor 5 changes the default behaviour of a source element after the editor destroy. So far, the source element was updated with the output coming from the data pipeline. Now, the source element becomes empty after destroying the editor and it is not updated anymore.
The last version of CKEditor 5 changes the default behaviour of a source element after the editor destroy. So far, the source element was updated with the output coming from the data pipeline. Now, the source element becomes empty after destroying the editor and it is not updated anymore.

However, this behaviour is configurable and could be enabled with the {@link module:core/editor/editorconfig~EditorConfig#updateSourceElementOnDestroy `updateSourceElementOnDestroy`} configuration option:

Expand All @@ -33,5 +33,27 @@ ClassicEditor.create( document.querySelector( '#editor' ), {
```

<info-box warning>
Enabling the `updateSourceElementOnDestroy` option in your configuration, depending on plugins you use, might have some security implications. While the editing view is secured, there might be some unsafe content in the data output, so enable this option only if you know what you are doing. Especially, be careful when using Markdown, General HTML Support and HTML embed features
</info-box>
Enabling the `updateSourceElementOnDestroy` option in your configuration, depending on plugins you use, might have some security implications. While the editing view is secured, there might be some unsafe content in the data output, so enable this option only if you know what you are doing. Especially, be careful when using Markdown, General HTML Support and HTML embed features.
</info-box>

### Dropdown focus is moved back to the dropdown button after choosing an option

Due to the ongoing accessibility improvements the default behaviour of {@link module:ui/dropdown/dropdownview~DropdownView dropdown UI component} was changed. From now on, after choosing an option from (any) dropdown (either by mouse or keyboard), the focus will be moved to the dropdown button. This means that custom UI components using dropdowns which need to move focus back to the editable after execution have to do it explicitly, using the listener on the dropdown's {@link module:ui/dropdown/dropdownview~DropdownView#event:execute `execute` event}, e.g.:

```js
// If `execute` event is delegated to the dropdown, one listener can handle both executing
// the command and focusing the editor editable.
dropdownView.on( 'execute', () => {
// editor.execute()...;
editor.editing.view.focus();
} );

// Otherwise, a dedicated listener may needed to be added.
differentUiComponent.on( 'execute', () => {
// editor.execute();
} );

dropdownView.on( 'execute', () => {
editor.editing.view.focus();
} );
```
6 changes: 6 additions & 0 deletions packages/ckeditor5-alignment/src/alignmentui.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,12 @@ export default class AlignmentUI extends Plugin {
// Enable button if any of the buttons is enabled.
dropdownView.bind( 'isEnabled' ).toMany( buttons, 'isEnabled', ( ...areEnabled ) => areEnabled.some( isEnabled => isEnabled ) );

// Focus the editable after executing the command.
// Overrides a default behaviour where the focus is moved to the dropdown button (#12125).
this.listenTo( dropdownView, 'execute', () => {
editor.editing.view.focus();
} );

return dropdownView;
} );
}
Expand Down
12 changes: 12 additions & 0 deletions packages/ckeditor5-alignment/tests/alignmentui.js
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,18 @@ describe( 'Alignment UI', () => {
sinon.assert.calledOnce( spy );
} );

it( 'should return focus to editable after executing a command', () => {
const buttonAlignLeft = dropdown.toolbarView.items.get( 0 );
const spy = sinon.spy( editor.editing.view, 'focus' );
dropdown.render();

buttonAlignLeft.fire( 'execute' );

// The focus is called twice - once by the button itself
// and once by the dropdown it is in.
sinon.assert.calledTwice( spy );
} );

describe( 'config', () => {
beforeEach( async () => {
// Clean up the editor created in main test suite hook.
Expand Down
4 changes: 3 additions & 1 deletion packages/ckeditor5-highlight/src/highlightui.js
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,9 @@ export default class HighlightUI extends Plugin {
dropdownView.toolbarView.ariaLabel = t( 'Text highlight toolbar' );

// Execute current action from dropdown's split button action button.
splitButtonView.on( 'execute', () => {
// Also focus the editable after executing the command.
// It overrides a default behaviour where the focus is moved to the dropdown button (#12125).
this.listenTo( dropdownView, 'execute', () => {
editor.execute( 'highlight', { value: splitButtonView.commandValue } );
editor.editing.view.focus();
} );
Expand Down
6 changes: 6 additions & 0 deletions packages/ckeditor5-image/src/imagestyle/imagestyleui.js
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,12 @@ export default class ImageStyleUI extends Plugin {
dropdownView.bind( 'isEnabled' )
.toMany( buttonViews, 'isEnabled', ( ...areEnabled ) => areEnabled.some( identity ) );

// Focus the editable after executing the command.
// Overrides a default behaviour where the focus is moved to the dropdown button (#12125).
this.listenTo( dropdownView, 'execute', () => {
this.editor.editing.view.focus();
} );

return dropdownView;
} );
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,12 @@ function getDropdownViewCreator( { editor, parentCommandName, buttonLabel, butto
// Accessibility: focus the first active style when opening the dropdown.
focusChildOnDropdownOpen( dropdownView, () => listPropertiesView.stylesView.children.find( child => child.isOn ) );

// Focus the editable after executing the command.
// Overrides a default behaviour where the focus is moved to the dropdown button (#12125).
dropdownView.on( 'execute', () => {
editor.editing.view.focus();
} );

return dropdownView;
};
}
Expand Down Expand Up @@ -237,8 +243,6 @@ function getStyleButtonCreator( { editor, listStyleCommand, parentCommandName }
editor.execute( 'listStyle', { type } );
} );
}

editor.editing.view.focus();
} );

return button;
Expand Down
5 changes: 4 additions & 1 deletion packages/ckeditor5-style/src/styleui.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,11 @@ export default class StyleUI extends Plugin {
panelView.delegate( 'execute' ).to( dropdown );

// Execute the command when a style is selected in the styles panel.
panelView.on( 'execute', evt => {
// Also focus the editable after executing the command.
// It overrides a default behaviour where the focus is moved to the dropdown button (#12125).
dropdown.on( 'execute', evt => {
editor.execute( 'style', { styleName: evt.source.styleDefinition.name } );
editor.editing.view.focus();
} );

// Bind the state of the styles panel to the command.
Expand Down
8 changes: 6 additions & 2 deletions packages/ckeditor5-table/src/tableui.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
*/

import { Plugin } from 'ckeditor5/src/core';
import { addListToDropdown, createDropdown, Model, SplitButtonView } from 'ckeditor5/src/ui';
import { addListToDropdown, createDropdown, Model, SplitButtonView, SwitchButtonView } from 'ckeditor5/src/ui';
import { Collection } from 'ckeditor5/src/utils';

import InsertTableView from './ui/inserttableview';
Expand Down Expand Up @@ -256,7 +256,11 @@ export default class TableUI extends Plugin {

this.listenTo( dropdownView, 'execute', evt => {
editor.execute( evt.source.commandName );
editor.editing.view.focus();

// Toggling a switch button view should not move the focus to the editable.
if ( !( evt.source instanceof SwitchButtonView ) ) {
editor.editing.view.focus();
}
} );

return dropdownView;
Expand Down
20 changes: 18 additions & 2 deletions packages/ckeditor5-table/tests/tableui.js
Original file line number Diff line number Diff line change
Expand Up @@ -193,11 +193,19 @@ describe( 'TableUI', () => {
it( 'should focus view after command execution', () => {
const focusSpy = testUtils.sinon.spy( editor.editing.view, 'focus' );

dropdown.listView.items.first.children.first.fire( 'execute' );
dropdown.listView.items.get( 2 ).children.last.fire( 'execute' );

sinon.assert.calledOnce( focusSpy );
} );

it( 'should not focus view after using a switchbutton', () => {
const focusSpy = testUtils.sinon.spy( editor.editing.view, 'focus' );

dropdown.listView.items.first.children.last.fire( 'execute' );

sinon.assert.notCalled( focusSpy );
} );

it( 'executes command when it\'s executed', () => {
const spy = sinon.stub( editor, 'execute' );

Expand Down Expand Up @@ -330,11 +338,19 @@ describe( 'TableUI', () => {
it( 'should focus view after command execution', () => {
const focusSpy = testUtils.sinon.spy( editor.editing.view, 'focus' );

dropdown.listView.items.first.children.first.fire( 'execute' );
dropdown.listView.items.get( 2 ).children.first.fire( 'execute' );

sinon.assert.calledOnce( focusSpy );
} );

it( 'should not focus view after using a switchbutton', () => {
const focusSpy = testUtils.sinon.spy( editor.editing.view, 'focus' );

dropdown.listView.items.first.children.last.fire( 'execute' );

sinon.assert.notCalled( focusSpy );
} );

it( 'executes command when it\'s executed', () => {
const spy = sinon.stub( editor, 'execute' );

Expand Down
4 changes: 3 additions & 1 deletion packages/ckeditor5-ui/src/dropdown/dropdownview.js
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,9 @@ export default class DropdownView extends View {
// be updated every time the dropdown is open.
this.on( 'change:isOpen', () => {
if ( !this.isOpen ) {
// If the dropdown was closed, move the focus back to the button (#12125).
this.focus();

return;
}

Expand All @@ -277,7 +280,6 @@ export default class DropdownView extends View {

const closeDropdown = ( data, cancel ) => {
if ( this.isOpen ) {
this.buttonView.focus();
this.isOpen = false;
cancel();
}
Expand Down

0 comments on commit 6fb6303

Please sign in to comment.