fix(scroll-to-selection): use getClientRects when selectionRect.top/height is still 0 in Safari #1446
Conversation
…ht still 0, use getClientRects
…b.com/zGrav/slate into feature/fix-newline-scrolljump-safari # Conflicts: # packages/slate-react/src/utils/scroll-to-selection.js
…b.com/zGrav/slate into feature/fix-newline-scrolljump-safari # Conflicts: # packages/slate-react/src/utils/scroll-to-selection.js
|
Apologies for so many commits about indentation, I finally figured out that it wanted spaces and not tabs (because I sometimes just don't read things...), took me a bit longer than expected 😢 |
zGrav
changed the title
| @@ -80,6 +80,10 @@ function scrollToSelection(selection) { | |||
| } | |||
|
|
|||
| selectionRect = range.getBoundingClientRect() | |||
|
|
|||
| if (selectionRect.top == 0 && selectionRect.height == 0) { | |||
There was a problem hiding this comment.
if (selectionRect.top === 0 && selectionRect.height === 0) {
There was a problem hiding this comment.
as slate apparently uses improper == checks throughout the code base... ignore my suggestion maybe?
| @@ -80,6 +80,10 @@ function scrollToSelection(selection) { | |||
| } | |||
|
|
|||
| selectionRect = range.getBoundingClientRect() | |||
|
|
|||
| if (selectionRect.top == 0 && selectionRect.height == 0) { | |||
| selectionRect = range.getClientRects()[0] | |||
There was a problem hiding this comment.
if (range.getClientRects && range.getClientRects().length) {
selectionRect = range.getClientRects()[0]
}
|
In my own codebase I use this check though, is this alternative suggestion also solve the issue? const selection = window.getSelection();
let range = selection.getRangeAt(0);
if (IS_SAFARI) {
// `getBoundingClientRect` is buggy on Safari when there is no selected text active.
// A workaround is available at https://stackoverflow.com/a/44261188
range = range.cloneRange();
range.setStart(range.startContainer, 0);
} |
|
|
|
@lxcid , I believe I tried the cloneRange and it wasn’t working as intended, my implementation does work for empty text and keeps the original implementation in place when there is text. I did not know about IS_SAFARI thanks! |
|
I see! Good to know of alternative solution! |
|
Thanks @zGrav! |
Tested on Safari 11.0.1 on OS X 10.13.1 with a scrollable container.
Fixes #1445 .