Opened 6 years ago

Closed 6 years ago

#7931 closed New Feature (fixed)

New event afterSetMode

Reported by: daveVW Owned by: Sa'ar Zac Elias
Priority: Normal Milestone: CKEditor 3.6.2
Component: General Version: 3.6
Keywords: Cc:

Description

When changing the mode between "wysiwyg" and "source" you have the event "beforeSetMode". It would be nice to also have the event "afterSetMode". I use that event to resize the editor depending on the mode.

CKEDITOR.editor.prototype.setMode = function( mode )
{
     this.fire( 'beforeSetMode', { newMode : mode } );

     ...

     this.fire( 'afterSetMode' );
};

Attachments (1)

7931.patch (2.1 KB) - added by Sa'ar Zac Elias 6 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 6 years ago by Duncan Simey

With a small refinement, this event would also allow me to solve a problem with focus. I'd like the 'afterSetMode' event to have both the oldMode and newMode passed as event parameters.

The problem I'm trying to solve is the lack of focus when changing modes. Click the Source button and note focus is handed back to the browser toolbar; it does not remain in CK editor. I solved the problem by:

CKEDITOR.editor.prototype.setMode = function( mode )
{
      var prevMode = this.mode;

      ...

      if (prevMode)
      {
         modeEditor.focus();
      }
};

If this new event is added, then I'd have been able to solve my focus problem without editing core CKEditor code.

comment:2 Changed 6 years ago by Garry Yao

Status: newpending

Please check if the current event would be enough for your request?

editor.on( 'mode', function()
{
	alert( editor.mode );
})

comment:3 Changed 6 years ago by Duncan Simey

It isn't... 1) it is fired before modeEditor.load() is called - so focus would still be lost. 2) it doesn't have parameters for the old and new modes - so I would be unable to suppress the focus when the editor is first loaded.

comment:4 in reply to:  3 Changed 6 years ago by Sa'ar Zac Elias

Keywords: Discussion added
Status: pendingconfirmed

Replying to duncansimey:

It isn't... 1) it is fired before modeEditor.load() is called - so focus would still be lost. 2) it doesn't have parameters for the old and new modes - so I would be unable to suppress the focus when the editor is first loaded.

1) In fact, the event is fired after the modeEditor.load() is called. 2) True, my suggestion is to pass the previous mode to the "mode" event. Attaching a patch.

Changed 6 years ago by Sa'ar Zac Elias

Attachment: 7931.patch added

comment:5 Changed 6 years ago by Sa'ar Zac Elias

Keywords: HasPatch added

comment:6 Changed 6 years ago by Garry Yao

@duncansimey, does Saar's provided patch address your request?

comment:7 Changed 6 years ago by Duncan Simey

Honest answer - I don't know! modeEditor.load() does stuff that causes focus to be lost; hence needing to place focus on the modeEditor after everything has settled. The 'mode' event used to be fired before modeEditor.load() which is why it was no good for managing focus.

The 'mode' event has been moved into the new mode plugins. There is code that could lose focus after the event is fired, so it's probable that focus control isn't going to work in this event. Hence the need for an 'afterSetMode' event which fires after focus has settled.

Hence I cant work out if the 'mode' event is good enough for managing focus. The only real way is to try it, but I strongly it isn't going to be good enough.

The 'real' problem I'm trying to solve is the loss of focus. The 'afterSetMode' event merely allows a workaround without hacking core code. I've got a separate support ticket raised about the loss of focus View Ticket. The loss of focus is a bug as it both breaks accessibility guidelines and prevents automated testing. I believe the focus problem needs to be fixed regardless of whether the event workaround does the job.

comment:8 Changed 6 years ago by Sa'ar Zac Elias

I got it to work using:

editor.on( 'mode', function( e )
{
    if ( this.mode == 'wysiwyg' && e.data.prevMode == 'source' )
        this.focus();
});

comment:9 Changed 6 years ago by Sa'ar Zac Elias

Keywords: Discussion HasPatch removed
Owner: set to Sa'ar Zac Elias
Status: confirmedreview

The user had reported that the patch fixes his problem.

comment:10 Changed 6 years ago by Garry Yao

Status: reviewreview_passed

Please change "prevMode" to "modeBefore" on committing.

comment:11 Changed 6 years ago by Sa'ar Zac Elias

Milestone: CKEditor 3.6.2
Resolution: fixed
Status: review_passedclosed

We've agreed on previousMode. Fixed with [7192].

Last edited 6 years ago by Sa'ar Zac Elias (previous) (diff)
Note: See TracTickets for help on using tickets.
© 2003 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy