Skip to content

chore: Add onEnter and onEscape to Input Component #1850

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

Merged
camielvs merged 1 commit into from
Mar 5, 2026
+46 −65
Merged

chore: Add onEnter and onEscape to Input Component #1850

Show file tree
Hide file tree
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
9 changes: 2 additions & 7 deletions src/components/Editor/Context/Tags/TagEditor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,8 @@ export const TagEditor = ({
value={value}
onChange={handleChange}
onBlur={onSave}
onKeyDown={(e) => {
if (e.key === "Enter") {
onSave();
} else if (e.key === "Escape") {
onCancel();
}
}}
onEnter={onSave}
onEscape={onCancel}
placeholder="Enter tag name..."
className="h-7 text-xs w-36"
/>
Expand Down
27 changes: 9 additions & 18 deletions src/components/shared/AnnotationFilterInput/AnnotationFilterInput.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import type { KeyboardEvent } from "react";
import { useState } from "react";

import { Button } from "@/components/ui/button";
Expand Down Expand Up @@ -48,16 +47,10 @@ export function AnnotationFilterInput({
onChange(newFilters);
};

const handleKeyDown = (e: KeyboardEvent<HTMLInputElement>) => {
if (e.key === "Enter" && keyInput.trim()) {
e.preventDefault();
handleAdd();
}
if (e.key === "Escape") {
setIsExpanded(false);
setKeyInput("");
setValueInput("");
}
const handleCancel = () => {
setIsExpanded(false);
setKeyInput("");
setValueInput("");
};

return (
Expand All @@ -77,15 +70,17 @@ export function AnnotationFilterInput({
placeholder="Key"
value={keyInput}
onChange={(e) => setKeyInput(e.target.value)}
onKeyDown={handleKeyDown}
onEnter={handleAdd}
onEscape={handleCancel}
className="w-28 h-8 text-sm"
autoFocus
/>
<Input
placeholder="Value (optional)"
value={valueInput}
onChange={(e) => setValueInput(e.target.value)}
onKeyDown={handleKeyDown}
onEnter={handleAdd}
onEscape={handleCancel}
className="w-36 h-8 text-sm"
/>
<Button
Expand All @@ -99,11 +94,7 @@ export function AnnotationFilterInput({
<Button
variant="ghost"
size="sm"
onClick={() => {
setIsExpanded(false);
setKeyInput("");
setValueInput("");
}}
onClick={handleCancel}
aria-label="Cancel"
>
<Icon name="X" size="xs" />
Expand Down
19 changes: 5 additions & 14 deletions src/components/shared/AnnotationFilterInput/EditableAnnotationBadge.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { KeyboardEvent, MouseEvent } from "react";
import type { MouseEvent } from "react";
import { useEffect, useRef, useState } from "react";

import { Badge } from "@/components/ui/badge";
Expand Down Expand Up @@ -54,17 +54,6 @@ export function EditableAnnotationBadge({
setIsEditing(false);
};

const handleKeyDown = (e: KeyboardEvent<HTMLInputElement>) => {
if (e.key === "Enter") {
e.preventDefault();
handleSave();
}
if (e.key === "Escape") {
e.preventDefault();
handleCancel();
}
};

const handleDoubleClick = (e: MouseEvent<HTMLElement>) => {
e.preventDefault();
setIsEditing(true);
Expand All @@ -81,14 +70,16 @@ export function EditableAnnotationBadge({
ref={keyInputRef}
value={editKey}
onChange={(e) => setEditKey(e.target.value)}
onKeyDown={handleKeyDown}
onEnter={handleSave}
onEscape={handleCancel}
className="h-6 w-20 px-1.5 text-xs"
placeholder="Key"
/>
<Input
value={editValue}
onChange={(e) => setEditValue(e.target.value)}
onKeyDown={handleKeyDown}
onEnter={handleSave}
onEscape={handleCancel}
className="h-6 w-24 px-1.5 text-xs"
placeholder="Value"
/>
Expand Down
19 changes: 7 additions & 12 deletions src/components/shared/Dialogs/InputDialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { type KeyboardEvent, type ReactNode, useEffect, useState } from "react";
import { type ReactNode, useEffect, useState } from "react";

import {
AlertDialog,
Expand Down Expand Up @@ -56,19 +56,13 @@ export function InputDialog({
};

const handleConfirm = () => {
onConfirm?.(value.trim());
};
const trimmedValue = value.trim();

const handleKeyDown = (e: KeyboardEvent) => {
if (e.key === "Escape") {
onCancel?.();
if (trimmedValue.length === 0 || disabled) {
return;
}

if (disabled) return;

if (e.key === "Enter" && value.trim()) {
handleConfirm();
}
onConfirm?.(trimmedValue);
};

useEffect(() => {
Expand Down Expand Up @@ -96,7 +90,8 @@ export function InputDialog({
value={value}
onChange={(e) => handleValueChange(e.target.value)}
placeholder={placeholder}
onKeyDown={handleKeyDown}
onEnter={handleConfirm}
onEscape={onCancel}
autoFocus
className={cn(!!error && "border-destructive")}
/>
Expand Down
15 changes: 2 additions & 13 deletions src/components/shared/Submitters/Oasis/components/SubmitTaskArgumentsDialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,5 @@
import { useMutation } from "@tanstack/react-query";
import {
type ChangeEvent,
type KeyboardEvent,
useEffect,
useState,
} from "react";
import { type ChangeEvent, useEffect, useState } from "react";

import type { TaskSpecOutput } from "@/api/types.gen";
import TooltipButton from "@/components/shared/Buttons/TooltipButton";
Expand Down Expand Up @@ -258,12 +253,6 @@ const CopyFromRunPopover = ({
}
};

const handleKeyDown = (e: KeyboardEvent<HTMLInputElement>) => {
if (e.key === "Enter") {
handleCustomRunIdSubmit();
}
};

return (
<Popover open={popoverOpen} onOpenChange={setPopoverOpen}>
<PopoverTrigger asChild>
Expand All @@ -283,7 +272,7 @@ const CopyFromRunPopover = ({
placeholder="Run ID"
value={customRunId}
onChange={(e) => setCustomRunId(e.target.value)}
onKeyDown={handleKeyDown}
onEnter={handleCustomRunIdSubmit}
disabled={isCopyingFromRun}
className={cn(isError && "border-red-300", "flex-1")}
/>
Expand Down
22 changes: 21 additions & 1 deletion src/components/ui/input.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,15 @@ function Input({
type,
readOnly,
variant = readOnly ? "readOnly" : "default",
onEnter,
onEscape,
onKeyDown,
...props
}: React.ComponentProps<"input"> & VariantProps<typeof inputVariants>) {
}: React.ComponentProps<"input"> &
VariantProps<typeof inputVariants> & {
onEnter?: () => void;
onEscape?: () => void;
Comment on lines +33 to +34
Copy link
Collaborator

@maxy-shpfy maxy-shpfy Feb 26, 2026

Choose a reason for hiding this comment

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

I do like the idea - simplifies a lot. I noticed that in most cases onEnter means "onSave" and "onEscape" means "onCancel". So maybe we should name them same way?

E.g. in InlineTextEditor same named as "onSave" and "onCancel" - we may introduce naming inconsistencies or establish a good convention.

Copy link
Collaborator Author

@camielvs camielvs Mar 3, 2026

Choose a reason for hiding this comment

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

Maybe... I can see the merit in this. onSave and onClose and more descriptive and nicer to work with.

One thing I like about onEnter and onEscape is that its syntactically ambiguous and thus does not assume anything about what the keys are doing. If we ever need to use the enter key or escape key for actions other than the typical save/close then it's clear how to go about achieving this and we wouldn't need to add any additional logic.

My gut feeling for now is to keep as-is but I am not attached. If you feel strongly about switching to onSave and onClose then I am happy to change.

}) {
return (
<input
type={type}
Expand All @@ -36,6 +43,19 @@ function Input({
className,
)}
readOnly={readOnly}
onKeyDown={(e) => {
if (e.key === "Enter" && onEnter) {
e.preventDefault();
e.stopPropagation();
onEnter();
} else if (e.key === "Escape" && onEscape) {
e.preventDefault();
e.stopPropagation();
onEscape();
}

onKeyDown?.(e);
}}
{...props}
/>
);
Expand Down