t/105: Moved rect utilities from BalloonPanelView #106
Conversation
…tersection for provided element/target/limiter/viewport rects.
…nstead of boundingClientRect–like.
|
R-. |
…ion of rects which has negative width and height (disjoint rects).
|
Firefox doesn't have |
|
Actually, we should have |
| /** | ||
| * Element that is to be positioned. | ||
| * | ||
| * @member {HTMLElement} module:utils/dom/position~Options#element |
There was a problem hiding this comment.
I don't think you need to specify the whole path. Just the #element should be fine (but please do check).
| * | ||
| * @property {Number} top Top position offset. | ||
| * @property {Number} left Left position offset. | ||
| * @property {String} name Name of the position. |
There was a problem hiding this comment.
Positions are named. This is useful because as the getOptimalPosition util selects the best position, some changes to the DOM may be needed, i.e. the position needs a proper class to be displayed (like an arrow in BalloonPanelView which changes with each position).
So, in fact, the knowledge about position geometry (top, left) is not enough. We need to know, which of the positioning functions passed to https://github.com/ckeditor/ckeditor5-ui-default/blob/t/131/src/balloonpanel/balloonpanelview.js#L145-L150 has been chosen.
That's why positioning functions are named, like https://github.com/ckeditor/ckeditor5-ui-default/blob/t/131/src/balloonpanel/balloonpanelview.js#L207-L211.
TBH, I don't like it either but I couldn't find any other way to simplify this API. I mean, to pass a number of functions, get the output data out of one of them (the best one) and to know precisely which function returned this output data. Ideas?
There was a problem hiding this comment.
I know that, I didn't mean that it's wrong. I think we can live with it (TBH, I don't have energy to try to find a better solution because this one isn't that bad :D). I just wanted this to be better documented. A single example in this module would do enough.
There was a problem hiding this comment.
Actually, the only place where position names look odd is https://github.com/ckeditor/ckeditor5-ui-default/blob/t/131/src/balloonpanel/balloonpanelview.js#L207. But it's reasonable there too. You can both, access the returned value satisfy the interface.
There was a problem hiding this comment.
Actually 2, you could mention there that these functions returns objects implementing this specific interface, just to make it clear.
| * TODO: more docs and example. | ||
| * | ||
| * @param {module:utils/dom/position~Options} options Positioning options object. | ||
| * @returns {Object} An object containing CSS `top`, `left` coordinates ready to use with `position: absolute` and the `name` |
| /** | ||
| * Creates an instance of rect. | ||
| * | ||
| * // Rect of an HTMLElement. |
There was a problem hiding this comment.
Something broke here with indentation.
|
Great job. I really like the new utils! |
Closes ckeditor/ckeditor5#4923.