Opened 14 years ago

Closed 13 years ago

#5084 closed Bug (fixed)

Dialog resize with mouse broken

Reported by: Matti Järvinen Owned by: Sa'ar Zac Elias
Priority: Normal Milestone: CKEditor 3.5
Component: UI : Dialogs Version: 3.0
Keywords: Cc: matti.jarvinen@…, Andrew

Description

It seems that dialog resizing is broken since change set [3276] relating to ticket #3191.

No handles for resizing in theme.js and dialog/plugin.js doesn't add any.

Attachments (9)

5084.patch (16.3 KB) - added by Sa'ar Zac Elias 14 years ago.
5084_2.patch (9.0 KB) - added by Sa'ar Zac Elias 13 years ago.
5084_3.patch (10.7 KB) - added by Sa'ar Zac Elias 13 years ago.
5084_4.patch (12.5 KB) - added by Sa'ar Zac Elias 13 years ago.
5084_5.patch (17.5 KB) - added by Garry Yao 13 years ago.
5084_6.patch (17.9 KB) - added by Sa'ar Zac Elias 13 years ago.
5084_7.patch (17.1 KB) - added by Garry Yao 13 years ago.
5084_8.patch (18.1 KB) - added by Sa'ar Zac Elias 13 years ago.
5084_9.patch (18.6 KB) - added by Sa'ar Zac Elias 13 years ago.

Download all attachments as: .zip

Change History (33)

comment:1 Changed 14 years ago by Matti Järvinen

Cc: matti.jarvinen@… added

comment:2 Changed 14 years ago by Matti Järvinen

Milestone: CKEditor 3.x

comment:3 Changed 14 years ago by Alfonso Martínez de Lizarrondo

Milestone: CKEditor 3.xCKEditor 3.4
Version: SVN (CKEditor)3.0

comment:4 Changed 14 years ago by Alfonso Martínez de Lizarrondo

Cc: Andrew added

#5896 has been marked as dup

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

Keywords: Confirmed added
Owner: set to Sa'ar Zac Elias
Status: newassigned

comment:6 Changed 14 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.4CKEditor 3.5

Changed 14 years ago by Sa'ar Zac Elias

Attachment: 5084.patch added

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

Status: assignedreview

The resizing code was already present, but the markup was removed and the initialization code was commented out.

  • The code block that was removed in the kama skin's js file was irrelevant as these parts of markup are hidden.
  • We need to make sure that the changes in the skin.js files don't break anything.

comment:8 Changed 14 years ago by Tobiasz Cudnik

Keywords: Confirmed removed
Status: reviewreview_failed

There are some serious problems on IE which includes:

  • very thin resizing grip activation area
  • wrong proportion of resizing to the bottom
  • sometimes horizontal resize grip isn't accessible (IE8 quirks)

Some of them are also reproducible in FF 3.6.

comment:9 Changed 14 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.4.1CKEditor 3.5

Additionally, the following code comment is to be considered seriously:

Simplify the resize logic, having just a single resize grip

Changed 13 years ago by Sa'ar Zac Elias

Attachment: 5084_2.patch added

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

Status: review_failedreview

Introducing a new way of resizing the editor, by using a resize grip in the bottom right/left (depends on language direction), like in the editor chrome.

comment:11 Changed 13 years ago by Garry Yao

Status: reviewreview_failed
  1. Auto centralize behavior should not happen if dialog position is already moved by user.
  2. Mouse and grip must be kept synced when doing auto centralizing.

comment:12 Changed 13 years ago by Garry Yao

I thought the resized dialog dimension was cached, but looks like I'm wrong, I'm not sure about the usability of this feature without memorizing it.

Changed 13 years ago by Sa'ar Zac Elias

Attachment: 5084_3.patch added

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

Status: review_failedreview

The new patch targets all points pointed by Garry here and some additional things.

comment:14 Changed 13 years ago by Garry Yao

Status: reviewreview_failed

Some additional points:

  1. Dialog maximum size should fit screen size, just like dialog movement;
  2. Resize an moved dialog should still expand at duo direction (but without center alignment), right now the dimension delta get doubled while dragging;
  3. Moved dialog position is not memorized (let's have it in this ticket);

Changed 13 years ago by Sa'ar Zac Elias

Attachment: 5084_4.patch added

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

Status: review_failedreview

It seems to me that the user would not want the dialog to be moved. I instead adjusted the resizing logic to match this kind of resizing, and also to have better support for RTL. The dialog position will now be memorized.

Changed 13 years ago by Garry Yao

Attachment: 5084_5.patch added

comment:16 Changed 13 years ago by Garry Yao

Status: reviewreview_failed

Continual enhancement is required, the patch fixes the following issue on 5084_4:

  1. dialog center alignment broken in IE6;
  2. cursor not stick strictly to resize grip in all browsers;
  3. dialog height "shrink" when starting the resize.
  4. "Link" dialog NOT center aligned at startup.

Things that the patch doesn't cover:

  1. Dialog shadows broken when dragging in Opera;
  2. none-kama skin maximum size (full screen) seems smaller than desired.

Changed 13 years ago by Sa'ar Zac Elias

Attachment: 5084_6.patch added

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

Status: review_failedreview

#6757 has been opened for the Opera bug.

Changed 13 years ago by Garry Yao

Attachment: 5084_7.patch added

comment:18 Changed 13 years ago by Garry Yao

Status: reviewreview_failed

I'm not intended to take over this ticket again, but 5084_6.patch is acceptable:

The skin margin (dialog shallow) should not be at all considered by the resize handler, as dialog::getSize already have it counted, you're probably following what has been done at the dialog moving logic, but actually I'm even not sure how that's necessary for move handler, but not something to concern now.

 // Calculate the offset between content and chrome size, don't include dialog margins (shadows).
 heightOffset = startSize.height - dialog.parts.contents.getSize( 'height',  ! ( CKEDITOR.env.gecko || CKEDITOR.env.opera || CKEDITOR.env.ie && CKEDITOR.env.quirks ) ) - margin[0] - margin[2];

comment:19 Changed 13 years ago by Garry Yao

Please review my patch from all the above TCs and make adjustments when necessary, hopefully we have 8 as the final patch.

Changed 13 years ago by Sa'ar Zac Elias

Attachment: 5084_8.patch added

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

Status: review_failedreview

The latest patch includes:

  • Various RTL resizing fixes.
  • It was possible to resize the dialog to less than the minHeight and minWidth.
  • Grip position was incorrect using IE6 and Quirks.

comment:21 Changed 13 years ago by Garry Yao

Status: reviewreview_failed

Two bugs:

  1. non-kama skin, rtl, drag dialog to the right hand side shrink the dialog width;
  2. resize to maximum size, close dialog, resize browser window to smaller size, open dialog again, resize handler out of screen.

Changed 13 years ago by Sa'ar Zac Elias

Attachment: 5084_9.patch added

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

Status: review_failedreview

Fixed the first issue, the second issue will be tackled at #6801.

comment:23 Changed 13 years ago by Garry Yao

Status: reviewreview_passed

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

Resolution: fixed
Status: review_passedclosed

Fixed with [6188].

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