Opened 14 years ago

Closed 14 years ago

#5268 closed New Feature (fixed)

Port DragResizeTable to V3

Reported by: Alfonso Martínez de Lizarrondo Owned by: Garry Yao
Priority: Normal Milestone: CKEditor 3.4
Component: General Version: 3.2
Keywords: Confirmed Review+ Cc:

Description

Several people have requested the upgrade of this plugin

Attachments (3)

5268.patch (12.9 KB) - added by Garry Yao 14 years ago.
5268_2.patch (11.5 KB) - added by Garry Yao 14 years ago.
5268_3.patch (12.8 KB) - added by Garry Yao 14 years ago.

Download all attachments as: .zip

Change History (11)

Changed 14 years ago by Garry Yao

Attachment: 5268.patch added

comment:1 Changed 14 years ago by Garry Yao

Keywords: Review? added
Milestone: CKEditor 3.3
Owner: set to Garry Yao
Status: newassigned

The patch recreated the v2 plugin with the same functionality.

comment:2 Changed 14 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.3CKEditor 3.4

comment:3 Changed 14 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed
  • This one is supposed to be an extra plugin, not included in the default list of plugins.
  • Let's also separate things a bit, to not make the core bloated. The buildTableMap function can even be appended to the CKEDITOR.tools object, but it should still be defined in the tabletools plugin. That plugin will then enter in the requires list for the tableresize plugin.

In the plugin code:

  • We don't need to have those functions defined in the private "_" property of the ColumnResizer class. I believe most of them can be private functions in the main anonymous function.
  • The class name should be columnResizer.
  • There are some jslint warnings in the code.
  • It happens to have decimal precision in the sizes, like "42.5px". Let's round them to have only integers.

Changed 14 years ago by Garry Yao

Attachment: 5268_2.patch added

comment:4 Changed 14 years ago by Garry Yao

Keywords: Review? added; Review- removed

We don't need to have those functions defined in the private "_" property of the ColumnResizer class.

I don't see problems in this case, since they're event handlers that bind to the columnResizer instance, scoping them as closure bring runtime performance penalty as well.

comment:5 in reply to:  4 Changed 14 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

Replying to garry.yao:

I don't see problems in this case, since they're event handlers that bind to the columnResizer instance,

The benefit of having them our of the "_" is that the code will compact much better on release. Other than having the function names reduced, their calls will result very shorter.

scoping them as closure bring runtime performance penalty as well.

Yes, a book theory. You'll never be able to demonstrate it in this specific case.

Changed 14 years ago by Garry Yao

Attachment: 5268_3.patch added

comment:6 Changed 14 years ago by Garry Yao

Keywords: Review? added; Review- removed

New patch with the following points addressed:

  1. Performance: Pillar discovery algorithm reduced from O(n2) to linear with table pillar caching enable;
  2. Better code packings, 0.8K reduced by utilizing module pattern.

comment:7 Changed 14 years ago by Frederico Caldeira Knabben

Keywords: Review+ added; Review? removed

The performance enhancements are noticeable, and wonderful. Congratulations for it.

Please have this one committed into the 3.4.x branch.

comment:8 Changed 14 years ago by Garry Yao

Resolution: fixed
Status: assignedclosed

Fixed with [5572] on 3.4.x branch.

Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy