Opened 15 years ago
Last modified 14 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: |
Description
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)
Change History (4)
comment:1 Changed 15 years ago by
Keywords: | Confirmed added; Discussion removed |
---|
comment:2 Changed 14 years ago by
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 14 years ago by
Milestone: | CKEditor 3.4 |
---|
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)