Opened 14 years ago

Last modified 13 years ago

#5105 review New Feature

Simplify getContentElement so it uses only elementId

Reported by: Alfonso Martínez de Lizarrondo Owned by:
Priority: Normal Milestone:
Component: UI : Dialogs Version:
Keywords: Cc:


The current definition of getContentElement, getValueOf and setValueOf requires both the pageId as well as the elementId, that means that each element is bound to the page where it has been defined, but it would allow greater flexibility for customization if those functions didn't require the pageId, store all the elements in a dialog in a new collection for example this._.allContents[elementId]

This would allow to just move one element from one page to another in the contents definition, with no need to adjust anything in the javascript code.

Are there any drawbacks?

Attachments (1)

5105.patch (17.8 KB) - added by Alfonso Martínez de Lizarrondo 13 years ago.
First approach

Download all attachments as: .zip

Change History (4)

comment:1 Changed 14 years ago by Frederico Caldeira Knabben

Keywords: Confirmed added; Discussion removed

That makes sense for me also. I think Martin wanted to avoid id name collision between the pages, but this is something that should not happen.

The dialog system needs a rewritting, so this one may be part of it. (must be backwards compatible)

Changed 13 years ago by Alfonso Martínez de Lizarrondo

Attachment: 5105.patch added

First approach

comment:2 Changed 13 years ago by Alfonso Martínez de Lizarrondo

Keywords: Review? added
Milestone: CKEditor 3.4

The patch is a first approach at this.

I only made the changes in the link and anchor dialogs, and a required fix for the image.

It's mostly backwards compatible, but if there are two elements with the same id (as happens in the image dialog), then only the last one will work correctly.

Also, if we want to provide a simple entry in #5915 to remove tabs or contents, the name of the "upload" inputs will have to be adjusted because it matches the id of the tab.

So we must add this info in the release notes and the Upgrade page at the wiki stating the renamed fields and a word of warning if people also used some fields with the same id.

Are there any problems with this way or can I go ahead and adjust the rest of the dialogs?

comment:3 Changed 13 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.4
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