Opened 11 years ago

Closed 8 years ago

Last modified 8 years ago

#2988 closed New Feature (fixed)

V3: Document Properties plugin

Reported by: Artur Formella Owned by: Sa'ar Zac Elias
Priority: Normal Milestone: CKEditor 3.6
Component: UI : Dialogs Version:
Keywords: Cc:

Description

Document Properties plugin with Select Color dialog.

Attachments (7)

2988_ref.patch (25.9 KB) - added by Sa'ar Zac Elias 9 years ago.
INCOMPLETE implementation of the plugin
2988.patch (28.5 KB) - added by Sa'ar Zac Elias 9 years ago.
2988_2.patch (27.0 KB) - added by Sa'ar Zac Elias 8 years ago.
2988_3.patch (27.8 KB) - added by Sa'ar Zac Elias 8 years ago.
2988_language_updates.patch (102.6 KB) - added by Wiktor Walc 8 years ago.
2988_4.patch (28.5 KB) - added by Sa'ar Zac Elias 8 years ago.
2988_5.patch (28.6 KB) - added by Sa'ar Zac Elias 8 years ago.

Download all attachments as: .zip

Change History (33)

comment:1 Changed 11 years ago by Artur Formella

Owner: set to Artur Formella

comment:2 Changed 11 years ago by Frederico Caldeira Knabben

This one needs full page support, which is not yet available and it's not our priority now. Please hold on with this ticket.

comment:3 Changed 11 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.0CKEditor 3.x
Type: BugNew Feature

comment:4 Changed 11 years ago by Artur Formella

#628 is connected with ticket.

comment:5 Changed 10 years ago by Artur Formella

Owner: Artur Formella deleted

comment:6 Changed 10 years ago by Frederico Caldeira Knabben

#4588 has been marked as DUP.

comment:7 Changed 9 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.x

Milestone CKEditor 3.x deleted

comment:8 Changed 9 years ago by Garry Yao

Milestone: CKEditor 3.6

comment:9 Changed 9 years ago by Sa'ar Zac Elias

Owner: set to Sa'ar Zac Elias
Status: newassigned

Changed 9 years ago by Sa'ar Zac Elias

Attachment: 2988_ref.patch added

INCOMPLETE implementation of the plugin

Changed 9 years ago by Sa'ar Zac Elias

Attachment: 2988.patch added

comment:10 Changed 9 years ago by Sa'ar Zac Elias

Status: assignedreview

comment:11 Changed 8 years ago by Garry Yao

Status: reviewreview_failed

Looks cool, but the patch breaks IE6/7, and link color options doesn't persist between mode switches - as it's quite difficult to handle them correctly, how about leave it for now?

comment:12 in reply to:  11 Changed 8 years ago by Sa'ar Zac Elias

Status: review_failedreview

Replying to garry.yao:

the patch breaks IE6/7

Another case of special usage of getAttribute, fixed.

link color options doesn't persist between mode switches - as it's quite difficult to handle them correctly, how about leave it for now?

I removed those options, and merged the remaining options in that tab with the background tab, creating the new "design" tab.
In addition, a few minor glitches had been fixed.

Changed 8 years ago by Sa'ar Zac Elias

Attachment: 2988_2.patch added

comment:13 Changed 8 years ago by Garry Yao

Status: reviewreview_failed

Some glitches around charset were found:

  1. Confirm an empty dialog will result in the following meta:
    <meta content="text/html; charset=" http-equiv="content-type" />
    
  2. "text/html" will be forced that show no respect of what specified by user;
  3. Other way of specifying charset is not supported:
    <meta http-equiv="charset" content="iso-8859-1">
    

The plugin should not be loaded at all in non-fullpage mode.

comment:14 Changed 8 years ago by Anna Tomanek

Just one minor note regarding the dialog window texts. I would suggest changing the following label:

Is:
Nonscrolling (Fixed) Background
Should be:
Non-scrolling (Fixed) Background

Changed 8 years ago by Sa'ar Zac Elias

Attachment: 2988_3.patch added

comment:15 Changed 8 years ago by Sa'ar Zac Elias

Status: review_failedreview

All of the above suggestions, including Anna's, are covered in the 3rd patch.

Changed 8 years ago by Wiktor Walc

Attachment: 2988_language_updates.patch added

comment:16 Changed 8 years ago by Wiktor Walc

Please make sure to commit also language updates when closing this ticket (took me a while to translate everything ;)).

Not sure if storing 'previewHtml' in language makes sense, I guess "Lorem ipsum" could be reused from the Image dialog.

comment:17 in reply to:  16 Changed 8 years ago by Garry Yao

Replying to wwalc:

Please make sure to commit also language updates when closing this ticket (took me a while to translate everything ;)).

Thanks Wiktor, I didn't recognize there's still people who master every language on this planet ;)

Not sure if storing 'previewHtml' in language makes sense, I guess "Lorem ipsum" could be reused from the Image dialog.

Considering that we need a link in preview.

comment:18 Changed 8 years ago by Garry Yao

Status: reviewreview_failed

Saved content type doesn't looks like a good idea, check this:

  1. First load with xhtml content-type:
    <meta content="application/xhtml+xml; charset=utf-8" http-equiv="content-type" />
    
  2. Open dialog do nothing and close.
  3. Second load with no meta tag of content-type specified.
  4. Open dialog and specify any charset;
  5. Check output:
  • Actual Result: application/xhtml+xml specified in the first time was enforced.

comment:19 Changed 8 years ago by Garry Yao

Also we must load the plugin into fullpage sample page btw.

Changed 8 years ago by Sa'ar Zac Elias

Attachment: 2988_4.patch added

comment:20 Changed 8 years ago by Sa'ar Zac Elias

Status: review_failedreview

comment:21 Changed 8 years ago by Garry Yao

Status: reviewreview_failed

Everything looks good right now except body text color is always enforced to white once open dialog, it should be instead leaving blank thus remains "inherit".

Changed 8 years ago by Sa'ar Zac Elias

Attachment: 2988_5.patch added

comment:22 Changed 8 years ago by Sa'ar Zac Elias

Status: review_failedreview

Changes will be applied after the field contents had been changed.

comment:23 Changed 8 years ago by Garry Yao

Status: reviewreview_passed

Nice done!

comment:24 Changed 8 years ago by Sa'ar Zac Elias

Resolution: fixed
Status: review_passedclosed

Fixed with [6788].

comment:25 Changed 8 years ago by Sa'ar Zac Elias

Addes icon coordinates to other skinds with [6789].

Version 0, edited 8 years ago by Sa'ar Zac Elias (next)

comment:26 Changed 8 years ago by Wiktor Walc

It looks like translations were not committed... I have added them with [7028].

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