Skip to content

add keyboard shortcuts for renaming layers #3641

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Ayush2k02 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

add keyboard shortcuts for renaming layers #3641

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 70 additions & 2 deletions frontend/src/components/panels/Layers.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@
addEventListener("pointermove", draggingPointerMove);
addEventListener("mousedown", draggingMouseDown);
addEventListener("keydown", draggingKeyDown);
addEventListener("keydown", handleLayerPanelKeyDown);

addEventListener("pointermove", clippingHover);
addEventListener("keydown", clippingKeyPress);
Expand All @@ -123,6 +124,7 @@
removeEventListener("pointermove", draggingPointerMove);
removeEventListener("mousedown", draggingMouseDown);
removeEventListener("keydown", draggingKeyDown);
removeEventListener("keydown", handleLayerPanelKeyDown);

removeEventListener("pointermove", clippingHover);
removeEventListener("keydown", clippingKeyPress);
Expand Down Expand Up @@ -490,6 +492,58 @@
}
}

function handleLayerPanelKeyDown(e: KeyboardEvent) {
// Only handle F2 if not currently editing a layer name
if (e.key === "F2" && !isAnyLayerBeingEdited()) {
// Find the first selected layer
const selectedLayer = layers.find((layer) => layer.entry.selected);
if (selectedLayer) {
e.preventDefault();
onEditLayerName(selectedLayer);
}
}
}

function isAnyLayerBeingEdited(): boolean {
return layers.some((layer) => layer.editingName);
}
Comment on lines +507 to +509
Copy link
Member

@Keavon Keavon Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A one-liner function that's only used once is rather unnecessary compared to just calling that logic inline and describing what's going on in the comment above it.


async function navigateToLayer(currentListing: LayerListingInfo, direction: "next" | "previous" | "up" | "down") {
Copy link
Member

@Keavon Keavon Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there's any reason to have four instead of two directions, when each pair are just synonyms of each other. Just name them "Up" and "Down", with the first letter capitalized because these are the JS equivalent of Rust's enum variants which are also PascalCase.

// Save the current layer name
const inputElement = document.activeElement;
if (inputElement instanceof HTMLInputElement) {
const name = inputElement.value || "";
editor.handle.setLayerName(currentListing.entry.id, name);
currentListing.entry.alias = name;
}

// Find current layer index
const currentIndex = layers.findIndex((layer) => layer.entry.id === currentListing.entry.id);
if (currentIndex === -1) return;

// Calculate target index based on direction
let targetIndex: number;
if (direction === "next" || direction === "down") {
targetIndex = currentIndex + 1;
if (targetIndex >= layers.length) return; // Don't wrap around at the end
} else {
// previous or up
targetIndex = currentIndex - 1;
if (targetIndex < 0) return; // Don't wrap around at the beginning
}
Comment on lines +525 to +533
Copy link
Member

@Keavon Keavon Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move targetIndex declaration to a ternary statement and perform both returns, checking both conditions with a logical OR, on the following line. Full if/else blocks are too verbose here when this could all be made into two lines.


const targetListing = layers[targetIndex];
if (!targetListing) return;

// Exit edit mode on current layer
currentListing.editingName = false;
draggable = true;
layers = layers;

// Start edit mode on target layer
await onEditLayerName(targetListing);
}

function fileDragOver(e: DragEvent) {
if (!draggable || !e.dataTransfer || !e.dataTransfer.types.includes("Files")) return;

Expand Down Expand Up @@ -655,8 +709,22 @@
placeholder={listing.entry.reference.data}
disabled={!listing.editingName}
on:blur={() => onEditLayerNameDeselect(listing)}
on:keydown={(e) => e.key === "Escape" && onEditLayerNameDeselect(listing)}
on:keydown={(e) => e.key === "Enter" && onEditLayerNameChange(listing, e)}
on:keydown={(e) => {
if (e.key === "Escape") {
onEditLayerNameDeselect(listing);
} else if (e.key === "Enter") {
onEditLayerNameChange(listing, e);
} else if (e.key === "Tab") {
e.preventDefault();
navigateToLayer(listing, e.shiftKey ? "previous" : "next");
} else if (e.key === "ArrowUp") {
e.preventDefault();
navigateToLayer(listing, "up");
} else if (e.key === "ArrowDown") {
e.preventDefault();
navigateToLayer(listing, "down");
}
}}
on:change={(e) => onEditLayerNameChange(listing, e)}
/>
</LayoutRow>
Expand Down
Loading