Opened 11 years ago

Closed 11 years ago

#2853 closed Task (fixed)

Move image dialog to trunk.

Reported by: Martin Kou Owned by: Artur Formella
Priority: Normal Milestone: CKEditor 3.0
Component: UI : Dialogs Version: SVN (FCKeditor) - Retired
Keywords: Confirmed Review+ Cc:

Description

The image dialog didn't make it to v3 beta, so it needs to be moved to trunk with a review to both its UI and its logic.

Attachments (4)

2853.patch (43.4 KB) - added by Artur Formella 11 years ago.
2853.2.patch (41.9 KB) - added by Artur Formella 11 years ago.
2853_3.patch (42.0 KB) - added by Artur Formella 11 years ago.
2853_4.patch (42.0 KB) - added by Artur Formella 11 years ago.

Download all attachments as: .zip

Change History (15)

Changed 11 years ago by Artur Formella

Attachment: 2853.patch added

comment:1 Changed 11 years ago by Artur Formella

I got it almost done

comment:2 Changed 11 years ago by Martin Kou

Status: newassigned

comment:3 Changed 11 years ago by Martin Kou

Owner: Martin Kou deleted
Status: assignednew

Oops! I was planning to accept it when I got the table dialog almost done. Turns out Artur got a patch out. :-)

comment:4 Changed 11 years ago by Artur Formella

Keywords: Confirmed added
Owner: set to Artur Formella

Ok. So I will finish it :)

Changed 11 years ago by Artur Formella

Attachment: 2853.2.patch added

comment:5 Changed 11 years ago by Artur Formella

Keywords: Review? added

Are there any suggestions?

comment:6 Changed 11 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed
  • The configurations must not be defined in "levels". They must go all directly under CKEDITOR.config. See #2822.
  • The uploadTab and showPreview setting can be avoided, as discussed previously. Such customizations can be done by API. I would live the browseServer option as this is quite used.
  • The imageDialog variable declaration, other than missing the "var" statement, is polluting the global namespace. It must go inside an anonymous function declaration.
  • All variables set to functions (like onImgLoadEvent and onSizeChange) have no semicolon after the function definition, and some of them is is missing the "var" statement.

Changed 11 years ago by Artur Formella

Attachment: 2853_3.patch added

comment:7 Changed 11 years ago by Artur Formella

Keywords: Review? added; Review- removed

comment:8 Changed 11 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

The pushDefault() and popDefault() functions have been removed recently (see #2803). The patch throw errors because of this. Please update your local copy, fix this issue and provide a new patch.

That's the only problem so far. Getting that fixed will probably bring a Review+ for it.

Changed 11 years ago by Artur Formella

Attachment: 2853_4.patch added

comment:9 Changed 11 years ago by Artur Formella

Keywords: Review? added; Review- removed

Ok. I removed pushDefault() and popDefault().

comment:10 Changed 11 years ago by Frederico Caldeira Knabben

Keywords: Review+ added; Review? removed

comment:11 Changed 11 years ago by Artur Formella

Resolution: fixed
Status: newclosed

Fixed with [3072]

Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy