Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Redesign on state in menu and dropdowns #16573

Merged
merged 31 commits into from
Jun 28, 2024
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
179fd9b
Add proper `role=menuitemcheckbox` aria roles, handle `isOn` state in…
Mati365 Jun 14, 2024
4bcd91d
Adjust docs
Mati365 Jun 20, 2024
cd05cfc
Revert remove declare
Mati365 Jun 20, 2024
18070c9
Fix docs
Mati365 Jun 20, 2024
74482fc
Adjust docs
Mati365 Jun 20, 2024
b03c915
Fix `FileDialogListItemButtonView` docs
Mati365 Jun 25, 2024
ad98d1f
Fix `FileDialogViewMixin` comment
Mati365 Jun 25, 2024
1a573e7
Remove unecessary checks
Mati365 Jun 25, 2024
90c3abe
Adjust docs
Mati365 Jun 25, 2024
4f4032a
Drop `maybe` prefix
Mati365 Jun 25, 2024
a131d8b
Reoder methods
Mati365 Jun 25, 2024
dc1a23b
Extend implementation of `getButtonCreator` to set `menuitemcheckbox`…
Mati365 Jun 25, 2024
3a19a71
Fix `ToggleSwitchButton` styling in lists
Mati365 Jun 25, 2024
ce4ac29
Fix `Link` plugin checkbox role
Mati365 Jun 25, 2024
1b974fe
Fix `BlockQuote` plugin checkbox role
Mati365 Jun 25, 2024
ac877d3
Fix linter
Mati365 Jun 25, 2024
f2f3653
Fix not bound `isOpen` `Dialog` property
Mati365 Jun 25, 2024
89d5ad6
Add toggleable accessibility menubar state
Mati365 Jun 25, 2024
f8b2ebb
Fix find and replace menu state
Mati365 Jun 25, 2024
9c4dba3
Improve binding
Mati365 Jun 25, 2024
0d15552
Fix wrong colorize of highlight ui
Mati365 Jun 25, 2024
ca8f1c9
Fix white revision history buttons
Mati365 Jun 25, 2024
3656138
Improve keyboard focusing
Mati365 Jun 25, 2024
6c04b64
Add missing doc
Mati365 Jun 25, 2024
9174553
Add missing doc
Mati365 Jun 25, 2024
8701e2f
Adjust hover styles in menubar root items
Mati365 Jun 25, 2024
4e20ec7
Use shorter names in check inputs
Mati365 Jun 25, 2024
7e0127f
Rename handler
Mati365 Jun 25, 2024
ad492fc
Move `tooltip: true` set to `getButtonCreator` and change `instanceof…
Mati365 Jun 26, 2024
7d7b3d9
Fix missing rename
Mati365 Jun 26, 2024
c8c82cd
Add missing focus tracking after focusing menu using alt+f9
Mati365 Jun 26, 2024
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
15 changes: 1 addition & 14 deletions packages/ckeditor5-basic-styles/src/bold/boldui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ export default class BoldUI extends Plugin {
public init(): void {
const editor = this.editor;
const t = editor.locale.t;
const command = editor.commands.get( BOLD )!;
const createButton = getButtonCreator( {
editor,
commandName: BOLD,
Expand All @@ -48,21 +47,9 @@ export default class BoldUI extends Plugin {
tooltip: true
} );

buttonView.bind( 'isOn' ).to( command, 'value' );

return buttonView;
} );

editor.ui.componentFactory.add( 'menuBar:' + BOLD, () => {
const buttonView = createButton( MenuBarMenuListItemButtonView );

buttonView.set( {
role: 'menuitemcheckbox'
} );

buttonView.bind( 'isOn' ).to( command, 'value' );

return buttonView;
} );
editor.ui.componentFactory.add( 'menuBar:' + BOLD, () => createButton( MenuBarMenuListItemButtonView ) );
}
}
17 changes: 1 addition & 16 deletions packages/ckeditor5-basic-styles/src/code/codeui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ export default class CodeUI extends Plugin {
*/
public init(): void {
const editor = this.editor;
const command = editor.commands.get( CODE )!;

const t = editor.locale.t;
const createButton = getButtonCreator( {
editor,
Expand All @@ -52,22 +50,9 @@ export default class CodeUI extends Plugin {
tooltip: true
} );

// Bind button model to command.
buttonView.bind( 'isOn' ).to( command, 'value' );

return buttonView;
} );

editor.ui.componentFactory.add( 'menuBar:' + CODE, () => {
const buttonView = createButton( MenuBarMenuListItemButtonView );

buttonView.set( {
role: 'menuitemcheckbox'
} );

buttonView.bind( 'isOn' ).to( command, 'value' );

return buttonView;
} );
editor.ui.componentFactory.add( 'menuBar:' + CODE, () => createButton( MenuBarMenuListItemButtonView ) );
}
}
15 changes: 1 addition & 14 deletions packages/ckeditor5-basic-styles/src/italic/italicui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ export default class ItalicUI extends Plugin {
*/
public init(): void {
const editor = this.editor;
const command = editor.commands.get( ITALIC )!;
const t = editor.locale.t;
const createButton = getButtonCreator( {
editor,
Expand All @@ -50,21 +49,9 @@ export default class ItalicUI extends Plugin {
tooltip: true
} );

buttonView.bind( 'isOn' ).to( command, 'value' );

return buttonView;
} );

editor.ui.componentFactory.add( 'menuBar:' + ITALIC, () => {
const buttonView = createButton( MenuBarMenuListItemButtonView );

buttonView.set( {
role: 'menuitemcheckbox'
} );

buttonView.bind( 'isOn' ).to( command, 'value' );

return buttonView;
} );
editor.ui.componentFactory.add( 'menuBar:' + ITALIC, () => createButton( MenuBarMenuListItemButtonView ) );
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ export default class StrikethroughUI extends Plugin {
*/
public init(): void {
const editor = this.editor;
const command = editor.commands.get( STRIKETHROUGH )!;
const t = editor.locale.t;
const createButton = getButtonCreator( {
editor,
Expand All @@ -50,22 +49,9 @@ export default class StrikethroughUI extends Plugin {
tooltip: true
} );

// Bind button model to command.
buttonView.bind( 'isOn' ).to( command, 'value' );

return buttonView;
} );

editor.ui.componentFactory.add( 'menuBar:' + STRIKETHROUGH, () => {
const buttonView = createButton( MenuBarMenuListItemButtonView );

buttonView.set( {
role: 'menuitemcheckbox'
} );

buttonView.bind( 'isOn' ).to( command, 'value' );

return buttonView;
} );
editor.ui.componentFactory.add( 'menuBar:' + STRIKETHROUGH, () => createButton( MenuBarMenuListItemButtonView ) );
}
}
16 changes: 1 addition & 15 deletions packages/ckeditor5-basic-styles/src/subscript/subscriptui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ export default class SubscriptUI extends Plugin {
public init(): void {
const editor = this.editor;
const t = editor.locale.t;
const command = editor.commands.get( SUBSCRIPT )!;

const createButton = getButtonCreator( {
editor,
Expand All @@ -50,22 +49,9 @@ export default class SubscriptUI extends Plugin {
tooltip: true
} );

// Bind button model to command.
buttonView.bind( 'isOn' ).to( command, 'value' );

return buttonView;
} );

editor.ui.componentFactory.add( 'menuBar:' + SUBSCRIPT, () => {
const buttonView = createButton( MenuBarMenuListItemButtonView );

buttonView.set( {
role: 'menuitemcheckbox'
} );

buttonView.bind( 'isOn' ).to( command, 'value' );

return buttonView;
} );
editor.ui.componentFactory.add( 'menuBar:' + SUBSCRIPT, () => createButton( MenuBarMenuListItemButtonView ) );
}
}
16 changes: 1 addition & 15 deletions packages/ckeditor5-basic-styles/src/superscript/superscriptui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ export default class SuperscriptUI extends Plugin {
public init(): void {
const editor = this.editor;
const t = editor.locale.t;
const command = editor.commands.get( SUPERSCRIPT )!;
const createButton = getButtonCreator( {
editor,
commandName: SUPERSCRIPT,
Expand All @@ -49,22 +48,9 @@ export default class SuperscriptUI extends Plugin {
tooltip: true
} );

// Bind button model to command.
buttonView.bind( 'isOn' ).to( command, 'value' );

return buttonView;
} );

editor.ui.componentFactory.add( 'menuBar:' + SUPERSCRIPT, () => {
const buttonView = createButton( MenuBarMenuListItemButtonView );

buttonView.set( {
role: 'menuitemcheckbox'
} );

buttonView.bind( 'isOn' ).to( command, 'value' );

return buttonView;
} );
editor.ui.componentFactory.add( 'menuBar:' + SUPERSCRIPT, () => createButton( MenuBarMenuListItemButtonView ) );
}
}
15 changes: 1 addition & 14 deletions packages/ckeditor5-basic-styles/src/underline/underlineui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ export default class UnderlineUI extends Plugin {
*/
public init(): void {
const editor = this.editor;
const command = editor.commands.get( UNDERLINE )!;
const t = editor.locale.t;
const createButton = getButtonCreator( {
editor,
Expand All @@ -50,21 +49,9 @@ export default class UnderlineUI extends Plugin {
tooltip: true
} );

buttonView.bind( 'isOn' ).to( command, 'value' );

return buttonView;
} );

editor.ui.componentFactory.add( 'menuBar:' + UNDERLINE, () => {
const buttonView = createButton( MenuBarMenuListItemButtonView );

buttonView.set( {
role: 'menuitemcheckbox'
} );

buttonView.bind( 'isOn' ).to( command, 'value' );

return buttonView;
} );
editor.ui.componentFactory.add( 'menuBar:' + UNDERLINE, () => createButton( MenuBarMenuListItemButtonView ) );
}
}
9 changes: 8 additions & 1 deletion packages/ckeditor5-basic-styles/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

import type { Editor, Plugin } from 'ckeditor5/src/core.js';
import type AttributeCommand from './attributecommand.js';
import type { ButtonView, MenuBarMenuListItemButtonView } from 'ckeditor5/src/ui.js';
import { ListItemButtonView, type ButtonView, type MenuBarMenuListItemButtonView } from 'ckeditor5/src/ui.js';

/**
* Returns a function that creates a (toolbar or menu bar) button for a basic style feature.
Expand All @@ -36,6 +36,13 @@ export function getButtonCreator( {
} );

view.bind( 'isEnabled' ).to( command, 'isEnabled' );
view.bind( 'isOn' ).to( command, 'value' );

if ( view instanceof ListItemButtonView ) {
Mati365 marked this conversation as resolved.
Show resolved Hide resolved
view.set( {
role: 'menuitemcheckbox'
} );
}

// Execute the command.
plugin.listenTo( view, 'execute', () => {
Expand Down
16 changes: 10 additions & 6 deletions packages/ckeditor5-block-quote/src/blockquoteui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

import { Plugin, icons } from 'ckeditor5/src/core.js';
import { ButtonView, MenuBarMenuListItemButtonView } from 'ckeditor5/src/ui.js';
import type BlockQuoteCommand from './blockquotecommand.js';

import '../theme/blockquote.css';

Expand All @@ -33,7 +32,6 @@ export default class BlockQuoteUI extends Plugin {
*/
public init(): void {
const editor = this.editor;
const command: BlockQuoteCommand = editor.commands.get( 'blockQuote' )!;

editor.ui.componentFactory.add( 'blockQuote', () => {
const buttonView = this._createButton( ButtonView );
Expand All @@ -42,13 +40,18 @@ export default class BlockQuoteUI extends Plugin {
tooltip: true
} );

// Bind button model to command.
buttonView.bind( 'isOn' ).to( command, 'value' );

return buttonView;
} );

editor.ui.componentFactory.add( 'menuBar:blockQuote', () => this._createButton( MenuBarMenuListItemButtonView ) );
editor.ui.componentFactory.add( 'menuBar:blockQuote', () => {
const buttonView = this._createButton( MenuBarMenuListItemButtonView );

buttonView.set( {
role: 'menuitemcheckbox'
} );

return buttonView;
} );
}

/**
Expand All @@ -68,6 +71,7 @@ export default class BlockQuoteUI extends Plugin {
} );

view.bind( 'isEnabled' ).to( command, 'isEnabled' );
view.bind( 'isOn' ).to( command, 'value' );

// Execute the command.
this.listenTo( view, 'execute', () => {
Expand Down
9 changes: 9 additions & 0 deletions packages/ckeditor5-find-and-replace/src/findandreplaceui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,15 @@ export default class FindAndReplaceUI extends Plugin {
private _createDialogButtonForMenuBar(): MenuBarMenuListItemButtonView {
const buttonView = this._createButton( MenuBarMenuListItemButtonView );
const dialogPlugin = this.editor.plugins.get( 'Dialog' );
const dialog = this.editor.plugins.get( 'Dialog' );

buttonView.set( {
role: 'menuitemcheckbox',
isToggleable: true
} );

// Button should be on when the find and replace dialog is opened.
buttonView.bind( 'isOn' ).to( dialog, 'id', id => id === 'findAndReplace' );

buttonView.on( 'execute', () => {
if ( dialogPlugin.id === 'findAndReplace' ) {
Expand Down
14 changes: 14 additions & 0 deletions packages/ckeditor5-find-and-replace/tests/findandreplaceui.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,10 @@ describe( 'FindAndReplaceUI', () => {
} );

testButton();

it( 'should have proper role set', () => {
expect( button.role ).to.be.equal( 'menuitemcheckbox' );
} );
} );

function testButton() {
Expand All @@ -139,6 +143,16 @@ describe( 'FindAndReplaceUI', () => {
sinon.assert.callOrder( disableCssTransitionsSpy, selectSpy, enableCssTransitionsSpy );
} );

it( 'should be bound to dialog id', () => {
dialogPlugin.id = 'findAndReplace';

expect( button.isOn ).to.be.true;

dialogPlugin.id = null;

expect( button.isOn ).to.be.false;
} );

it( 'the form should be reset', () => {
const spy = sinon.spy( form, 'reset' );

Expand Down
3 changes: 2 additions & 1 deletion packages/ckeditor5-highlight/src/highlightui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,9 +291,10 @@ export default class HighlightUI extends Plugin {
isToggleable: true
} );

buttonView.iconView.fillColor = option.color;

buttonView.delegate( 'execute' ).to( menuView );
buttonView.bind( 'isOn' ).to( command, 'value', value => value === option.model );
buttonView.iconView.bind( 'fillColor' ).to( buttonView, 'isOn', value => value ? 'transparent' : option.color );

buttonView.on( 'execute', () => {
editor.execute( 'highlight', { value: option.model } );
Expand Down
26 changes: 0 additions & 26 deletions packages/ckeditor5-highlight/tests/highlightui.js
Original file line number Diff line number Diff line change
Expand Up @@ -425,32 +425,6 @@ describe( 'HighlightUI', () => {
] );
} );

it( 'should bind #fillColor to #isOn', () => {
command.value = 'pinkMarker';

expect( dumpItems( buttonView => buttonView.iconView.fillColor ) ).to.have.deep.ordered.members( [
[ 'Yellow marker', 'var(--ck-highlight-marker-yellow)' ],
[ 'Green marker', 'var(--ck-highlight-marker-green)' ],
[ 'Pink marker', 'transparent' ],
[ 'Blue marker', 'var(--ck-highlight-marker-blue)' ],
[ 'Red pen', 'var(--ck-highlight-pen-red)' ],
[ 'Green pen', 'var(--ck-highlight-pen-green)' ],
[ 'Remove highlight', '' ]
] );

command.value = 'redPen';

expect( dumpItems( buttonView => buttonView.iconView.fillColor ) ).to.have.deep.ordered.members( [
[ 'Yellow marker', 'var(--ck-highlight-marker-yellow)' ],
[ 'Green marker', 'var(--ck-highlight-marker-green)' ],
[ 'Pink marker', 'var(--ck-highlight-marker-pink)' ],
[ 'Blue marker', 'var(--ck-highlight-marker-blue)' ],
[ 'Red pen', 'transparent' ],
[ 'Green pen', 'var(--ck-highlight-pen-green)' ],
[ 'Remove highlight', '' ]
] );
} );

it( 'should delegate #execute from an item to the menu', () => {
const spy = sinon.spy();

Expand Down
Loading