Opened 15 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)
Change History (11)
Changed 15 years ago by
Attachment: | 5268.patch added |
---|
comment:1 Changed 15 years ago by
Keywords: | Review? added |
---|---|
Milestone: | → CKEditor 3.3 |
Owner: | set to Garry Yao |
Status: | new → assigned |
comment:2 Changed 15 years ago by
Milestone: | CKEditor 3.3 → CKEditor 3.4 |
---|
comment:3 Changed 14 years ago by
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.
- Instead of editor.document.$.documentElement, you should use editor.document.getDocumentElement().
- 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
Attachment: | 5268_2.patch added |
---|
comment:4 follow-up: 5 Changed 14 years ago by
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 Changed 14 years ago by
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
Attachment: | 5268_3.patch added |
---|
comment:6 Changed 14 years ago by
Keywords: | Review? added; Review- removed |
---|
New patch with the following points addressed:
- Performance: Pillar discovery algorithm reduced from O(n2) to linear with table pillar caching enable;
- Better code packings, 0.8K reduced by utilizing module pattern.
comment:7 Changed 14 years ago by
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
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Fixed with [5572] on 3.4.x branch.
The patch recreated the v2 plugin with the same functionality.