Balloon panel sticks out of the limiter element while scrolling

[oskarwrobel]

Follow-up of: ckeditor/ckeditor5#5320

[oleq]

And what is the limiter here?

[oskarwrobel]

Editable element.

[oleq]

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.

[oskarwrobel]

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]

Should we hide it or keep inside limiter bounds?

[oleq]

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.

[fredck]

keep inside limiter bounds

+1

[oleq]

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

   

   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.

[Reinmar]

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.

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

[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.

[oleq]

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.

[Reinmar]

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).

[oleq]

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.

[oleq]

The issue also appears when h–scrolling some wide content

[dungkvy]

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)?

[jswiderski]

[albertfdp]

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

[lslowikowska]

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 👋

[Dumluregn]

Let's start with creating a PoC according to @oleq 's guidelines that will appear below soon.

[oleq]

I think there's an alternative solution to what I proposed in https://github.com/ckeditor/ckeditor5/issues/5328#issuecomment-294888313.

• We could replicate the powered by logo logic to some extent https://github.com/ckeditor/ckeditor5/blob/01ea41e10e296ba2c64afd896f1168ba4e7e2e8c/packages/ckeditor5-ui/src/editorui/poweredby.ts#L321-L327
• The idea is not to hide the element if it becomes "unpositionable" but position it far away from the viewport.
  • 👍 The element retains focus, so we don't have to worry about focus management. The change is also transparent to screen reader users.
  • 👎 In theory, the user could do something by accident because the positioned UI is still there and has focus, so typing in it or pressing enter could execute some actions.
• We could incorporate this into the getBestPosition() helper https://github.com/ckeditor/ckeditor5/blob/01ea41e10e296ba2c64afd896f1168ba4e7e2e8c/packages/ckeditor5-utils/src/dom/position.ts#L152-L155
• The helper would yield a far-off-the-screen position if none of the available ones would guarantee that the positioned element is not cropped.
• This way we spare ourselves the hassle of hiding and showing the positioned element (balloon), which includes focus management and such (see my previous attempt from 2017).
  • Note that if you hide a balloon that has a focused input, the focus must return to the editing root. Otherwise, the editor will lose focus altogether which is unacceptable. Managing this would require a communication bridge between getOptimalPosition() helper and every UI controller (e.g. LinkUI, TableToolbar etc.). I'd like to avoid that because this would require a lot of work.

[mlewand]

During final testing phase turned out there's some new regressions:

• #14667
• #14668

We don't want to merge the improvement with this regression and the fix requires extra effort and time to think this out. Thus we're delaying this fix to the next release.

[mlewand]

Problems that we have today

Screen	Reason	Solutions
	Balloon leaking outside of a browser viewport.	Sticky position should be shown only if target is not entirely visible in editing viewport. Account for browser viewport (or parent viewport?) to be a limiter too
	The balloon overlays the caret.	Sticky position should be shown only if target is not entirely visible in editing viewport. Selection edge caret should be considered a blocker. Make a common rectangle of all selected ranges to be a blocker. :point_up: This could be too generous,think of long content where you use "select all" feature.

Possible solutions

• Sticky position should be shown only if target is not entirely visible in editing viewport.
  • To be determined, what does entirely visible mean?
    • Is it enough for target to be overflow only in one edge? (e.g. only top part is outside of editing viewport)
    • Or must it overflow in both edges (top and bottom).
  • Will it affect any other cases?

[oleq]

@mlewand What we need here is a complete understanding of all use cases that we have to address. These two only prove that we're missing the big picture. There could be more.

There are two options used in getOptimalPosition(): limiter and fitInViewport. They are used by different editor features in different ways to satisfy different user stories. https://github.com/ckeditor/ckeditor5/pull/14630 proved that we don't understand these user stories well and we're shooting in the dark. We focused on a manual test with specific corner cases and we missed the others.

Let's aggregate all user stories somewhere first (Figma?). That will help us understand and rethink the concepts of limiters and fitting in the viewport. My gut tells me they don't mean the same thing when we start making sure balloons get hidden when their target gets out of sight.

[oleq]

Yet another related issue https://github.com/ckeditor/ckeditor5/issues/7388#issuecomment-1663423751