Opened 7 years ago

Closed 7 years ago

#9223 closed Bug (fixed)

Float panel doesn't scroll with page for inline

Reported by: Piotrek Koszuliński Owned by: Piotrek Koszuliński
Priority: Normal Milestone: CKEditor 4.0
Component: General Version: 4.0
Keywords: Cc:

Description

Context menu and rich combo don't scroll with page once opened.

Change History (7)

comment:1 Changed 7 years ago by Piotrek Koszuliński

First of all - it turned out that we cannot hide panels on focus, because then reaching the end of panel which has scrollable iframe inside usually ends with disappearing panel. Not the most usable thing in the world :D

Detailed issue description from my chat with Garry:

It's like this: you've got opened combo, which has its own scrollbar

So you place mouse cursor over this combo and scroll

It's scrolled up to its end (scroll events are fired inside combo's iframe)

but when you reach the end of the combo's iframe events are fired on the host page and it starts to scroll

Which we cannot prevent

So if we want to hide combo when scroll was caught on the host page, then just after reaching end of combo's iframe you'll hide it

I tested this and it doesn't work right

Of course we may try to ignore first scroll events after reaching iframe's end, but this doesnt' look like a right solution. Hacky.

So we've got to keep constant floatpanel to floatingspace position. There are two ways:

  • Update floatpanel position when floatingspace position was changed. I've pushed t/9223a with a prototype of this solution. It's brute and even polished it IMO will be.
  • Append floatpanel to the floatingspace. Now they both are in body element, so it isn't possible to easily position one relatively to another. I've also made quick check that this is possible and it works even better than 1. So my question is - are there any other contraindications against this solution? Is there any reason why floatpanel is appended outside floatingspace?

BTW. Context menus positioning works correctly, so this exception has to be somehow included in both solutions.

comment:2 Changed 7 years ago by Piotrek Koszuliński

Call for a concept review for an approach, as simple as the following code show:

// Applies for rich combo and panel button:
if ( editor.elementMode == CKEDITOR.ELEMENT_MODE_INLINE )
	this._.panelDefinition.parent = editor.ui.space( 'chrome' );

For context menu instead, looks like it doesn't need any fix, except for overlapping the floatspace in some case.

comment:3 Changed 7 years ago by Piotrek Koszuliński

Owner: set to Piotrek Koszuliński
Status: newassigned

comment:4 Changed 7 years ago by Piotrek Koszuliński

Status: assignedreview

Created branch t/9223.

Richcombos and panelmenus are now positioned relatively to the toolbar (the new UI's 'chrome' element - second commit in the branch may not be a safe change) when floatspace is used. I also fixed context menu overlapping the floatspace.

Last edited 7 years ago by Piotrek Koszuliński (previous) (diff)

comment:5 Changed 7 years ago by Garry Yao

Status: reviewreview_passed

The patch works pretty well, I've checked thoroughly but finding no code broken by the chrome space change, so R+

comment:6 Changed 7 years ago by Piotrek Koszuliński

I've pushed t/9223 with revert main element's id change. We don't need it and it wasn't backward compatible.

comment:7 Changed 7 years ago by Piotrek Koszuliński

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