Ticket #5587 (closed Bug: fixed)

Opened 4 years ago

Last modified 4 years ago

Visual improvements in dialogs

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

image_dialog_de.png (30.7 KB) - added by wwalc 4 years ago.
image_dialog_de_modified.png (34.6 KB) - added by wwalc 4 years ago.
link_dialog_de.png (24.0 KB) - added by wwalc 4 years ago.
link_dialog_de_modified.png (23.8 KB) - added by wwalc 4 years ago.
table_cell_dialog_de.png (36.0 KB) - added by wwalc 4 years ago.
table_cell_dialog_de_modified.png (34.2 KB) - added by wwalc 4 years ago.
5587.patch (5.7 KB) - added by Saare 4 years ago.
krst_14.35.24.png (46.1 KB) - added by krst 4 years ago.
Table cell properties dialog box under Opera 10.61, CKE 3.4.1 Nightly
5587_2.patch (6.1 KB) - added by Saare 4 years ago.
5587_3.patch (2.7 KB) - added by Saare 4 years ago.
5587_4.patch (6.5 KB) - added by garry.yao 4 years ago.
5587_5.patch (9.9 KB) - added by garry.yao 4 years ago.
2010-11-25-120310_1178x624_scrot.png (61.6 KB) - added by tobiasz.cudnik 4 years ago.
IE7 strict
2010-11-25-120436_474x356_scrot.png (18.2 KB) - added by tobiasz.cudnik 4 years ago.
5587_6.patch (14.0 KB) - added by garry.yao 4 years ago.
5587_7.patch (11.9 KB) - added by garry.yao 4 years ago.

Change History

Changed 4 years ago by wwalc

Changed 4 years ago by wwalc

Changed 4 years ago by wwalc

Changed 4 years ago by wwalc

Changed 4 years ago by wwalc

Changed 4 years ago by wwalc

comment:1 Changed 4 years ago by wwalc

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

comment:2 Changed 4 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 4 years ago by fredck

  • Keywords Discussion removed
  • Component changed from General to UI : Dialogs

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

  • Milestone changed from CKEditor 3.4 to CKEditor 3.5

comment:5 Changed 4 years ago by fredck

  • Milestone changed from CKEditor 3.4.1 to CKEditor 3.5

comment:6 Changed 4 years ago by Saare

  • Owner set to Saare
  • Status changed from confirmed to assigned

Changed 4 years ago by Saare

comment:7 Changed 4 years ago by Saare

  • Status changed from assigned to review

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

#6264 is a dup.

Changed 4 years ago by krst

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

comment:9 Changed 4 years ago by garry.yao

  • Status changed from review to review_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 4 years ago by Saare

comment:10 Changed 4 years ago by Saare

  • Status changed from review_failed to review

comment:11 Changed 4 years ago by garry.yao

  • Status changed from review to review_passed

comment:12 Changed 4 years ago by Saare

  • Status changed from review_passed to closed
  • Resolution set to fixed

Fixed with [5930].

comment:13 Changed 4 years ago by Saare

  • Status changed from closed to reopened
  • Resolution fixed deleted

The last patch brought some regressions for IE7 and Quirks.

Changed 4 years ago by Saare

comment:14 Changed 4 years ago by Saare

  • Status changed from reopened to review

comment:15 Changed 4 years ago by garry.yao

  • Status changed from review to review_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 4 years ago by fredck

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

  • Status changed from review_failed to new
  • Owner Saare deleted

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

  • Status changed from new to confirmed

Changed 4 years ago by garry.yao

comment:19 Changed 4 years ago by garry.yao

  • Owner set to garry.yao
  • Status changed from confirmed to review

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 4 years ago by garry.yao

  • Type changed from Task to Bug
  • Milestone changed from CKEditor 3.5 to CKEditor 3.4.3

comment:21 Changed 4 years ago by Saare

  • Status changed from review to review_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 4 years ago by garry.yao

comment:22 Changed 4 years ago by garry.yao

  • Status changed from review_failed to review

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

Changed 4 years ago by tobiasz.cudnik

IE7 strict

Changed 4 years ago by tobiasz.cudnik

comment:23 Changed 4 years ago by tobiasz.cudnik

  • Status changed from review to review_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 4 years ago by garry.yao

comment:24 Changed 4 years ago by garry.yao

  • Status changed from review_failed to review

Updates in new patch:

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

comment:25 follow-up: ↓ 27 Changed 4 years ago by fredck

  • Status changed from review to review_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 4 years ago by fredck

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 4 years ago by garry.yao

  • Status changed from review_failed to review

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 4 years ago by garry.yao

comment:28 Changed 4 years ago by tobiasz.cudnik

  • Status changed from review to review_passed

comment:29 Changed 4 years ago by garry.yao

  • Status changed from review_passed to closed
  • Resolution set to fixed
  • Milestone changed from CKEditor 3.4.3 to CKEditor 3.5

Fixed with [6137].

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