Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make limiter element more configurable #5381

Closed
oskarwrobel opened this issue Jun 26, 2017 · 3 comments · Fixed by ckeditor/ckeditor5-ui#288
Closed

Make limiter element more configurable #5381

oskarwrobel opened this issue Jun 26, 2017 · 3 comments · Fixed by ckeditor/ckeditor5-ui#288
Assignees
Labels
package:ui type:improvement This issue reports a possible enhancement of an existing feature.
Milestone

Comments

@oskarwrobel
Copy link
Contributor

Let's assume that someone has a sticky header over the editable, and doesn't want to show BPV over the header and doesn't want to hide BPV under the header.

image

There is no way to add sticky header height to the limiter top offset.

@fredck
Copy link
Contributor

fredck commented Jun 27, 2017

What if [limiter top-offset] == [skicky header top-offset]? I mean, the header is floating above the limiter?

For sure, having a limiter element is important as it covers many scenarios. Still, we may need to eventually provide configurable padding somehow, in the [ top, right, bottom, left ] format.

@oleq
Copy link
Member

oleq commented Jul 4, 2017

Since https://github.com/ckeditor/ckeditor5-utils/issues/157, you can define target as Functions. ContextualToolbar uses this approach.

All we need is to enable this thing for limiter. It's a 1LOC enhancement (+docs). Then, such function limiter function would be totally configurable and as specific as the implementation requires. For the case described by @oskarwrobel:

limiter: () => {
    const limiterRect = new Rect( limiterElementInDOM );
    const stickyHeaderRect = new Rect( stickyHeaderInDOM );

    // Adjust the limiterRect to consider the stickyHeader. We could even 
    // introduce some sugar like Rect#substract() similar to existing Rect#getIntersection()
    limiterRect.top += stickyHeaderRect.height;
    limiterRect.height -= stickyHeaderRect.height;
    // etc.

    return limtierRect;
}

Still, we may need to eventually provide configurable padding somehow, in the [ top, right, bottom, left ] format.

It would fail to meet more complex scenario requirements very soon. Then we'd be back exactly where we started. With the functional limiter, you can return any sort of Rect, e.g. in this case

  +--------------------------------------------+
  |                                            |
  |           The limiter element              |
  |                                            |
  |                                            |
  |     +--------------------------------+     |
  |     |                                |     |
  |     |     Some "taboo" zone          |     |
  |     |                                |     |
  |     +--------------------------------+     |
  |                                            |
  |                                            |
  |                                            |
  |                                            |
  +--------------------------------------------+

paddings would do no good but it's trivial case for Rect manipulation.

@oskarwrobel
Copy link
Contributor Author

We need to:

  • make possible to use function as limiter
  • make possible to override default limiter for current balloons (Link, ContextualToolbar).

@oleq oleq self-assigned this Aug 11, 2017
oskarwrobel referenced this issue in ckeditor/ckeditor5-ui Aug 18, 2017
Feature: Allowed `BalloonPanelView` position limiter defined as a function. Made `ContextualBalloon` position limiter configurable via `#positionLimiter` property. Closes #260.
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-ui Oct 9, 2019
@mlewand mlewand added this to the iteration 11 milestone Oct 9, 2019
@mlewand mlewand added status:confirmed type:improvement This issue reports a possible enhancement of an existing feature. package:ui labels Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:ui type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants