Ticket #5105 (review New Feature)

Opened 4 years ago

Last modified 4 years ago

Simplify getContentElement so it uses only elementId

Reported by: alfonsoml 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

5105.patch (17.8 KB) - added by alfonsoml 4 years ago.
First approach

Change History

comment:1 Changed 4 years ago by fredck

  • 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 4 years ago by alfonsoml

First approach

comment:2 Changed 4 years ago by alfonsoml

  • Keywords Review? added
  • Milestone set to 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 4 years ago by fredck

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