Opened 10 years ago

Closed 10 years ago

#2912 closed Bug (fixed)

V3: Encapsulate the command state change logic inside the CKEDITOR.command class

Reported by: Frederico Caldeira Knabben Owned by: Frederico Caldeira Knabben
Priority: Must have (possibly next milestone) Milestone: CKEditor 3.0
Component: General Version:
Keywords: Confirmed Review+ Cc:

Description

There is a common repetition in the current code:

var command = editor.getCommand( name );
command.state = newState;
command.fire( 'state' );

This logic could be encapsulated inside the CKEDITOR.command class, so we can achieve the same thing with:

editor.getCommand( name ).setState( newState );

Attachments (1)

2912.patch (3.8 KB) - added by Frederico Caldeira Knabben 10 years ago.

Download all attachments as: .zip

Change History (6)

Changed 10 years ago by Frederico Caldeira Knabben

Attachment: 2912.patch added

comment:1 Changed 10 years ago by Frederico Caldeira Knabben

Keywords: Review? added
Status: newassigned

This patch makes it also possible to set the initial state of the command into the command definition.

comment:2 Changed 10 years ago by Garry Yao

Apply the patch in conjunction with #2913, I found some edge cases on the initial state setting:

  • command source initial state setting is override by CKEDITOR.config.startupMode.
  • stateless commands e.g. smiley should not respond to inital state definition of CKEDITOR.TRISTATE_OFF.

comment:3 Changed 10 years ago by Garry Yao

Besides, in case we choose not to get initial focus on editor instances, stateful commands e.g. Unlink and Indent require manually config their state to CKEDITOR.TRISTATE_OFF via CKEDITOR.commandDefinition::state, which is now missing in _source\core\commanddefinition.js, not sure if this's out of the scope of this patch.

comment:4 Changed 10 years ago by Martin Kou

Keywords: Review+ added; Review? removed

The purpose of this ticket is to simplify code, which it does.

Let's open another bug ticket for other issues.

comment:5 Changed 10 years ago by Frederico Caldeira Knabben

Resolution: fixed
Status: assignedclosed

Fixed with [3057]. Click here for more info about our SVN system.

Note: See TracTickets for help on using tickets.
© 2003 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy