Skip to content

Commit

Permalink
Merge pull request #307 from ckeditor/i/6631
Browse files Browse the repository at this point in the history
Internal: The "Merge cells" split button should always be enabled to improve discoverability of tools in the dropdown. Closes #6631.
  • Loading branch information
Reinmar authored Apr 21, 2020
2 parents fa71bdd + c1974ca commit edccd37
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 13 deletions.
13 changes: 2 additions & 11 deletions packages/ckeditor5-table/src/tableui.js
Original file line number Diff line number Diff line change
Expand Up @@ -278,19 +278,10 @@ export default class TableUI extends Plugin {
dropdownView.buttonView.set( {
label,
icon,
tooltip: true
tooltip: true,
isEnabled: true
} );

// The main part of the split button is bound to the "mergeTableCells" command only.
dropdownView.bind( 'isEnabled' ).to( editor.commands.get( mergeCommandName ) );

// The split button dropdown must be **always** enabled and ready to open no matter the state
// of the "mergeTableCells" command. You may not be able to merge multiple cells but you may want
// to split them. This is also about mobile devices where multi–cell selection will never work
// (that's why "Merge cell right", "Merge cell down", etc. are still there in the first place).
dropdownView.buttonView.arrowView.unbind( 'isEnabled' );
dropdownView.buttonView.arrowView.isEnabled = true;

// Merge selected table cells when the main part of the split button is clicked.
this.listenTo( dropdownView.buttonView, 'execute', () => {
editor.execute( mergeCommandName );
Expand Down
4 changes: 2 additions & 2 deletions packages/ckeditor5-table/tests/tableui.js
Original file line number Diff line number Diff line change
Expand Up @@ -386,9 +386,9 @@ describe( 'TableUI', () => {
expect( dropdown.buttonView ).to.be.instanceOf( SplitButtonView );
} );

it( 'should bind #isEnabled to the "mergeTableCells" command', () => {
it( 'should have #isEnabled always true regardless of the "mergeTableCells" command state', () => {
command.isEnabled = false;
expect( dropdown.isEnabled ).to.be.false;
expect( dropdown.isEnabled ).to.be.true;

command.isEnabled = true;
expect( dropdown.isEnabled ).to.be.true;
Expand Down

0 comments on commit edccd37

Please sign in to comment.