ckeditor5 icon indicating copy to clipboard operation
ckeditor5 copied to clipboard

Balloon panel sticks out of the limiter element while scrolling

Open oskarwrobel opened this issue 7 years ago • 18 comments

Follow-up of: ckeditor/ckeditor5#5320

mar-31-2017 12-37-33

oskarwrobel avatar Mar 31 '17 10:03 oskarwrobel

And what is the limiter here?

oleq avatar Mar 31 '17 11:03 oleq

Editable element.

oskarwrobel avatar Mar 31 '17 11:03 oskarwrobel

https://github.com/ckeditor/ckeditor5-ui/issues/173 should resolve it. Precisely:

Additionally getOptimalPosition() could check all the ancestors of the limiter which have overflow different than visible and intersect all their rects one by one up to window to find the real visible area which is available to position the BalloonPanel. That would, in most cases, make the limiter configuration obsolete.

oleq avatar Mar 31 '17 11:03 oleq

Problem is not with configuration. The question is what should we do with panel element when target element is out of the visible part of the limiter?

oskarwrobel avatar Mar 31 '17 11:03 oskarwrobel

Should we hide it or keep inside limiter bounds?

oskarwrobel avatar Mar 31 '17 11:03 oskarwrobel

That's exactly what I quoted ;-)

getOptimalPosition() will essentially traverse up to the root of the document to check if the limiter isn't restricted by some overflow: * and if so, consider that fact. The result Rect of the limiter will then be an intersection of limiter's Rect and "overflow container's" Rect. That's it.

oleq avatar Mar 31 '17 11:03 oleq

keep inside limiter bounds

+1

fredck avatar Apr 07 '17 13:04 fredck

For the record, this issue is actually more complex than https://github.com/ckeditor/ckeditor5-utils/issues/148 and not covered by the fix.

It is complex because:

  1. As #editable is the limiter, there's no way to move the body collection containing the panel inside of it, so the panel gets cropped by the overflow of the (common) parent.

  2. Because of 1., the only solution is analyzing the geometry of the panel/limiter and taking necessary actions.

  3. It's a nasty business, though: suddenly attachTo() and pin() become responsible for the visibility of the panel, which is controlled by #isVisible attribute.

    1. #isVisible is the interface of the balloon. Features use it to control it. So usually it's bound to some external attribute like isFocused of focusManager (like the contextual toolbar) or something else.

    2. When attachTo() and pin() start touching #isVisible things get complicated. The balloon becomes hidden when it reaches the edge of the limiter.

      But can we show it back again if the geometry allows?

      It's not so simple, because after #isVisible switched to false thanks to attachTo() and pin(), tons of things could happen in the editor. Link/Image/Some toolbar feature may not want to display the balloon any longer because the focus was lost or the selection changed. All those things while the balloon hidden because "off the limiter". The features may actually want it to remain hidden, but there's no way to tell – #isVisible remained false and didn't "record" the demands of the features.

    3. To deal with this issue, there could be two different attributes. #isVisible remaining an interface for the features and, let's call it #_withinLimiter, for the attachTo() and pin() logic. Because bind.if in the Template does not support complex bindings, a simple

      bind.if( 'isVisible', 'ck-balloon-panel_visible' ),
      

      becomes

      	this.bind( 'hasVisibleClass' ).to( 
      		this, 'isVisible', 
      		this, '_withinLimiter', 
      		( isVisible, _withinLimiter ) => {
      			return isVisible && _withinLimiter;
      		} );
      
      	...
      
      	bind.if( 'hasVisibleClass', 'ck-balloon-panel_visible' ),
      

      so now the features control #isVisible and the internal logic of the balloon controls #_withinLimiter and the presence of the 'ck-balloon-panel_visible' is controlled by both. It could become part of bind.if API in the future to get rid of that intermediate #hasVisibleClass attribute. Looks good? Yes, but...

    4. The utils and algorithms behind attachTo() and pin() depend on DOM window#getBoundingClientRect method. Long story short, it doesn't work if the element is hidden. So once the balloon gets hidden when it's off the limiter, there's no easy way to display it again, even if it fits, because there's no way to get the geometry of the hidden element – it's controlled by #hasVisibleClass.

      1. To deal with it, we'd need to show the panel with opacity: 0, get its rect and hide it quickly. And again, and again, and again, until it's in the limiter's rect and it's OK to show it permanently to the user. It means lots (hundreds...) of CSS style changes, which is very, very slow.
        • We can optimize the whole thing a little bit by caching panel's dimensions before hiding assuming they won't change. I think it's a good assumption and will work for most of the cases.
        • Throttle/debounce become mandatory.
      2. Alternatively, instead of hiding, we can position the balloon with top: -10000px, left: -10000px so it remains invisible to the user but still it can be analyzed by window#getBoundingClientRect, which means no performance loss. Such panel could steal the focus in some cases, which could totally confuse the user – needs to be checked.
  4. ...and there's still the matter of the scrollbar. window#getBoundingClientRect obtains the rect with the scrollbars, the outermost area of the element. So to avoid situations like this

    image

    we must make the whole system scrollbar–aware.

    1. Again, it's not that simple. Scrollbar width/height can be computed using window#getBoundingClientRect and clientWidth|Height. It's a matter of correcting the rect (width, height, right, bottom) with the difference of #width - #clientWidth.
    2. But now comes the RTL. In RTL, we must correct it to the left (width, height, left, bottom) and to learn what direction is used in the webpage, I'm (almost) sure the window#getComputedStyles is necessary, which means another loss of the performance.
      • Caching the direction by the utility could help. It will fail for the mixed direction content, though. The web page could be in RTL with scrollbars on the left side, but some content us LTR and scrollbars are on the right side (didn't check that). OTOH, it's clearly an edge case.

oleq avatar Apr 18 '17 15:04 oleq

As #editable is the limiter, there's no way to move the body collection containing the panel inside of it, so the panel gets cropped by the overflow of the (common) parent.

For the future reference – I tried moving the toolbar/balloon to the element which has overflow:hidden and fixed height. It doesn't solve the problem automatically because the balloon is positioned absolutely, which negates the overflow:hidden of its parent.

image

We'd need to rewrite positioning of the balloon using relative of fixed positions (uuueeee...) AFAIR.

Reinmar avatar May 19 '17 13:05 Reinmar

Besides, there's one more important problem – the scrolling is captured and blocked by the balloon which makes for a terrible UX. I don't think that it's easy to workaround.

Reinmar avatar May 19 '17 13:05 Reinmar

Besides, there's one more important problem – the scrolling is captured and blocked by the balloon which makes for a terrible UX. I don't think that it's easy to workaround.

Some examples? Because I don't quite get what you had in mind.

oleq avatar May 25 '17 06:05 oleq

I showed it to you live ;) If you keep the mouse over the balloon the page won't scroll. This was, actually, quite surprising because I didn't know that you could capture scroll (we have similar issues with dropdowns, but we'd like there to capture the scroll).

Reinmar avatar May 25 '17 07:05 Reinmar

The issue came back to us in the document editor, which implements a scrollable editable by default. It's time we did something about that.

image

oleq avatar Mar 12 '18 13:03 oleq

The issue also appears when h–scrolling some wide content

image

oleq avatar Oct 16 '19 11:10 oleq

I still got this bug. How about the solution that we can hide ck-ballon-panel when scroll or add a backdrop to disable all page elements (restore when clicking outside)?

Screen Shot 2020-02-17 at 10 02 13 PM

dungkvy avatar Feb 17 '20 15:02 dungkvy

jswiderski avatar Nov 23 '21 09:11 jswiderski

👍 We're seeing this issue at Zendesk too for image resize toolbar

albertfdp avatar Jul 26 '22 13:07 albertfdp

Hey @albertfdp, thank you for your comment. Would you get in touch with us at [email protected]? We might have more questions for you regarding this issue. Hope to hear back from you soon 👋

lslowikowska avatar Sep 21 '22 08:09 lslowikowska