Skip to content

Commit

Permalink
fix: Fix copy instance if no text is selected (#4858)
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
kof and istarkov authored Feb 12, 2025
1 parent 9e710f4 commit 4855ffd
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 20 deletions.
32 changes: 28 additions & 4 deletions apps/builder/app/builder/features/navigator/navigator-tree.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,12 @@ const canDrag = (instance: Instance, instanceSelector: InstanceSelector) => {
if (instanceSelector.length === 1) {
return false;
}

// Do not drag if the instance name is being edited
if ($editingItemSelector.get()?.join(",") === instanceSelector.join(",")) {
return false;
}

if ($isContentMode.get()) {
const parentId = instanceSelector[1];
const parentInstance = $instances.get().get(parentId);
Expand Down Expand Up @@ -542,6 +548,20 @@ export const NavigatorTree = () => {
}
}, [selectedInstanceSelector]);

const selectInstanceAndClearSelection = (
instanceSelector: undefined | Instance["id"][],
event: React.MouseEvent | React.FocusEvent
) => {
if (event.currentTarget.querySelector("[contenteditable]") === null) {
// Allow text selection and edits inside current TreeNode
// Outside if text is selected, it needs to be unselected before selecting the instance.
// Otherwise user will cmd+c the text instead of copying the instance.
window.getSelection()?.removeAllRanges();
}

selectInstance(instanceSelector);
};

return (
<ScrollArea
direction="both"
Expand All @@ -558,8 +578,10 @@ export const NavigatorTree = () => {
level={0}
isSelected={selectedKey === ROOT_INSTANCE_ID}
buttonProps={{
onClick: () => selectInstance([ROOT_INSTANCE_ID]),
onFocus: () => selectInstance([ROOT_INSTANCE_ID]),
onClick: (event) =>
selectInstanceAndClearSelection([ROOT_INSTANCE_ID], event),
onFocus: (event) =>
selectInstanceAndClearSelection([ROOT_INSTANCE_ID], event),
}}
action={
<Tooltip
Expand Down Expand Up @@ -664,8 +686,10 @@ export const NavigatorTree = () => {
$blockChildOutline.set(undefined);
},
onMouseLeave: () => $hoveredInstanceSelector.set(undefined),
onClick: () => selectInstance(item.selector),
onFocus: () => selectInstance(item.selector),
onClick: (event) =>
selectInstanceAndClearSelection(item.selector, event),
onFocus: (event) =>
selectInstanceAndClearSelection(item.selector, event),
onKeyDown: (event) => {
if (event.key === "Enter") {
emitCommand("editInstanceText");
Expand Down
19 changes: 3 additions & 16 deletions apps/builder/app/shared/copy-paste/init-copy-paste.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,23 +49,10 @@ const validateClipboardEvent = (event: ClipboardEvent) => {
// Allows native selection of text in the Builder panels, such as CSS Preview.
if (event.type === "copy") {
const isInBuilderContext = window.self === window.top;
const selection = window.getSelection();

if (isInBuilderContext) {
// Note on event.target:
//
// The spec (https://w3c.github.io/clipboard-apis/#to-fire-a-clipboard-event)
// says that if the context is not editable, the target should be the focused node.
//
// But in practice it seems that the target is based
// on where the cursor is, rather than which element has focus.
// For example, if a <button> has focus, the target is the <body> element.
// If some text is selected, the target is a wrapping element of the text.
// (at least in Chrome).

// 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.
if (event.target !== window.document.body) {
return false;
}
if (isInBuilderContext && selection && selection.isCollapsed === false) {
return false;
}
}

Expand Down

0 comments on commit 4855ffd

Please sign in to comment.