Opened 14 years ago

Closed 13 years ago

#5587 closed Bug (fixed)

Visual improvements in dialogs

Reported by: Wiktor Walc Owned by: Garry Yao
Priority: Normal Milestone: CKEditor 3.5
Component: UI : Dialogs Version: 3.0
Keywords: Cc:

Description

While designing dialogs we used to check how they look having the English language enabled.

It might be worth checking how CKEditor looks like in other popular languages, like German, Russian and also in languages which translation files are much bigger than en.js (like Thai).

Changing the design of some dialogs would be a relatively small change in the code, but a possibly big improvement in the quality of user interface.

Attachments (16)

image_dialog_de.png (30.7 KB) - added by Wiktor Walc 14 years ago.
image_dialog_de_modified.png (34.6 KB) - added by Wiktor Walc 14 years ago.
link_dialog_de.png (24.0 KB) - added by Wiktor Walc 14 years ago.
link_dialog_de_modified.png (23.8 KB) - added by Wiktor Walc 14 years ago.
table_cell_dialog_de.png (36.0 KB) - added by Wiktor Walc 14 years ago.
table_cell_dialog_de_modified.png (34.2 KB) - added by Wiktor Walc 14 years ago.
5587.patch (5.7 KB) - added by Sa'ar Zac Elias 14 years ago.
krst_14.35.24.png (46.1 KB) - added by Krzysztof Studnik 14 years ago.
Table cell properties dialog box under Opera 10.61, CKE 3.4.1 Nightly
5587_2.patch (6.1 KB) - added by Sa'ar Zac Elias 13 years ago.
5587_3.patch (2.7 KB) - added by Sa'ar Zac Elias 13 years ago.
5587_4.patch (6.5 KB) - added by Garry Yao 13 years ago.
5587_5.patch (9.9 KB) - added by Garry Yao 13 years ago.
2010-11-25-120310_1178x624_scrot.png (61.6 KB) - added by Tobiasz Cudnik 13 years ago.
IE7 strict
2010-11-25-120436_474x356_scrot.png (18.2 KB) - added by Tobiasz Cudnik 13 years ago.
5587_6.patch (14.0 KB) - added by Garry Yao 13 years ago.
5587_7.patch (11.9 KB) - added by Garry Yao 13 years ago.

Download all attachments as: .zip

Change History (45)

Changed 14 years ago by Wiktor Walc

Attachment: image_dialog_de.png added

Changed 14 years ago by Wiktor Walc

Changed 14 years ago by Wiktor Walc

Attachment: link_dialog_de.png added

Changed 14 years ago by Wiktor Walc

Attachment: link_dialog_de_modified.png added

Changed 14 years ago by Wiktor Walc

Attachment: table_cell_dialog_de.png added

Changed 14 years ago by Wiktor Walc

comment:1 Changed 14 years ago by Wiktor Walc

The attached images show the difference between how dialogs look right now and how they could look like.

comment:2 Changed 14 years ago by Garry Yao

Keywords: Discussion added

It make sense to introduce an "auto-wrap" feature for dialog fields, the idea is that when dialog has a fixed size and thus is not big enough for horizontal label layout (e.g. when resized), input could wrap it self to form a layout same as if labelLayout : 'horizontal' is specified.

comment:3 Changed 14 years ago by Frederico Caldeira Knabben

Component: GeneralUI : Dialogs
Keywords: Discussion removed

KISS for now... let's simply fix the evident issues, rearranging the dialog structure.

I'm totally ok for the image dialog changes.

The cell properties dialog must definitely be reviewed, and have a new design just like we did for the table dialog. Basically, we should always try to have labels on top of the fields.

comment:4 Changed 14 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.4CKEditor 3.5

comment:5 Changed 14 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.4.1CKEditor 3.5

comment:6 Changed 14 years ago by Sa'ar Zac Elias

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

Changed 14 years ago by Sa'ar Zac Elias

Attachment: 5587.patch added

comment:7 Changed 14 years ago by Sa'ar Zac Elias

Status: assignedreview

I also included a fix for a small layout bug in which the choose color button had the wrong margin in the tableCell dialog in RTL UI.

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

#6264 is a dup.

Changed 14 years ago by Krzysztof Studnik

Attachment: krst_14.35.24.png added

Table cell properties dialog box under Opera 10.61, CKE 3.4.1 Nightly

comment:9 Changed 13 years ago by Garry Yao

Status: reviewreview_failed

The two color boxes looks really unhealthy as Krst pointed out, we should go with the following layout I think

[label]: 
[field][picker]

Changed 13 years ago by Sa'ar Zac Elias

Attachment: 5587_2.patch added

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

Status: review_failedreview

comment:11 Changed 13 years ago by Garry Yao

Status: reviewreview_passed

comment:12 Changed 13 years ago by Sa'ar Zac Elias

Resolution: fixed
Status: review_passedclosed

Fixed with [5930].

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

Resolution: fixed
Status: closedreopened

The last patch brought some regressions for IE7 and Quirks.

Changed 13 years ago by Sa'ar Zac Elias

Attachment: 5587_3.patch added

comment:14 Changed 13 years ago by Sa'ar Zac Elias

Status: reopenedreview

comment:15 Changed 13 years ago by Garry Yao

Status: reviewreview_failed

IE7 is the only browser that doesn't get applied the [defined left margin http://dev.fckeditor.net/browser/CKEditor/trunk/_source/plugins/tabletools/dialogs/tableCell.js#L486].

comment:16 Changed 13 years ago by Frederico Caldeira Knabben

Other than any other issue, the mixed formatting we have after patch is NOT acceptable:

  • Labels must be at the top of ALL fields, not only half of them.
  • Spacing between fields must be coherent and the same in all cases (e.g. there is space between background and border color fields)

comment:17 Changed 13 years ago by Sa'ar Zac Elias

Owner: Sa'ar Zac Elias deleted
Status: review_failednew

As I don't have the same result as you all get, I'd better leave that ticket to someone else and confirm that it doesn't brake my side.

comment:18 Changed 13 years ago by Sa'ar Zac Elias

Status: newconfirmed

Changed 13 years ago by Garry Yao

Attachment: 5587_4.patch added

comment:19 Changed 13 years ago by Garry Yao

Owner: set to Garry Yao
Status: confirmedreview

Changes to the dialog plugin are mandatory to make dialog body auto expands in IE7 while doesn't introduce any side effects for other browsers, note that now all browsers expect for IE quirks are now auto expanding.

comment:20 Changed 13 years ago by Garry Yao

Milestone: CKEditor 3.5CKEditor 3.4.3
Type: TaskBug

comment:21 Changed 13 years ago by Sa'ar Zac Elias

Status: reviewreview_failed
  • Let's have the same width for the "width" and "height" fields in the cellProperties dialog.
  • Also, the top margin that was added to the the buttons is only valid for Kama appearently, it doesn't look good on other skins.

Changed 13 years ago by Garry Yao

Attachment: 5587_5.patch added

comment:22 Changed 13 years ago by Garry Yao

Status: review_failedreview

We need to resort to skin part to resolve those issues from Saar.

Changed 13 years ago by Tobiasz Cudnik

IE7 strict

Changed 13 years ago by Tobiasz Cudnik

comment:23 Changed 13 years ago by Tobiasz Cudnik

Status: reviewreview_failed

Ive attached 2 screens from IE7 strict, one from LTR, second from RTL sample. Both contain an error.

Besides that, what with the Image dialog, it won't be included?

Changed 13 years ago by Garry Yao

Attachment: 5587_6.patch added

comment:24 Changed 13 years ago by Garry Yao

Status: review_failedreview

Updates in new patch:

  1. Have picker button fixed width as with footer buttons;
  2. Re-layouted "image" dialog.

comment:25 Changed 13 years ago by Frederico Caldeira Knabben

Status: reviewreview_failed

The file naming convention for the new CSS file is wrong. It must be all lowercased.

Ideally, we should not have plugin specific files inside skins. Even if not a big issue, if we do that now, it'll open a precedence for this, and we may start having several of these files there, which is wrong.

If a specific dialog element type is missing (in this case the button), the right approach would be instead creating the infrastructure to support this *generic* element. Right now it would be used by this dialog only, but it could be then easily reused by other dialogs.

But, aren't buttons already used in dialogs in that way?

comment:26 Changed 13 years ago by Frederico Caldeira Knabben

Other than that, it's clear that the fields are not all left aligned in the Image dialog. Probably an old issue, but it's a fact.

comment:27 in reply to:  25 Changed 13 years ago by Garry Yao

Status: review_failedreview

Replying to fredck:

Ideally, we should not have plugin specific files inside skins. Even if not a big issue, if we do that now, it'll open a precedence for this, and we may start having several of these files there, which is wrong.

I agree, but unfortunately there's currently no way of introduce plugin specific CSS without bloating dialog.css which just slows down dialog loading, this skin part hack was initially used in "templates" dialog to resolve such an embarrassment, but eventually we'll still need a solution for this.

Changed 13 years ago by Garry Yao

Attachment: 5587_7.patch added

comment:28 Changed 13 years ago by Tobiasz Cudnik

Status: reviewreview_passed

comment:29 Changed 13 years ago by Garry Yao

Milestone: CKEditor 3.4.3CKEditor 3.5
Resolution: fixed
Status: review_passedclosed

Fixed with [6137].

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