Skip to content

Commit

Permalink
Fix (alignment): Tooltips for buttons inside the alignment dropdown s…
Browse files Browse the repository at this point in the history
…hould not obscure adjacent buttons. Closes #16109.
  • Loading branch information
Mati365 committed Mar 27, 2024
1 parent 42135f1 commit 64b51c4
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 22 deletions.
59 changes: 38 additions & 21 deletions packages/ckeditor5-alignment/src/alignmentui.ts
Expand Up @@ -8,7 +8,8 @@
*/

import { Plugin, icons } from 'ckeditor5/src/core.js';
import { ButtonView, createDropdown, addToolbarToDropdown } from 'ckeditor5/src/ui.js';
import { ButtonView, createDropdown, addToolbarToDropdown, type Button } from 'ckeditor5/src/ui.js';
import type { Locale } from 'ckeditor5/src/utils.js';

import { isSupported, normalizeAlignmentOptions } from './utils.js';
import type { SupportedOption } from './alignmentconfig.js';
Expand Down Expand Up @@ -75,11 +76,12 @@ export default class AlignmentUI extends Plugin {

componentFactory.add( 'alignment', locale => {
const dropdownView = createDropdown( locale );
const tooltipPosition = locale.uiLanguageDirection === 'rtl' ? 'w' : 'e';

// Add existing alignment buttons to dropdown's toolbar.
addToolbarToDropdown(
dropdownView,
() => options.map( option => componentFactory.create( `alignment:${ option.name }` ) ) as Array<ButtonView>,
() => options.map( option => this._createButton( locale, option.name, { tooltipPosition } ) ) as Array<ButtonView>,
{
enableActiveItemFocusOnDropdownOpen: true,
isVertical: true,
Expand Down Expand Up @@ -127,28 +129,43 @@ export default class AlignmentUI extends Plugin {
private _addButton( option: SupportedOption ): void {
const editor = this.editor;

editor.ui.componentFactory.add( `alignment:${ option }`, locale => {
const command: AlignmentCommand = editor.commands.get( 'alignment' )!;
const buttonView = new ButtonView( locale );

buttonView.set( {
label: this.localizedOptionTitles[ option ],
icon: iconsMap.get( option ),
tooltip: true,
isToggleable: true
} );
editor.ui.componentFactory.add( `alignment:${ option }`, locale => this._createButton( locale, option ) );
}

// Bind button model to command.
buttonView.bind( 'isEnabled' ).to( command );
buttonView.bind( 'isOn' ).to( command, 'value', value => value === option );
/**
* Helper method for creating the button view element.
*
* @param locale Editor locale.
* @param option The name of the alignment option for which the button is added.
* @param buttonAttrs Optional parameters passed to button view instance.
*/
private _createButton(
locale: Locale,
option: SupportedOption,
buttonAttrs: Partial<Button> = {}
): ButtonView {
const editor = this.editor;
const command: AlignmentCommand = editor.commands.get( 'alignment' )!;
const buttonView = new ButtonView( locale );

buttonView.set( {
label: this.localizedOptionTitles[ option ],
icon: iconsMap.get( option ),
tooltip: true,
isToggleable: true,
...buttonAttrs
} );

// Execute command.
this.listenTo( buttonView, 'execute', () => {
editor.execute( 'alignment', { value: option } );
editor.editing.view.focus();
} );
// Bind button model to command.
buttonView.bind( 'isEnabled' ).to( command );
buttonView.bind( 'isOn' ).to( command, 'value', value => value === option );

return buttonView;
// Execute command.
this.listenTo( buttonView, 'execute', () => {
editor.execute( 'alignment', { value: option } );
editor.editing.view.focus();
} );

return buttonView;
}
}
40 changes: 40 additions & 0 deletions packages/ckeditor5-alignment/tests/alignmentui.js
Expand Up @@ -61,6 +61,7 @@ describe( 'Alignment UI', () => {
expect( button ).to.have.property( 'icon' );
expect( button ).to.have.property( 'tooltip', true );
expect( button ).to.have.property( 'isToggleable', true );
expect( button ).to.have.property( 'tooltipPosition', 's' );
} );

it( 'has isOn bound to command\'s value', () => {
Expand Down Expand Up @@ -103,6 +104,7 @@ describe( 'Alignment UI', () => {
expect( button ).to.have.property( 'label', 'Align right' );
expect( button ).to.have.property( 'icon' );
expect( button ).to.have.property( 'tooltip', true );
expect( button ).to.have.property( 'tooltipPosition', 's' );
} );

it( 'has isOn bound to command\'s value', () => {
Expand Down Expand Up @@ -145,6 +147,7 @@ describe( 'Alignment UI', () => {
expect( button ).to.have.property( 'label', 'Align center' );
expect( button ).to.have.property( 'icon' );
expect( button ).to.have.property( 'tooltip', true );
expect( button ).to.have.property( 'tooltipPosition', 's' );
} );

it( 'has isOn bound to command\'s value', () => {
Expand Down Expand Up @@ -187,6 +190,7 @@ describe( 'Alignment UI', () => {
expect( button ).to.have.property( 'label', 'Justify' );
expect( button ).to.have.property( 'icon' );
expect( button ).to.have.property( 'tooltip', true );
expect( button ).to.have.property( 'tooltipPosition', 's' );
} );

it( 'has isOn bound to command\'s value', () => {
Expand Down Expand Up @@ -271,6 +275,42 @@ describe( 'Alignment UI', () => {
expect( items.includes( 'Justify' ) ).to.be.true;
} );

it( 'tooltips pinned to buttons should be aligned on east', () => {
// Make sure that toolbar view is not created before first dropdown open.
expect( dropdown.toolbarView ).to.be.undefined;

// Trigger toolbar view creation (lazy init).
dropdown.isOpen = true;

const items = [ ...dropdown.toolbarView.items ].map( item => item.tooltipPosition );

expect( items ).to.have.length( 4 );
expect( items.every( item => item === 'e' ) ).to.be.true;
} );

it( 'tooltips pinned to buttons should be aligned on west (RTL ui)', async () => {
// Clean up the editor created in main test suite hook.
await editor.destroy();

const newEditor = await ClassicTestEditor.create( element, {
language: {
content: 'ar',
ui: 'ar'
},
plugins: [ AlignmentEditing, AlignmentUI ]
} );

dropdown = newEditor.ui.componentFactory.create( 'alignment' );
dropdown.isOpen = true;

const items = [ ...dropdown.toolbarView.items ].map( item => item.tooltipPosition );

expect( items ).to.have.length( 4 );
expect( items.every( item => item === 'w' ) ).to.be.true;

await newEditor.destroy();
} );

it( 'should use icon related to current command value', () => {
// Make sure that toolbar view is not created before first dropdown open.
expect( dropdown.toolbarView ).to.be.undefined;
Expand Down
2 changes: 1 addition & 1 deletion packages/ckeditor5-ui/src/index.ts
Expand Up @@ -17,7 +17,7 @@ export { default as AccessibilityHelp } from './editorui/accessibilityhelp/acces

export { default as BodyCollection } from './editorui/bodycollection.js';

export { type ButtonExecuteEvent } from './button/button.js';
export { default as Button, type ButtonExecuteEvent } from './button/button.js';
export type { default as ButtonLabel } from './button/buttonlabel.js';
export { default as ButtonView } from './button/buttonview.js';
export { default as ButtonLabelView } from './button/buttonlabelview.js';
Expand Down

0 comments on commit 64b51c4

Please sign in to comment.