Ticket #4210 (closed New Feature: fixed)

Opened 5 years ago

Last modified 4 years ago

CKEditor plugin for jQuery

Reported by: tobiasz.cudnik Owned by: tobiasz.cudnik
Priority: Normal Milestone: CKEditor 3.1
Component: General Version:
Keywords: Confirmed Review+ Cc:

Description (last modified by tobiasz.cudnik) (diff)

Create a jQuery plugin, which will allow jQuery users easily integrate rich text editing into their applications.

Features

  • All API mockups
    • .ckeditor( func, config )
      Create new editor instance and fire callback when ready. Allowed are all elements, not only textareas.
    • .ckeditorGet()
      Get already existing editor instance.
    • .val()
      val() on textareas returns/sets data on the editor
    • jQuery.ckeditorConfig()
      Change global CKEditor config
  • Forwarding some CKEditor events (namespaced)
    • newInstance
    • setData
    • getData
    • destroy
  • Submit integration
    • normal submit
    • ajaxSubmit

Below mockup of planned API.

Editor creation

// chainably transform textareas into CKEditor instance
$('textarea').ckeditor()

// extensive example
$('#editors textarea')
  .eq(0).ckeditor({ lang: 'pl', width: 300 }).end()
  .eq(1).ckeditor({ width: 500, height: 400 }).end()
  // more then one at once
  .slice(2).ckeditor({ lang: 'ar' }).end()

Internal API access

// get data from editor
$('textarea').ckeditor(function(){
  alert(this.getData());
});

// set data into editor
$('textarea').ckeditor(function(){
  this.setData("New editor content");
});

// change ui color
$('textarea').ckeditor(function(){
  this.setUiColor('#FFFFFF');
});

// remove editor from the page
$('textarea').ckeditor(function(){
  this.destroy();
});

// use editor synchronously
// requires it to be created earlier
var editor = $('textarea').ckeditorGet();
alert( editor.getData() );

jQuery integration

// use val() to get data
$('textarea:first').ckeditor(function( textarea ){
  $(textarea).val();
});

// use val() to set data
$('textarea:first').ckeditor(function( textarea ){
  $(textarea).val("New editor content");
});

// listen to creation event of any CKEditor
$( document ).bind( 'instanceCreated.ckeditor', function( editor ){
  alert( editor.name + ' has been created' );
});

Possible, not confirmed:

  1. Easy editor's content lookup using selectors

There is also interesting topic about framework integration on our forum.

Attachments

4210.patch (6.8 KB) - added by tobiasz.cudnik 5 years ago.
4210_2.patch (6.8 KB) - added by tobiasz.cudnik 5 years ago.
4210_3.patch (8.4 KB) - added by tobiasz.cudnik 5 years ago.
4210_4.patch (10.0 KB) - added by tobiasz.cudnik 5 years ago.
4210_5.patch (11.2 KB) - added by tobiasz.cudnik 5 years ago.

Change History

comment:1 Changed 5 years ago by tobiasz.cudnik

  • Status changed from new to assigned
  • Owner set to tobiasz.cudnik

comment:2 Changed 5 years ago by tobiasz.cudnik

  • Description modified (diff)

comment:3 Changed 5 years ago by fredck

  • Description modified (diff)

comment:4 Changed 5 years ago by tobiasz.cudnik

  • Keywords Confirmed added
  • Description modified (diff)

comment:5 Changed 5 years ago by tobiasz.cudnik

  • Description modified (diff)

comment:6 Changed 5 years ago by tobiasz.cudnik

  • Description modified (diff)

comment:7 Changed 5 years ago by tobiasz.cudnik

  • Description modified (diff)

comment:8 Changed 5 years ago by tobiasz.cudnik

  • Description modified (diff)

comment:9 Changed 5 years ago by tobiasz.cudnik

  • Description modified (diff)

Changed 5 years ago by tobiasz.cudnik

comment:10 Changed 5 years ago by tobiasz.cudnik

This patch implements all things described in actual description, although it's not subject to review yet.

Test Case is available online.

To get it running you have to execute this line after loading jQuery:

CKEDITOR.plugins.load( 'jquery', function()
{
  // loaded
} );

comment:11 Changed 5 years ago by tobiasz.cudnik

  • Description modified (diff)

comment:12 follow-up: ↓ 13 Changed 5 years ago by garry.yao

Great idea and implemenations, the plugin should appear at jQuery Plugins Repository very soon :). Though I have the following points regard it:

  1. Could we overload 'jQuery.fn.ckeditor' instead of a new 'jQuery.fn.ckeditorGet' in following form?
    • Instance creation when instance doesn't exist with the specified config and invoke the callback if any.
    • Return instance when it's already existed
  2. We should manually filter out unmatched elements out in the jQuery result set ( textarea/div currently ).
  3. It seems 'jquerySyncTextareaOnSubmit' is unnecessary since the editor has already listened to 'submit' event for textarea so it's integrate 'jQuery.fn.trigger' naturally.
  4. It will be nice to see a 'autoLoad_jqueryPlugin' to auto load this plugin if jQuery is on the page.

comment:13 in reply to: ↑ 12 Changed 5 years ago by tobiasz.cudnik

Replying to garry.yao:

  1. Could we overload 'jQuery.fn.ckeditor' instead of a new 'jQuery.fn.ckeditorGet' in following form?
    • Instance creation when instance doesn't exist with the specified config and invoke the callback if any.
    • Return instance when it's already existed

This can't be done because of following:

  • jQuery chaining prevents returning anything other then jQuery object (if we want to preserve chain)
  • This could create troubles for end developers which will expect synchronous behavior (returning instance at once) when it wont be possible (editor wasn't ready then)
  • End developer most of the time would have to write both synchronous and asynchronous version of code dealing with CKEditor. This is one possible situation:
    function myCode()
    {
      this.setUiColor( '#FFFFFF' );
    }
    var editor = $( 'textarea' ).ckeditor( myCode );
    if ( editor )
      myCode.call( editor );
    
    Which is quite terrible and raises questions like "does myCode was executed as callback or not ?".

That's why synchronous and asynchronous calls have been separated into 2 explicit methods.

Replying to garry.yao:

  1. It seems 'jquerySyncTextareaOnSubmit' is unnecessary since the editor has already listened to 'submit' event for textarea so it's integrate 'jQuery.fn.trigger' naturally

Native submit listener which is used by config.autoUpdateElement is probably triggered too late to support AJAX forms, thats why this is done using native jQuery event system. Although merging those settings is possible and reasonable.

Changed 5 years ago by tobiasz.cudnik

Changed 5 years ago by tobiasz.cudnik

comment:14 Changed 5 years ago by tobiasz.cudnik

Updated TC is available in new tests branch as plugins/jquery/jquery.html.

What's missing now is autoload support and cross browser tests.

Changed 5 years ago by tobiasz.cudnik

comment:15 Changed 5 years ago by tobiasz.cudnik

  • Keywords Review? added

Latest patch adds new config option CKEDITOR.config.autoLoad_jqueryPlugin witch will perform loading on bootstrap. Docs are quite complete, but i didn't run jstoolkit on them.

All browsers test passes with some issues regarding testing framework. Running tests separately allows them to pass. This concerns old testing env, not new one based on CKTester as i've been still using it for this ticket.

comment:16 Changed 5 years ago by garry.yao

As dicussed with Tobias, the TCs should be updated and there'll be also some correction regarding the usage of 'setData' event.

Changed 5 years ago by tobiasz.cudnik

comment:17 Changed 5 years ago by garry.yao

  • Keywords Review+ added; Review? removed

As we discussed, please commit the patch without the autoLoad feature.

comment:18 Changed 5 years ago by tobiasz.cudnik

  • Type changed from Bug to New Feature

New branch has been created for this feature - CKEditor/branches/features/4210.

comment:19 Changed 5 years ago by tobiasz.cudnik

Correction - this feature is available in 3.1.x version branch in /CKEditor/branches/versions/3.1.x.

comment:20 Changed 5 years ago by tobiasz.cudnik

  • Keywords Review? added; Review+ removed

Yet another change - edicated branch is available in branches/features/4210/3.1.x.

New directory has been introduced - _sources/adapters.

TC is updated with setData.ckeditor event binding to control async operations.

comment:21 Changed 5 years ago by tobiasz.cudnik

New sample is available in branches/features/4210/3.1.x/_samples/jqueryadapter.html. Although there are issues when using releaser, as it truncates new sample to default one.

comment:22 Changed 5 years ago by fredck

  • Keywords Review- added; Review? removed
  • I don't understand why the branch has been set at branches/features/4210/3.1.x (I mean, why the "3.1.x"?), but that's ok... not an issue.
  • To make things clearer, we should change the order the scripts are loaded in the sample. In this way we leave the CKEditor stuff grouped:
  1. JQuery
  2. CKEditor
  3. Adapter
  • The sample code must also set the "release" path for the adapter, which is "../adapters/jquery/adapter.js".
  • Usually we'll have a single file for the adapters. As those are to be used inside <script> tags in the pages, I think it would be a better thing to not have folders for each of them, so end developers could simply use "/ckeditor/adapters/jquery.js". It would look less cumbersome.
  • The "setData" event firing logic is a bit unclear. It also forces the getData() call even when not needed, which impacts on performance. I think a better solution could be proposed after #4445.
  • I think the new "autoUpdateElementJquery" config is not needed as this thing can be controlled by the autoUpdateElement config, and a local variable could be used to save the original value of it. If the autoUpdateElementJquery setting is still needed, it should be prefixed with an underscore as it has a private scope.
  • CKEDITOR.on( 'instanceReady' ) is being used to setup the instances. The problem is that it is that on() is being called for every single instance and every instance creation is passed for each of the listeners. This means that the setup is being done multiple times for multiple editor instances. We should actually have a single CKEDITOR.on( 'instanceReady' ) being defined, or instead having the "per instance" instanceReady event being listened.
  • $element.bind( 'destroy.ckeditor' ) calls is being used by our code. We don't need to take the long road for it and we can safely use editor.on( 'destroy' ).
  • The jqueryOverrideVal will be always "true". We're not giving the chance to set it to false. We could simply consider the default value as true and then check if its undefined or "true".

comment:23 follow-up: ↓ 25 Changed 4 years ago by tobiasz.cudnik

  • Keywords Review? added; Review- removed
  • Description modified (diff)
  • I don't understand why the branch has been set at branches/features/4210/3.1.x (I mean, why the "3.1.x"?), but that's ok... not an issue.

I'm not sure about 3.1.x, but because of small misunderstanding this is actually a second branch, which i've decided not to create third time.

  • To make things clearer, we should change the order the scripts are loaded in the sample. In this way we leave the CKEditor stuff grouped:

Fixed.

  • The sample code must also set the "release" path for the adapter, which is "../adapters/jquery/adapter.js".

Fixed.

  • Usually we'll have a single file for the adapters. As those are to be used inside <script> tags in the pages, I think it would be a better thing to not have folders for each of them, so end developers could simply use "/ckeditor/adapters/jquery.js". It would look less cumbersome.

Fixed.

  • The "setData" event firing logic is a bit unclear. It also forces the getData() call even when not needed, which impacts on performance. I think a better solution could be proposed after #4445.

"dataReady" event makes "setData" forwarding easier. Although it's not perfect it works in this case. It's important to notice, that it needs code from actual trunk.

I've removed getData call and param, as the only way to skip performance penalty would be passing lazy func getter which is almost the same as editor.getData() call.

  • I think the new "autoUpdateElementJquery" config is not needed as this thing can be controlled by the autoUpdateElement config, and a local variable could be used to save the original value of it. If the autoUpdateElementJquery setting is still needed, it should be prefixed with an underscore as it has a private scope.

The reason for this was order of fired events for Ajax Forms plugin, which handles (and cancels) "submit" event on his own.

  • CKEDITOR.on( 'instanceReady' ) is being used to setup the instances. The problem is that it is that on() is being called for every single instance and every instance creation is passed for each of the listeners. This means that the setup is being done multiple times for multiple editor instances. We should actually have a single CKEDITOR.on( 'instanceReady' ) being defined, or instead having the "per instance" instanceReady event being listened.

Fixed.

  • The jqueryOverrideVal will be always "true". We're not giving the chance to set it to false. We could simply consider the default value as true and then check if its undefined or "true".

I forgot it's a global option and doesn't come from the instance. Fixed.

I've also extended the docs with some informations about provided jQuery events.

Mentioned fixes were commited with [4465], [4467], [4468], [4469].

comment:24 Changed 4 years ago by tobiasz.cudnik

I've missed instances creation fixes in previous commits, they are in [4473].

comment:25 in reply to: ↑ 23 Changed 4 years ago by fredck

Replying to tobiasz.cudnik:

"dataReady" event makes "setData" forwarding easier. Although it's not perfect it works in this case. It's important to notice, that it needs code from actual trunk.

You should have updated the branch so. I just did it with [4476].

comment:26 Changed 4 years ago by fredck

  • Keywords Review+ added; Review? removed

Ok... go ahead merging this one into the 3.1 branch.

comment:27 Changed 4 years ago by tobiasz.cudnik

  • Keywords Review? added; Review+ removed

Merged with [4480].

Thank you for merging with the trunk.

comment:28 Changed 4 years ago by tobiasz.cudnik

  • Status changed from assigned to closed
  • Keywords Review+ added; Review? removed
  • Resolution set to fixed

Reverting missing Review+ keyword, closing.

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