Skip to content

Commit

Permalink
fix: Deleting in the code editor causes the instance on the canvas to…
Browse files Browse the repository at this point in the history
… be removed. (#4717)

## Description

Removed logic based on preventDefault

## 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
  • Loading branch information
istarkov authored Jan 5, 2025
1 parent 3de8c87 commit e9fa0cd
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 25 deletions.
4 changes: 3 additions & 1 deletion apps/builder/app/builder/features/topbar/menu/menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,9 @@ export const Menu = () => {
</DropdownMenuItem>
*/}
<DropdownMenuItem onSelect={() => emitCommand("deleteInstance")}>
<DropdownMenuItem
onSelect={() => emitCommand("deleteInstanceBuilder")}
>
Delete
<DropdownMenuItemRightSlot>
<Kbd value={["backspace"]} />
Expand Down
9 changes: 3 additions & 6 deletions apps/builder/app/builder/shared/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ const makeBreakpointCommand = <CommandName extends string>(

export const deleteSelectedInstance = () => {
if ($isPreviewMode.get()) {
builderApi.toast.info("Deleting is not allowed in preview mode.");
return;
}
const textEditingInstanceSelector = $textEditingInstanceSelector.get();
Expand Down Expand Up @@ -375,13 +374,11 @@ export const { emitCommand, subscribeCommands } = createCommandsEmitter({
},
*/

// instances

{
name: "deleteInstance",
name: "deleteInstanceBuilder",
defaultHotkeys: ["backspace", "delete"],
// this disables hotkey for inputs on style panel
// but still work for input on canvas which call event.preventDefault() in keydown handler
// See "deleteInstanceCanvas" for details on why the command is separated for the canvas and builder.
disableHotkeyOutsideApp: true,
disableOnInputLikeControls: true,
handler: deleteSelectedInstance,
},
Expand Down
10 changes: 10 additions & 0 deletions apps/builder/app/canvas/shared/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,21 @@ import {
} from "../features/text-editor/toolbar-connector";
import { selectInstance } from "~/shared/awareness";
import { isDescendantOrSelf, type InstanceSelector } from "~/shared/tree-utils";
import { deleteSelectedInstance } from "~/builder/shared/commands";

export const { emitCommand, subscribeCommands } = createCommandsEmitter({
source: "canvas",
externalCommands: ["clickCanvas"],
commands: [
{
name: "deleteInstanceCanvas",
defaultHotkeys: ["backspace", "delete"],
disableHotkeyOutsideApp: true,
// We are not disabling "Backspace" or "Delete" on the canvas. This is the main reason we have separate functions: deleteInstanceCanvas and deleteInstanceBuilder.
disableOnInputLikeControls: false,
handler: deleteSelectedInstance,
},

{
name: "editInstanceText",
hidden: true,
Expand Down
32 changes: 14 additions & 18 deletions apps/builder/app/shared/commands-emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,26 +148,22 @@ export const createCommandsEmitter = <CommandName extends string>({
) {
continue;
}
const element = event.target as HTMLElement;
const isOnInputLikeControl =
["input", "select", "textarea"].includes(
element.tagName.toLowerCase()
) ||
element.isContentEditable ||
// Detect Radix select, dropdown and co.
element.getAttribute("role") === "option";

const { disableOnInputLikeControls } = commandMeta;

// in some cases hotkey override default behavior
// on form tags and contentEditable
// though still proceed when default behavior is prevented
// this hack makes hotkeys work on canvas instances of input etc.
if (
isOnInputLikeControl &&
disableOnInputLikeControls &&
event.defaultPrevented === false
) {
continue;
if (disableOnInputLikeControls) {
const element = event.target as HTMLElement;
const isOnInputLikeControl =
["input", "select", "textarea"].includes(
element.tagName.toLowerCase()
) ||
element.isContentEditable ||
// Detect Radix select, dropdown and co.
element.getAttribute("role") === "option";

if (isOnInputLikeControl) {
continue;
}
}

emitted = true;
Expand Down

0 comments on commit e9fa0cd

Please sign in to comment.