Skip to content

Commit 22fd84f

Browse files
kofistarkov
authored andcommitted
fix: Fix copy instance if no text is selected (#4858)
Closes #4856 ## Description Current logic broke in Chrome 133 1. Select instance in navigator 2. Copy 3. Paste 4. Should still allow selecting text and copying it ## 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 --------- Co-authored-by: istarkov <[email protected]>
1 parent e314552 commit 22fd84f

File tree

2 files changed

+31
-20
lines changed

2 files changed

+31
-20
lines changed

apps/builder/app/builder/features/navigator/navigator-tree.tsx

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,12 @@ const canDrag = (instance: Instance, instanceSelector: InstanceSelector) => {
442442
if (instanceSelector.length === 1) {
443443
return false;
444444
}
445+
446+
// Do not drag if the instance name is being edited
447+
if ($editingItemSelector.get()?.join(",") === instanceSelector.join(",")) {
448+
return false;
449+
}
450+
445451
if ($isContentMode.get()) {
446452
const parentId = instanceSelector[1];
447453
const parentInstance = $instances.get().get(parentId);
@@ -542,6 +548,20 @@ export const NavigatorTree = () => {
542548
}
543549
}, [selectedInstanceSelector]);
544550

551+
const selectInstanceAndClearSelection = (
552+
instanceSelector: undefined | Instance["id"][],
553+
event: React.MouseEvent | React.FocusEvent
554+
) => {
555+
if (event.currentTarget.querySelector("[contenteditable]") === null) {
556+
// Allow text selection and edits inside current TreeNode
557+
// Outside if text is selected, it needs to be unselected before selecting the instance.
558+
// Otherwise user will cmd+c the text instead of copying the instance.
559+
window.getSelection()?.removeAllRanges();
560+
}
561+
562+
selectInstance(instanceSelector);
563+
};
564+
545565
return (
546566
<ScrollArea
547567
direction="both"
@@ -558,8 +578,10 @@ export const NavigatorTree = () => {
558578
level={0}
559579
isSelected={selectedKey === ROOT_INSTANCE_ID}
560580
buttonProps={{
561-
onClick: () => selectInstance([ROOT_INSTANCE_ID]),
562-
onFocus: () => selectInstance([ROOT_INSTANCE_ID]),
581+
onClick: (event) =>
582+
selectInstanceAndClearSelection([ROOT_INSTANCE_ID], event),
583+
onFocus: (event) =>
584+
selectInstanceAndClearSelection([ROOT_INSTANCE_ID], event),
563585
}}
564586
action={
565587
<Tooltip
@@ -664,8 +686,10 @@ export const NavigatorTree = () => {
664686
$blockChildOutline.set(undefined);
665687
},
666688
onMouseLeave: () => $hoveredInstanceSelector.set(undefined),
667-
onClick: () => selectInstance(item.selector),
668-
onFocus: () => selectInstance(item.selector),
689+
onClick: (event) =>
690+
selectInstanceAndClearSelection(item.selector, event),
691+
onFocus: (event) =>
692+
selectInstanceAndClearSelection(item.selector, event),
669693
onKeyDown: (event) => {
670694
if (event.key === "Enter") {
671695
emitCommand("editInstanceText");

apps/builder/app/shared/copy-paste/init-copy-paste.ts

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -49,23 +49,10 @@ const validateClipboardEvent = (event: ClipboardEvent) => {
4949
// Allows native selection of text in the Builder panels, such as CSS Preview.
5050
if (event.type === "copy") {
5151
const isInBuilderContext = window.self === window.top;
52+
const selection = window.getSelection();
5253

53-
if (isInBuilderContext) {
54-
// Note on event.target:
55-
//
56-
// The spec (https://w3c.github.io/clipboard-apis/#to-fire-a-clipboard-event)
57-
// says that if the context is not editable, the target should be the focused node.
58-
//
59-
// But in practice it seems that the target is based
60-
// on where the cursor is, rather than which element has focus.
61-
// For example, if a <button> has focus, the target is the <body> element.
62-
// If some text is selected, the target is a wrapping element of the text.
63-
// (at least in Chrome).
64-
65-
// We are using the behavior described above: if some text is selected, the target is usually (at least in the cases we need) not the body.
66-
if (event.target !== window.document.body) {
67-
return false;
68-
}
54+
if (isInBuilderContext && selection && selection.isCollapsed === false) {
55+
return false;
6956
}
7057
}
7158

0 commit comments

Comments
 (0)