Skip to content

Commit

Permalink
Merge pull request #17562 from ckeditor/ck/epic/17230-change-back-but…
Browse files Browse the repository at this point in the history
…ton-visibility

Do not show back button if balloon was not opened using another balloon.
  • Loading branch information
Mati365 authored Dec 3, 2024
2 parents ed11fb2 + 675e150 commit 0cfd778
Show file tree
Hide file tree
Showing 15 changed files with 219 additions and 20 deletions.
4 changes: 2 additions & 2 deletions packages/ckeditor5-bookmark/src/bookmarkui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -373,8 +373,7 @@ export default class BookmarkUI extends Plugin {
return;
}

const editor = this.editor;
const updateBookmarkCommand: UpdateBookmarkCommand = editor.commands.get( 'updateBookmark' )!;
const updateBookmarkCommand: UpdateBookmarkCommand = this.editor.commands.get( 'updateBookmark' )!;

this.formView!.disableCssTransitions();
this.formView!.resetFormStatus();
Expand All @@ -384,6 +383,7 @@ export default class BookmarkUI extends Plugin {
position: this._getBalloonPositionData()
} );

this.formView!.backButtonView.isVisible = updateBookmarkCommand.isEnabled;
this.formView!.idInputView.fieldView.value = updateBookmarkCommand.value || '';

// Select input when form view is currently visible.
Expand Down
1 change: 1 addition & 0 deletions packages/ckeditor5-bookmark/src/ui/bookmarkformview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,7 @@ export default class BookmarkFormView extends View {
const backButton = new ButtonView( this.locale );

backButton.set( {
class: 'ck-button-back',
label: t( 'Back' ),
icon: icons.previousArrow,
tooltip: true
Expand Down
27 changes: 27 additions & 0 deletions packages/ckeditor5-bookmark/tests/bookmarkui.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,24 @@ describe( 'BookmarkUI', () => {

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

it( 'should toggle the balloon UI with hidden back button (if not updating)', () => {
const updateBookmark = editor.commands.get( 'updateBookmark' );

sinon.stub( updateBookmark, 'isEnabled' ).get( () => false );
button.fire( 'execute' );

expect( bookmarkUIFeature.formView.backButtonView.isVisible ).to.be.false;
} );

it( 'should toggle the balloon UI with visible back button (if updating)', () => {
const updateBookmark = editor.commands.get( 'updateBookmark' );

sinon.stub( updateBookmark, 'isEnabled' ).get( () => true );
button.fire( 'execute' );

expect( bookmarkUIFeature.formView.backButtonView.isVisible ).to.be.true;
} );
}

describe( 'bookmark toolbar components', () => {
Expand Down Expand Up @@ -211,6 +229,15 @@ describe( 'BookmarkUI', () => {
expect( button.isEnabled ).to.equal( false );
} );

it( 'should toggle the balloon UI with visible back button', () => {
const updateBookmarkCommand = editor.commands.get( 'updateBookmark' );

sinon.stub( updateBookmarkCommand, 'isEnabled' ).get( () => true );
button.fire( 'execute' );

expect( bookmarkUIFeature.formView.backButtonView.isVisible ).to.be.true;
} );

it( 'should trigger #_showFormView() on execute', () => {
const spy = sinon.stub( bookmarkUIFeature, '_showFormView' );

Expand Down
5 changes: 5 additions & 0 deletions packages/ckeditor5-bookmark/tests/ui/bookmarkformview.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ describe( 'BookmarkFormView', () => {
expect( formHeaderView.children.get( 0 ) ).to.equal( view.backButtonView );
} );

it( 'should create back button view with proper classes', () => {
expect( view.backButtonView.element.classList.contains( 'ck-button' ) ).to.be.true;
expect( view.backButtonView.element.classList.contains( 'ck-button-back' ) ).to.be.true;
} );

it( 'should create #focusTracker instance', () => {
expect( view.focusTracker ).to.be.instanceOf( FocusTracker );
} );
Expand Down
35 changes: 26 additions & 9 deletions packages/ckeditor5-image/src/imageresize/imageresizebuttons.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,11 @@ export default class ImageResizeButtons extends Plugin {
} else {
const optionValueWithUnit = value ? value + this._resizeUnit : null;

button.bind( 'isOn' ).to( command, 'value', getIsOnButtonCallback( optionValueWithUnit ) );
button.bind( 'isOn' ).to(
command, 'value',
command, 'isEnabled',
getIsOnButtonCallback( optionValueWithUnit )
);

this.listenTo( button, 'execute', () => {
editor.execute( 'resizeImage', { width: optionValueWithUnit } );
Expand Down Expand Up @@ -303,7 +307,11 @@ export default class ImageResizeButtons extends Plugin {

const allDropdownValues = map( optionsWithSerializedValues, 'valueWithUnits' );

definition.model.bind( 'isOn' ).to( command, 'value', getIsOnCustomButtonCallback( allDropdownValues ) );
definition.model.bind( 'isOn' ).to(
command, 'value',
command, 'isEnabled',
getIsOnCustomButtonCallback( allDropdownValues )
);
} else {
definition = {
type: 'button',
Expand All @@ -317,7 +325,11 @@ export default class ImageResizeButtons extends Plugin {
} )
};

definition.model.bind( 'isOn' ).to( command, 'value', getIsOnButtonCallback( option.valueWithUnits ) );
definition.model.bind( 'isOn' ).to(
command, 'value',
command, 'isEnabled',
getIsOnButtonCallback( option.valueWithUnits )
);
}

definition.model.bind( 'isEnabled' ).to( command, 'isEnabled' );
Expand All @@ -338,9 +350,14 @@ function isCustomImageResizeOption( option: ImageResizeOption ) {
/**
* A helper function for setting the `isOn` state of buttons in value bindings.
*/
function getIsOnButtonCallback( value: string | null ): ( commandValue: unknown ) => boolean {
return ( commandValue: unknown ): boolean => {
const objectCommandValue = commandValue as null | { width: string | null };
function getIsOnButtonCallback( value: string | null ) {
return ( commandValue: ResizeImageCommand['value'], isEnabled: boolean ): boolean => {
const objectCommandValue = commandValue as null | undefined | { width: string | null };

if ( objectCommandValue === undefined || !isEnabled ) {
return false;
}

if ( value === null && objectCommandValue === value ) {
return true;
}
Expand All @@ -352,8 +369,8 @@ function getIsOnButtonCallback( value: string | null ): ( commandValue: unknown
/**
* A helper function for setting the `isOn` state of custom size button in value bindings.
*/
function getIsOnCustomButtonCallback( allDropdownValues: Array<string | null> ): ( commandValue: unknown ) => boolean {
return ( commandValue: unknown ): boolean => !allDropdownValues.some(
dropdownValue => getIsOnButtonCallback( dropdownValue )( commandValue )
function getIsOnCustomButtonCallback( allDropdownValues: Array<string | null> ) {
return ( commandValue: ResizeImageCommand['value'], isEnabled: boolean ): boolean => !allDropdownValues.some(
dropdownValue => getIsOnButtonCallback( dropdownValue )( commandValue, isEnabled )
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export default class ResizeImageCommand extends Command {
/**
* Desired image width and height.
*/
declare public value: null | {
declare public value: undefined | null | {
width: string | null;
height: string | null;
};
Expand Down
103 changes: 103 additions & 0 deletions packages/ckeditor5-image/tests/imageresize/imageresizebuttons.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

/* global document */

import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor.js';
import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor.js';
import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph.js';
import Image from '../../src/image.js';
Expand Down Expand Up @@ -111,6 +112,108 @@ describe( 'ImageResizeButtons', () => {
} );
} );

describe( 'resize options main toolbar buttons', () => {
let editor;

beforeEach( async () => {
editor = await ClassicEditor
.create( editorElement, {
plugins: [ Image, ImageStyle, Paragraph, Undo, Table, ImageResizeButtons, ImageCustomResizeUI ],
image: {
resizeUnit: '%',
resizeOptions: [ {
name: 'resizeImage:original',
value: null,
icon: 'original'
},
{
name: 'resizeImage:custom',
value: 'custom',
icon: 'custom'
},
{
name: 'resizeImage:25',
value: '25',
icon: 'small'
},
{
name: 'resizeImage:50',
value: '50',
icon: 'medium'
},
{
name: 'resizeImage:75',
value: '75',
icon: 'large'
} ]
},
toolbar: [ 'resizeImage:original', 'resizeImage:custom', 'resizeImage:25', 'resizeImage:50', 'resizeImage:75' ]
} );

plugin = editor.plugins.get( 'ImageResizeButtons' );
} );

afterEach( async () => {
if ( editorElement ) {
editorElement.remove();
}

if ( editor && editor.state !== 'destroyed' ) {
await editor.destroy();
}
} );

it( 'should register resize options as items in the main toolbar', () => {
const toolbar = editor.ui.view.toolbar;

expect( toolbar.items.map( item => item.label ) ).to.deep.equal( [
'Resize image to the original size',
'Custom image size',
'Resize image to 25%',
'Resize image to 50%',
'Resize image to 75%'
] );
} );

it( 'should synchronize button states with command\'s isEnabled property', () => {
const toolbar = editor.ui.view.toolbar;
const resizeCommand = editor.commands.get( 'resizeImage' );
const resizeComponents = toolbar.items.filter( item => item.label && item.label.includes( 'Resize image' ) );

resizeCommand.isEnabled = true;
expect( resizeComponents.every( item => item.isEnabled ) ).to.be.true;

resizeCommand.isEnabled = false;
expect( resizeComponents.every( item => item.isEnabled ) ).to.be.false;
} );

it( 'should properly sync isOn states of buttons', () => {
const toolbar = editor.ui.view.toolbar;
const resizeCommand = editor.commands.get( 'resizeImage' );
const resizeComponents = toolbar.items.filter( item => item.label && item.label.includes( 'Resize image' ) );

resizeCommand.isEnabled = false;
resizeCommand.value = undefined;

expect( resizeComponents.every( item => item.isOn ) ).to.be.false;

resizeCommand.isEnabled = false;
expect( resizeComponents.every( item => item.isOn ) ).to.be.false;

resizeCommand.value = undefined;
resizeCommand.isEnabled = true;
expect( resizeComponents.every( item => item.isOn ) ).to.be.false;

resizeCommand.value = { width: '50%' };
resizeCommand.isEnabled = true;

expect( resizeComponents[ 2 ].isOn ).to.be.true;

resizeCommand.isEnabled = false;
expect( resizeComponents[ 2 ].isOn ).to.be.false;
} );
} );

describe( 'resize options dropdown', () => {
it( 'should be disabled when plugin is disabled', () => {
const dropdownView = editor.ui.componentFactory.create( 'resizeImage' );
Expand Down
9 changes: 8 additions & 1 deletion packages/ckeditor5-image/tests/manual/imageresizebuttons.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,14 @@ const imageConfig2 = {

const config2 = {
...commonConfig,
image: imageConfig2
image: imageConfig2,
toolbar: [
...commonConfig.toolbar, '|',
'resizeImage:50',
'resizeImage:75',
'resizeImage:original',
'resizeImage:custom'
]
};

ClassicEditor
Expand Down
2 changes: 1 addition & 1 deletion packages/ckeditor5-image/tests/manual/textalternative.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import ImageToolbar from '../../src/imagetoolbar.js';
ClassicEditor
.create( document.querySelector( '#editor' ), {
plugins: [ EnterPlugin, TypingPlugin, ParagraphPlugin, HeadingPlugin, ImagePlugin, UndoPlugin, ClipboardPlugin, ImageToolbar ],
toolbar: [ 'heading', '|', 'undo', 'redo' ],
toolbar: [ 'heading', '|', 'undo', 'redo', '|', 'imageTextAlternative' ],
image: {
toolbar: [ 'imageTextAlternative' ]
}
Expand Down
5 changes: 3 additions & 2 deletions packages/ckeditor5-link/src/linkui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ export default class LinkUI extends Plugin {
// Open the form view on Ctrl+K when the **link toolbar have focus**..
toolbarView.keystrokes.set( LINK_KEYSTROKE, ( data, cancel ) => {
this._addFormView();

cancel();
} );

Expand Down Expand Up @@ -727,11 +728,11 @@ export default class LinkUI extends Plugin {
return;
}

const editor = this.editor;
const linkCommand: LinkCommand = editor.commands.get( 'link' )!;
const linkCommand: LinkCommand = this.editor.commands.get( 'link' )!;

this.formView!.disableCssTransitions();
this.formView!.resetFormStatus();
this.formView!.backButtonView.isVisible = linkCommand.isEnabled && !!linkCommand.value;

this._balloon.add( {
view: this.formView!,
Expand Down
1 change: 1 addition & 0 deletions packages/ckeditor5-link/src/ui/linkformview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ export default class LinkFormView extends View {
const backButton = new ButtonView( this.locale );

backButton.set( {
class: 'ck-button-back',
label: t( 'Back' ),
icon: icons.previousArrow,
tooltip: true
Expand Down
18 changes: 18 additions & 0 deletions packages/ckeditor5-link/tests/linkui.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,12 @@ describe( 'LinkUI', () => {
command.isEnabled = !initState;
expect( linkButton.isEnabled ).to.equal( !initState );
} );

it( 'should toggle the link UI with hidden back button', () => {
linkButton.fire( 'execute' );

expect( linkUIFeature.formView.backButtonView.isVisible ).to.be.false;
} );
}

describe( 'the "linkPreview" toolbar button', () => {
Expand Down Expand Up @@ -301,6 +307,18 @@ describe( 'LinkUI', () => {

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

it( 'should open link form view with back button', () => {
const linkCommand = editor.commands.get( 'link' );

// Simulate link selection.
linkCommand.isEnabled = true;
linkCommand.value = 'http://ckeditor.com';

button.fire( 'execute' );

expect( linkUIFeature.formView.backButtonView.isVisible ).to.be.true;
} );
} );

describe( 'the "linkProperties" toolbar button', () => {
Expand Down
7 changes: 7 additions & 0 deletions packages/ckeditor5-link/tests/ui/linkformview.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,13 @@ describe( 'LinkFormView', () => {

expect( backButton.template.children[ 0 ].get( 1 ).text ).to.equal( 'Back' );
} );

it( 'should `backButtonView` has correct CSS class', () => {
const headerChildren = view.template.children[ 0 ].get( 0 ).template.children[ 0 ];
const backButton = headerChildren.get( 0 );

expect( backButton.class ).to.equal( 'ck-button-back' );
} );
} );
} );

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,14 @@
& .ck-form__header {
padding: var(--ck-spacing-small) var(--ck-spacing-medium);

& > .ck:not(:first-child) {
margin-inline-start: var(--ck-spacing-small);
/* Back button is hidden */
&:has(.ck-button-back.ck-hidden) {
padding-inline-start: var(--ck-spacing-large);
padding-inline-end: var(--ck-spacing-large);
}

& > .ck-button-back {
margin-inline-end: var(--ck-spacing-small);
}
}

Expand Down
Loading

0 comments on commit 0cfd778

Please sign in to comment.