Skip to content

Commit 36afe0c

Browse files
istarkovistarkov
authored
fix: Strange behaviour on scroll with tooltip content (#4934)
## Description https://discord.com/channels/955905230107738152/1345038669496586310/1345038669496586310 - [x] This fix introduces other bug https://p-0f63c6ab-adfe-4052-b065-d87223ef2b03-dot-fix-scroll-tooltip.development.webstudio.is/ start editing text in content block, up arrow, scroll on button is broken <img width="290" alt="image" src="https://github.com/user-attachments/assets/ed2dff1e-ef01-4844-9123-e2212f9853a6" /> ## Steps for reproduction 1. click button 2. expect xyz ## Code Review - [ ] hi @kof, I need you to do - conceptual review (architecture, feature-correctness) - detailed review (read every line) - test it on preview ## Before requesting a review - [ ] made a self-review - [ ] added inline comments where things may be not obvious (the "why", not "what") ## Before merging - [ ] tested locally and on preview environment (preview dev login: 0000) - [ ] updated [test cases](https://github.com/webstudio-is/webstudio/blob/main/apps/builder/docs/test-cases.md) document - [ ] added tests - [ ] if any new env variables are added, added them to `.env` file
1 parent 8891748 commit 36afe0c

File tree

2 files changed

+166
-144
lines changed

2 files changed

+166
-144
lines changed

‎apps/builder/app/canvas/features/text-editor/text-editor.tsx

+163-141
Original file line numberDiff line numberDiff line change
@@ -184,8 +184,22 @@ const OnChangeOnBlurPlugin = ({
184184
}) => {
185185
const [editor] = useLexicalComposerContext();
186186
const handleChange = useEffectEvent(onChange);
187+
187188
useEffect(
188189
() => () => {
190+
// The issue is related to React’s development mode.
191+
// When we set the initial selection in the Editor, we disable Lexical’s internal
192+
// scrolling using the update operation tag tag: "skip-scroll-into-view".
193+
// The problem is that a read operation forces all pending update operations to commit,
194+
// and for some reason, this forced commit does not respect tags.
195+
// In React’s development mode, useEffect runs twice, which causes scrollIntoView
196+
// to be called during the first read.
197+
// To prevent this, we disconnect the editor from the DOM
198+
// by setting editor._rootElement = null;.
199+
// This makes Lexical assume it’s in headless mode,
200+
// preventing it from executing DOM operations.
201+
editor._rootElement = null;
202+
189203
// Safari and FF support as no blur event is triggered in some cases
190204
editor.read(() => {
191205
handleChange(editor.getEditorState(), "unmount");
@@ -559,192 +573,200 @@ const InitCursorPlugin = () => {
559573
return;
560574
}
561575

562-
editor.update(() => {
563-
const textEditingInstanceSelector = $textEditingInstanceSelector.get();
564-
if (textEditingInstanceSelector === undefined) {
565-
return;
566-
}
576+
editor.update(
577+
() => {
578+
const textEditingInstanceSelector = $textEditingInstanceSelector.get();
579+
if (textEditingInstanceSelector === undefined) {
580+
return;
581+
}
567582

568-
const { reason } = textEditingInstanceSelector;
583+
const { reason } = textEditingInstanceSelector;
569584

570-
if (reason === undefined) {
571-
return;
572-
}
585+
if (reason === undefined) {
586+
return;
587+
}
573588

574-
if (reason === "click") {
575-
const { mouseX, mouseY } = textEditingInstanceSelector;
589+
if (reason === "click") {
590+
const { mouseX, mouseY } = textEditingInstanceSelector;
576591

577-
const eventRange = caretFromPoint(mouseX, mouseY);
592+
const eventRange = caretFromPoint(mouseX, mouseY);
578593

579-
if (eventRange !== null) {
580-
const { offset: domOffset, node: domNode } = eventRange;
581-
const node = $getNearestNodeFromDOMNode(domNode);
594+
if (eventRange !== null) {
595+
const { offset: domOffset, node: domNode } = eventRange;
596+
const node = $getNearestNodeFromDOMNode(domNode);
582597

583-
if (node !== null) {
584-
const selection = $createRangeSelection();
585-
if ($isTextNode(node)) {
586-
selection.anchor.set(node.getKey(), domOffset, "text");
587-
selection.focus.set(node.getKey(), domOffset, "text");
588-
const normalizedSelection =
589-
$normalizeSelection__EXPERIMENTAL(selection);
598+
if (node !== null) {
599+
const selection = $createRangeSelection();
600+
if ($isTextNode(node)) {
601+
selection.anchor.set(node.getKey(), domOffset, "text");
602+
selection.focus.set(node.getKey(), domOffset, "text");
603+
const normalizedSelection =
604+
$normalizeSelection__EXPERIMENTAL(selection);
590605

591-
$setSelection(normalizedSelection);
592-
return;
606+
$setSelection(normalizedSelection);
607+
return;
608+
}
593609
}
594-
}
595610

596-
if (domNode instanceof Element) {
597-
const rect = domNode.getBoundingClientRect();
598-
if (mouseX > rect.right) {
599-
const selection = $getRoot().selectEnd();
600-
$setSelection(selection);
601-
return;
611+
if (domNode instanceof Element) {
612+
const rect = domNode.getBoundingClientRect();
613+
if (mouseX > rect.right) {
614+
const selection = $getRoot().selectEnd();
615+
$setSelection(selection);
616+
return;
617+
}
602618
}
603619
}
604620
}
605-
}
606-
607-
while (reason === "down" || reason === "up") {
608-
const { cursorX } = textEditingInstanceSelector;
609621

610-
const [topRects, bottomRects] = getTopBottomRects(editor);
622+
while (reason === "down" || reason === "up") {
623+
const { cursorX } = textEditingInstanceSelector;
611624

612-
// Smoodge the cursor a little to the left and right to find the nearest text node
613-
const smoodgeOffsets = [1, 2, 4];
614-
const maxOffset = Math.max(...smoodgeOffsets);
625+
const [topRects, bottomRects] = getTopBottomRects(editor);
615626

616-
const rects = reason === "down" ? topRects : bottomRects;
627+
// Smoodge the cursor a little to the left and right to find the nearest text node
628+
const smoodgeOffsets = [1, 2, 4];
629+
const maxOffset = Math.max(...smoodgeOffsets);
617630

618-
rects.sort((a, b) => a.left - b.left);
631+
const rects = reason === "down" ? topRects : bottomRects;
619632

620-
const rectWithText = rects.find(
621-
(rect, index) =>
622-
rect.left - (index === 0 ? maxOffset : 0) <= cursorX &&
623-
cursorX <= rect.right + (index === rects.length - 1 ? maxOffset : 0)
624-
);
633+
rects.sort((a, b) => a.left - b.left);
625634

626-
if (rectWithText === undefined) {
627-
break;
628-
}
635+
const rectWithText = rects.find(
636+
(rect, index) =>
637+
rect.left - (index === 0 ? maxOffset : 0) <= cursorX &&
638+
cursorX <=
639+
rect.right + (index === rects.length - 1 ? maxOffset : 0)
640+
);
629641

630-
const newCursorY = rectWithText.top + rectWithText.height / 2;
642+
if (rectWithText === undefined) {
643+
break;
644+
}
631645

632-
const eventRanges = [caretFromPoint(cursorX, newCursorY)];
633-
for (const offset of smoodgeOffsets) {
634-
eventRanges.push(caretFromPoint(cursorX - offset, newCursorY));
635-
eventRanges.push(caretFromPoint(cursorX + offset, newCursorY));
636-
}
646+
const newCursorY = rectWithText.top + rectWithText.height / 2;
637647

638-
for (const eventRange of eventRanges) {
639-
if (eventRange === null) {
640-
continue;
648+
const eventRanges = [caretFromPoint(cursorX, newCursorY)];
649+
for (const offset of smoodgeOffsets) {
650+
eventRanges.push(caretFromPoint(cursorX - offset, newCursorY));
651+
eventRanges.push(caretFromPoint(cursorX + offset, newCursorY));
641652
}
642653

643-
const { offset: domOffset, node: domNode } = eventRange;
644-
const node = $getNearestNodeFromDOMNode(domNode);
645-
646-
if (node !== null && $isTextNode(node)) {
647-
const selection = $createRangeSelection();
648-
selection.anchor.set(node.getKey(), domOffset, "text");
649-
selection.focus.set(node.getKey(), domOffset, "text");
650-
const normalizedSelection =
651-
$normalizeSelection__EXPERIMENTAL(selection);
652-
$setSelection(normalizedSelection);
654+
for (const eventRange of eventRanges) {
655+
if (eventRange === null) {
656+
continue;
657+
}
653658

654-
return;
655-
}
656-
}
659+
const { offset: domOffset, node: domNode } = eventRange;
660+
const node = $getNearestNodeFromDOMNode(domNode);
657661

658-
break;
659-
}
662+
if (node !== null && $isTextNode(node)) {
663+
const selection = $createRangeSelection();
664+
selection.anchor.set(node.getKey(), domOffset, "text");
665+
selection.focus.set(node.getKey(), domOffset, "text");
666+
const normalizedSelection =
667+
$normalizeSelection__EXPERIMENTAL(selection);
668+
$setSelection(normalizedSelection);
660669

661-
if (
662-
reason === "down" ||
663-
reason === "right" ||
664-
reason === "enter" ||
665-
reason === "click"
666-
) {
667-
const firstNode = $getRoot().getFirstDescendant();
670+
return;
671+
}
672+
}
668673

669-
if (firstNode === null) {
670-
return;
674+
break;
671675
}
672676

673-
if ($isTextNode(firstNode)) {
674-
const selection = $createRangeSelection();
675-
selection.anchor.set(firstNode.getKey(), 0, "text");
676-
selection.focus.set(firstNode.getKey(), 0, "text");
677-
$setSelection(selection);
678-
}
677+
if (
678+
reason === "down" ||
679+
reason === "right" ||
680+
reason === "enter" ||
681+
reason === "click"
682+
) {
683+
const firstNode = $getRoot().getFirstDescendant();
679684

680-
if ($isElementNode(firstNode)) {
681-
// e.g. Box is empty
682-
const selection = $createRangeSelection();
683-
selection.anchor.set(firstNode.getKey(), 0, "element");
684-
selection.focus.set(firstNode.getKey(), 0, "element");
685-
$setSelection(selection);
686-
}
685+
if (firstNode === null) {
686+
return;
687+
}
687688

688-
if ($isLineBreakNode(firstNode)) {
689-
// e.g. Box contains 2+ empty lines
690-
const selection = $createRangeSelection();
691-
$setSelection(selection);
692-
}
689+
if ($isTextNode(firstNode)) {
690+
const selection = $createRangeSelection();
691+
selection.anchor.set(firstNode.getKey(), 0, "text");
692+
selection.focus.set(firstNode.getKey(), 0, "text");
693+
$setSelection(selection);
694+
}
693695

694-
return;
695-
}
696+
if ($isElementNode(firstNode)) {
697+
// e.g. Box is empty
698+
const selection = $createRangeSelection();
699+
selection.anchor.set(firstNode.getKey(), 0, "element");
700+
selection.focus.set(firstNode.getKey(), 0, "element");
701+
$setSelection(selection);
702+
}
696703

697-
if (reason === "up" || reason === "left") {
698-
const selection = $createRangeSelection();
699-
const lastNode = $getRoot().getLastDescendant();
704+
if ($isLineBreakNode(firstNode)) {
705+
// e.g. Box contains 2+ empty lines
706+
const selection = $createRangeSelection();
707+
$setSelection(selection);
708+
}
700709

701-
if (lastNode === null) {
702710
return;
703711
}
704712

705-
if ($isTextNode(lastNode)) {
706-
const contentSize = lastNode.getTextContentSize();
707-
selection.anchor.set(lastNode.getKey(), contentSize, "text");
708-
selection.focus.set(lastNode.getKey(), contentSize, "text");
709-
$setSelection(selection);
710-
}
711-
712-
if ($isElementNode(lastNode)) {
713-
// e.g. Box is empty
713+
if (reason === "up" || reason === "left") {
714714
const selection = $createRangeSelection();
715-
selection.anchor.set(lastNode.getKey(), 0, "element");
716-
selection.focus.set(lastNode.getKey(), 0, "element");
717-
$setSelection(selection);
718-
}
715+
const lastNode = $getRoot().getLastDescendant();
719716

720-
if ($isLineBreakNode(lastNode)) {
721-
// e.g. Box contains 2+ empty lines
722-
const parent = lastNode.getParent();
723-
if ($isElementNode(parent)) {
717+
if (lastNode === null) {
718+
return;
719+
}
720+
721+
if ($isTextNode(lastNode)) {
722+
const contentSize = lastNode.getTextContentSize();
723+
selection.anchor.set(lastNode.getKey(), contentSize, "text");
724+
selection.focus.set(lastNode.getKey(), contentSize, "text");
725+
$setSelection(selection);
726+
}
727+
728+
if ($isElementNode(lastNode)) {
729+
// e.g. Box is empty
724730
const selection = $createRangeSelection();
725-
selection.anchor.set(
726-
parent.getKey(),
727-
parent.getChildrenSize(),
728-
"element"
729-
);
730-
selection.focus.set(
731-
parent.getKey(),
732-
parent.getChildrenSize(),
733-
"element"
734-
);
731+
selection.anchor.set(lastNode.getKey(), 0, "element");
732+
selection.focus.set(lastNode.getKey(), 0, "element");
735733
$setSelection(selection);
736734
}
735+
736+
if ($isLineBreakNode(lastNode)) {
737+
// e.g. Box contains 2+ empty lines
738+
const parent = lastNode.getParent();
739+
if ($isElementNode(parent)) {
740+
const selection = $createRangeSelection();
741+
selection.anchor.set(
742+
parent.getKey(),
743+
parent.getChildrenSize(),
744+
"element"
745+
);
746+
selection.focus.set(
747+
parent.getKey(),
748+
parent.getChildrenSize(),
749+
"element"
750+
);
751+
$setSelection(selection);
752+
}
753+
}
754+
755+
return;
756+
}
757+
if (reason === "new") {
758+
$selectAll();
759+
return;
737760
}
738761

739-
return;
740-
}
741-
if (reason === "new") {
742-
$selectAll();
743-
return;
762+
reason satisfies never;
763+
},
764+
{
765+
// We are controlling scroll ourself in instance-selected.ts see updateScroll.
766+
// Without skipping we are getting side effects of composition in scrollBy, scrollIntoView calls
767+
tag: "skip-scroll-into-view",
744768
}
745-
746-
reason satisfies never;
747-
});
769+
);
748770
}, [editor]);
749771

750772
return null;

‎apps/builder/app/canvas/instance-selected.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -204,12 +204,12 @@ const subscribeSelectedInstance = (
204204

205205
// Having that elements can be changed (i.e. div => address tag change, observe again)
206206
updateObservers();
207-
208-
// update scroll state
209-
updateScroll();
210207
});
211208
};
212209

210+
// update scroll state
211+
updateScroll();
212+
213213
// Lightweight update
214214
const updateOutline: MutationCallback = (mutationRecords) => {
215215
if (hasCollapsedMutationRecord(mutationRecords)) {

0 commit comments

Comments
 (0)