-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Table props dialog displayed offscreen (while table cell dialog works fine) #6181
Comments
Let's give this thing 2h and see if we see a solution. |
This is weird... I've been seeing this for a very long time, but right now I can't reproduce it... 😅 EDIT: Ok, nevermind. Got it. The table has to be near to the bottom of the editor. |
I think the problem is around here https://github.com/ckeditor/ckeditor5-utils/blob/master/src/dom/position.js#L185-L200. When there's both limiter and Give it just a little bit more space below the widget, and suddenly the opposite position is better: I think this logic is just a little naive. I think we should separately check
For instance, we could loop over the positions and sort them according to the biggest It's a 2-D optimization problem. Picture it as the X-axis representing the element/viewport intersection and the Y-axis representing the limiter/viewport intersection. We want to find the (x,y) point in that 2-D space that will create the rectangle of the biggest area [ (0,0), (x,y) ]. |
On the second thought, maybe it's not a plain 2-D optimization after all. We may need to prioritize if fitting in viewport is more important than fitting into the editable (or vice versa). Research... |
Fix: The `getOptimalPosition()` helper should prefer positions that fit inside the viewport even though there are some others that fit better into the limiter. Closes ckeditor/ckeditor5#6181.
Fix: The `getOptimalPosition()` helper should prefer positions that fit inside the viewport even though there are some others that fit better into the limiter. Closes #6181.
If you'd like to see this fixed sooner, add a 👍 reaction to this post.
The text was updated successfully, but these errors were encountered: