Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Merge pull request #390 from ckeditor/t/389
Browse files Browse the repository at this point in the history
Other: Disabling a `ListItemView` should be possible using the `isEnabled` property. Closes #389.
  • Loading branch information
oleq authored Jun 5, 2018
2 parents beb9f96 + f7c4177 commit 76a4d47
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 11 deletions.
2 changes: 1 addition & 1 deletion src/button/buttonview.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ export default class ButtonView extends View {
class: [
'ck',
'ck-button',
bind.to( 'isEnabled', value => value ? 'ck-enabled' : 'ck-disabled' ),
bind.if( 'isEnabled', 'ck-disabled', value => !value ),
bind.if( 'isVisible', 'ck-hidden', value => !value ),
bind.to( 'isOn', value => value ? 'ck-on' : 'ck-off' ),
bind.if( 'withText', 'ck-button_with-text' )
Expand Down
2 changes: 1 addition & 1 deletion src/dropdown/dropdownview.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ export default class DropdownView extends View {
class: [
'ck',
'ck-dropdown',
bind.to( 'isEnabled', isEnabled => isEnabled ? '' : 'ck-disabled' )
bind.if( 'isEnabled', 'ck-disabled', value => !value )
]
},

Expand Down
24 changes: 22 additions & 2 deletions src/list/listitemview.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ export default class ListItemView extends View {
'ck',
'ck-list__item',
bind.to( 'class' ),
bind.if( 'isActive', 'ck-list__item_active' )
bind.if( 'isActive', 'ck-list__item_active' ),
bind.if( 'isEnabled', 'ck-disabled', value => !value )
],
style: bind.to( 'style' ),
tabindex: bind.to( 'tabindex' )
Expand All @@ -62,10 +63,29 @@ export default class ListItemView extends View {
],

on: {
click: bind.to( 'execute' )
click: bind.to( evt => {
// We can't make the button disabled using the disabled attribute, because it won't be focusable.
// Though, shouldn't this condition be moved to the button controller?
if ( this.isEnabled ) {
this.fire( 'execute' );
} else {
// Prevent the default when button is disabled, to block e.g.
// automatic form submitting. See ckeditor/ckeditor5-link#74.
evt.preventDefault();
}
} )
}
} );

/**
* (Optional) Controls whether the list item is enabled, i.e. it can be clicked and execute an action.
*
* @observable
* @default true
* @member {Boolean} #isEnabled
*/
this.set( 'isEnabled', true );

/**
* The label of the list item.
*
Expand Down
4 changes: 2 additions & 2 deletions tests/button/buttonview.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,10 @@ describe( 'ButtonView', () => {
describe( '<button> bindings', () => {
describe( 'class', () => {
it( 'is set initially', () => {
expect( view.element.classList ).to.have.length( 4 );
expect( view.element.classList ).to.have.length( 3 );
expect( view.element.classList.contains( 'ck' ) ).to.true;
expect( view.element.classList.contains( 'ck-button' ) ).to.true;
expect( view.element.classList.contains( 'ck-enabled' ) ).to.true;
expect( view.element.classList.contains( 'ck-disabled' ) ).to.false;
expect( view.element.classList.contains( 'ck-off' ) ).to.true;
} );

Expand Down
10 changes: 5 additions & 5 deletions tests/dropdown/manual/dropdown.html
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
<h2>Empty</h2>

<div id="dropdown" class="ck-reset_all"></div>
<div id="dropdown" class="ck ck-reset_all"></div>

<h2>Dropdown with ListView</h2>

<div id="list-dropdown" class="ck-reset_all"></div>
<div id="list-dropdown" class="ck ck-reset_all"></div>

<h2>Long label (truncated)</h2>

<div id="dropdown-label" class="ck-reset_all"></div>
<div id="dropdown-label" class="ck ck-reset_all"></div>

<h2>Dropdown with ToolbarView</h2>

<div id="dropdown-toolbar" class="ck-reset_all"></div>
<div id="dropdown-toolbar" class="ck ck-reset_all"></div>

<h2>SplitButton Dropdown</h2>

<div id="dropdown-splitbutton" class="ck-reset_all"></div>
<div id="dropdown-splitbutton" class="ck ck-reset_all"></div>
25 changes: 25 additions & 0 deletions tests/list/listitemview.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,20 @@ describe( 'ListItemView', () => {

expect( view.element.classList.contains( 'ck-list__item_active' ) ).to.be.true;
} );

it( 'reacts on view#isEnabled', () => {
expect( view.element.classList ).to.have.length( 2 );

expect( view.element.classList.contains( 'ck-disabled' ) ).to.be.false;

view.set( 'isEnabled', false );

expect( view.element.classList.contains( 'ck-disabled' ) ).to.be.true;

view.set( 'isEnabled', true );

expect( view.element.classList.contains( 'ck-disabled' ) ).to.be.false;
} );
} );

describe( '"style" attribute', () => {
Expand Down Expand Up @@ -132,6 +146,17 @@ describe( 'ListItemView', () => {
view.element.dispatchEvent( new Event( 'click' ) );
expect( spy.calledOnce ).to.be.true;
} );

it( 'does not triggers view#execute event when "click" is fired in DOM and isEnables=false', () => {
const spy = sinon.spy();

view.on( 'execute', spy );

view.isEnabled = false;

view.element.dispatchEvent( new Event( 'click' ) );
sinon.assert.notCalled( spy );
} );
} );
} );

Expand Down

0 comments on commit 76a4d47

Please sign in to comment.