Skip to content
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.

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

chore: Add Rename context menu #37116

Merged
merged 21 commits into from
Oct 31, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -1068,6 +1068,10 @@ const ExternalLinkIcon = importRemixIcon(
async () => import("remixicon-react/ExternalLinkLineIcon"),
);

const InputCursorMoveIcon = importSvg(
async () => import("../__assets__/icons/ads/input-cursor-move.svg"),
);

import PlayIconPNG from "../__assets__/icons/control/play-icon.png";

function PlayIconPNGWrapper() {
Expand Down Expand Up @@ -1363,6 +1367,7 @@ const ICON_LOOKUP = {
"minimize-v3": MinimizeV3Icon,
"maximize-v3": MaximizeV3Icon,
"workflows-mono": WorkflowsMonochromeIcon,
"input-cursor-move": InputCursorMoveIcon,
billing: BillingIcon,
binding: Binding,
book: BookIcon,
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
56 changes: 47 additions & 9 deletions app/client/src/IDE/Components/EditableName/EditableName.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,17 @@ describe("EditableName", () => {
ESC: { key: "Esc", keyCode: 27 },
};

const setup = ({ isEditing = false, isLoading = false }) => {
const setup = ({
forceEdit = false,
isEditing = false,
isLoading = false,
}) => {
// Define the props
const props = {
name,
icon: <TabIcon />,
isEditing,
forceEdit,
onNameSave: mockOnNameSave,
exitEditing: mockOnExitEditing,
isLoading,
Expand Down Expand Up @@ -90,7 +95,7 @@ describe("EditableName", () => {

await userEvent.click(document.body);

expect(onNameSave).toHaveBeenCalledWith(clickOutsideTitle);
expect(onNameSave).not.toHaveBeenCalledWith(clickOutsideTitle);
expect(exitEditing).toHaveBeenCalled();
});

Expand Down Expand Up @@ -145,8 +150,6 @@ describe("EditableName", () => {
target: { value: invalidTitle },
});

fireEvent.keyUp(inputElement, KEY_CONFIG.ENTER);

expect(getByRole("tooltip")).toBeInTheDocument();

expect(getByRole("tooltip").textContent).toEqual(validationError);
Expand All @@ -169,7 +172,6 @@ describe("EditableName", () => {
target: { value: invalidTitle },
});

fireEvent.keyUp(inputElement, KEY_CONFIG.ENTER);
fireEvent.keyUp(inputElement, KEY_CONFIG.ESC);

expect(getByRole("tooltip")).toBeInTheDocument();
Expand All @@ -189,7 +191,6 @@ describe("EditableName", () => {
target: { value: invalidTitle },
});

fireEvent.keyUp(inputElement, KEY_CONFIG.ENTER);
fireEvent.focusOut(inputElement);
expect(getByRole("tooltip").textContent).toEqual("");
expect(exitEditing).toHaveBeenCalled();
Expand All @@ -201,12 +202,49 @@ describe("EditableName", () => {
const input = getByRole("textbox");

fireEvent.change(input, { target: { value: "" } });
fireEvent.keyUp(input, KEY_CONFIG.ENTER);

expect(onNameSave).not.toHaveBeenCalledWith("");
expect(getByRole("tooltip")).toHaveTextContent(
"Please enter a valid name",
);
fireEvent.keyUp(input, KEY_CONFIG.ENTER);

expect(onNameSave).not.toHaveBeenCalledWith("");
});
});

describe("force Edit behaviour", () => {
test("has the input in focus", () => {
const { getByRole } = setup({
isEditing: true,
forceEdit: true,
});
const input = getByRole("textbox");

expect(document.activeElement).toBe(input);
});
test("focus out will refocus input", () => {
const { getByRole } = setup({
isEditing: true,
forceEdit: true,
});
const input = getByRole("textbox");

fireEvent.focusOut(input);

expect(document.activeElement).toBe(input);
});
test("keyboard interaction allows focus out", async () => {
const { getByRole } = setup({
isEditing: true,
forceEdit: true,
});
const input = getByRole("textbox");

expect(input).toBeInTheDocument();

fireEvent.keyUp(input, { key: "U" });
await userEvent.click(document.body);

expect(document.activeElement).toBe(document.body);
});
});
});
80 changes: 57 additions & 23 deletions app/client/src/IDE/Components/EditableName/EditableName.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,20 @@ import { useNameEditor } from "./useNameEditor";

interface EditableTextProps {
name: string;
/** isLoading true will show a spinner **/
isLoading?: boolean;
onNameSave: (name: string) => void;
/** Used in conjunction with exit editing to control
* this component input editable state */
isEditing: boolean;
/** When this is true, the exit out
* of edit mode will be blocked till the
* user has a keyboard interaction **/
needsInteractionBeforeExit?: boolean;
/** Used in conjunction with exit editing to control this component
* input editable state This function will be called when the
* user is trying to exit the editing mode. This can be
* restricted by the needsInteractionBeforeExit prop **/
exitEditing: () => void;
icon: React.ReactNode;
inputTestId?: string;
Expand All @@ -21,41 +32,66 @@ export const EditableName = ({
isEditing,
isLoading = false,
name,
needsInteractionBeforeExit,
onNameSave,
}: EditableTextProps) => {
const previousName = usePrevious(name);
const [editableName, setEditableName] = useState(name);
const [hasInteracted, setHasInteracted] = useState(false);
const [validationError, setValidationError] = useState<string | null>(null);
const inputRef = useRef<HTMLInputElement>(null);

const { normalizeName, validateName } = useNameEditor({
entityName: name,
});

const attemptSave = () => {
const nameError = validateName(editableName);

if (nameError === null) {
exitEditing();
onNameSave(editableName);
}
};

const exitWithoutSaving = () => {
exitEditing();
setEditableName(name);
setValidationError(null);
};

const validate = (name: string) => {
const nameError = validateName(name);

if (nameError === null) {
setValidationError(null);
} else {
setValidationError(nameError);
}
};
hetunandu marked this conversation as resolved.
Show resolved Hide resolved

const handleKeyUp = useEventCallback(
(e: React.KeyboardEvent<HTMLInputElement>) => {
if (e.key === "Enter") {
const nameError = validateName(editableName);
setHasInteracted(true);

if (nameError === null) {
exitEditing();
onNameSave(editableName);
if (e.key === "Enter") {
if (editableName === name) {
exitWithoutSaving();
} else {
setValidationError(nameError);
attemptSave();
}
} else if (e.key === "Escape") {
exitEditing();
setEditableName(name);
setValidationError(null);
} else {
setValidationError(null);
exitWithoutSaving();
}
},
);

const handleTitleChange = useEventCallback(
(e: React.ChangeEvent<HTMLInputElement>) => {
setEditableName(normalizeName(e.target.value));
const value = normalizeName(e.target.value);

setEditableName(value);
validate(value);
},
);

Expand All @@ -67,22 +103,18 @@ export const EditableName = ({
autoFocus: true,
style: { paddingTop: 0, paddingBottom: 0, left: -1, top: -1 },
}),
[handleKeyUp, handleTitleChange],
[handleKeyUp, handleTitleChange, inputTestId],
);

useEventListener(
"focusout",
function handleFocusOut() {
if (isEditing) {
const nameError = validateName(editableName);

exitEditing();

if (nameError === null) {
onNameSave(editableName);
if (!hasInteracted && needsInteractionBeforeExit) {
// Refocus the input to ensure the user interacts with the input before exiting
inputRef.current?.focus();
} else {
setEditableName(name);
setValidationError(null);
exitWithoutSaving();
}
}
},
Expand All @@ -108,7 +140,9 @@ export const EditableName = ({
if (isEditing && input) {
setTimeout(() => {
input.focus();
}, 200);
}, 400);
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Avoid using arbitrary timeouts and improve comment language

Using setTimeout with a fixed delay of 400ms can lead to inconsistent behavior across different devices and environments. Consider finding a more reliable method to ensure the input refocuses when needed. Additionally, please rephrase the comment to maintain a professional tone.

Apply this diff to update the comment:

- // this is a nasty hack to re-focus the input after context retention applies focus to its target
+ // Temporary workaround to re-focus the input after context retention applies focus to its target

Committable suggestion was skipped due to low confidence.

setHasInteracted(false);
}
},
[isEditing],
Expand All @@ -120,9 +154,9 @@ export const EditableName = ({
<Tooltip content={validationError} visible={Boolean(validationError)}>
<Text
inputProps={inputProps}
inputRef={inputRef}
isEditable={isEditing}
kind="body-s"
ref={inputRef}
>
{editableName}
</Text>
Expand Down
27 changes: 27 additions & 0 deletions app/client/src/IDE/Components/EditableName/RenameMenuItem.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import React, { useCallback } from "react";
import { MenuItem } from "@appsmith/ads";
import { useDispatch } from "react-redux";
import { setRenameEntity } from "actions/ideActions";

interface Props {
disabled?: boolean;
entityId: string;
}

export const RenameMenuItem = ({ disabled, entityId }: Props) => {
const dispatch = useDispatch();

const setRename = useCallback(() => {
dispatch(setRenameEntity(entityId));
}, [entityId]);

return (
<MenuItem
disabled={disabled}
onSelect={setRename}
startIcon="input-cursor-move"
>
Rename
</MenuItem>
);
};
2 changes: 2 additions & 0 deletions app/client/src/IDE/Components/EditableName/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
export { EditableName } from "./EditableName";
export { RenameMenuItem } from "./RenameMenuItem";
export { useIsRenaming } from "./useIsRenaming";
40 changes: 40 additions & 0 deletions app/client/src/IDE/Components/EditableName/useIsRenaming.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { useDispatch, useSelector } from "react-redux";
import { getIsRenaming } from "selectors/ideSelectors";
import { useCallback, useEffect, useState } from "react";
import { setRenameEntity } from "actions/ideActions";

export const useIsRenaming = (id: string) => {
const dispatch = useDispatch();
const [isEditing, setIsEditing] = useState(false);
const [forcedEdit, setForcedEdit] = useState(false);

const isEditingViaExternal = useSelector(getIsRenaming(id));

useEffect(
function onExternalEditEvent() {
if (isEditingViaExternal) {
setIsEditing(true);
setForcedEdit(true);
}

return () => {
setIsEditing(false);
setForcedEdit(false);
};
},
[isEditingViaExternal],
);

const enterEditMode = useCallback(() => {
setIsEditing(true);
setForcedEdit(false);
}, []);

const exitEditMode = useCallback(() => {
dispatch(setRenameEntity(""));
setIsEditing(false);
setForcedEdit(false);
}, [id]);
hetunandu marked this conversation as resolved.
Show resolved Hide resolved

return { isEditing, forcedEdit, enterEditMode, exitEditMode };
};
6 changes: 5 additions & 1 deletion app/client/src/IDE/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,11 @@ export { ToolbarSettingsPopover } from "./Components/ToolbarSettingsPopover";
* EditableName is a component that allows the user to edit the name of an entity
* It is used in the IDE for renaming pages, actions, queries, etc.
*/
export { EditableName } from "./Components/EditableName";
export {
EditableName,
RenameMenuItem,
useIsRenaming,
} from "./Components/EditableName";

/* ====================================================
**** Interfaces ****
Expand Down
7 changes: 7 additions & 0 deletions app/client/src/actions/ideActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,10 @@ export const setListViewActiveState = (payload: boolean) => {
payload,
};
};

export const setRenameEntity = (id: string) => {
return {
type: ReduxActionTypes.SET_RENAME_ENTITY,
payload: id,
};
};
1 change: 1 addition & 0 deletions app/client/src/ce/constants/ReduxActionConstants.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,7 @@ const IDEActionTypes = {
CLOSE_QUERY_ACTION_TAB_SUCCESS: "CLOSE_QUERY_ACTION_TAB_SUCCESS",
SET_IS_LIST_VIEW_ACTIVE: "SET_IS_LIST_VIEW_ACTIVE",
OPEN_PLUGIN_ACTION_SETTINGS: "OPEN_PLUGIN_ACTION_SETTINGS",
SET_RENAME_ENTITY: "SET_RENAME_ENTITY",
};

const IDEActionErrorTypes = {
Expand Down
Loading
Loading