Skip to content

Commit

Permalink
fix: kebab menu rename jumps to wrong tab (#38346)
Browse files Browse the repository at this point in the history
## Description
Fixes an issue when renaming from context menu in JS objects, cursor
always jumps to the first tab.

Fixes #38207 

## Automation

/ok-to-test tags="@tag.IDE"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/12480124658>
> Commit: 221119c
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=12480124658&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.IDE`
> Spec:
> <hr>Tue, 24 Dec 2024 10:31:34 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [ ] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **New Features**
	- Improved context menu options with enhanced performance and clarity.
- **Bug Fixes**
- Added checks for the availability of the CodeMirror instance to
prevent errors.
- **Refactor**
- Simplified state management by replacing `useState` with `useBoolean`.
	- Optimized context menu option generation using `useMemo`.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
alex-golovanov authored Dec 24, 2024
1 parent ecf9934 commit c0d393a
Showing 1 changed file with 125 additions and 102 deletions.
227 changes: 125 additions & 102 deletions app/client/src/pages/Editor/JSEditor/AppJSEditorContextMenu.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React, { useCallback, useState } from "react";
import React, { useCallback, useMemo } from "react";
import { useBoolean } from "usehooks-ts";
import { useDispatch, useSelector } from "react-redux";
import {
moveJSCollectionRequest,
Expand Down Expand Up @@ -34,6 +35,7 @@ import { useFeatureFlag } from "utils/hooks/useFeatureFlag";
import { FEATURE_FLAG } from "ee/entities/FeatureFlag";
import type { JSCollection } from "entities/JSCollection";
import { setRenameEntity } from "actions/ideActions";
import type CodeMirror from "codemirror";

interface AppJSEditorContextMenuProps {
pageId: string;
Expand All @@ -46,7 +48,11 @@ export function AppJSEditorContextMenu({
jsCollection,
pageId,
}: AppJSEditorContextMenuProps) {
const [confirmDelete, setConfirmDelete] = useState(false);
const {
setFalse: cancelConfirmDelete,
setValue: setConfirmDelete,
value: confirmDelete,
} = useBoolean(false);
const dispatch = useDispatch();
const isFeatureEnabled = useFeatureFlag(FEATURE_FLAG.license_gac_enabled);
const isDeletePermitted = getHasDeleteActionPermission(
Expand All @@ -63,7 +69,7 @@ export function AppJSEditorContextMenu({
setTimeout(() => {
dispatch(setRenameEntity(jsCollection.id));
}, 100);
}, []);
}, [dispatch, jsCollection.id]);

const copyJSCollectionToPage = useCallback(
(actionId: string, actionName: string, pageId: string) => {
Expand Down Expand Up @@ -95,113 +101,130 @@ export function AppJSEditorContextMenu({
dispatch(deleteJSCollection({ id: actionId, name: actionName }));
setConfirmDelete(false);
},
[dispatch],
[dispatch, setConfirmDelete],
);

const confirmDeletion = (value: boolean, event?: Event) => {
event?.preventDefault?.();
setConfirmDelete(value);
};

const menuPages = useSelector(getPageListAsOptions, equal);

const renameOption = {
icon: "input-cursor-move" as IconName,
value: "rename",
onSelect: renameJS,
label: createMessage(CONTEXT_RENAME),
disabled: !isChangePermitted,
};

const copyOption = {
icon: "duplicate" as IconName,
value: "copy",
onSelect: noop,
label: createMessage(CONTEXT_COPY),
children: menuPages.map((page) => {
return {
...page,
onSelect: () =>
copyJSCollectionToPage(jsCollection.id, jsCollection.name, page.id),
};
}),
};

const moveOption = {
icon: "swap-horizontal" as IconName,
value: "move",
onSelect: noop,
label: createMessage(CONTEXT_MOVE),
children:
menuPages.length > 1
? menuPages
.filter((page) => page.id !== pageId) // Remove current page from the list
.map((page) => {
return {
...page,
onSelect: () =>
moveJSCollectionToPage(
jsCollection.id,
jsCollection.name,
page.id,
),
};
})
: [{ value: "No Pages", onSelect: noop, label: "No Pages" }],
};

const prettifyOptions = {
value: "prettify",
icon: "code" as IconName,
subText: prettifyCodeKeyboardShortCut,
onSelect: () => {
/*
PS: Please do not remove ts-ignore from here, TS keeps suggesting that
the object is null, but that is not the case, and we need an
instance of the editor to pass to autoIndentCode function
*/
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
const editor = document.querySelector(".CodeMirror").CodeMirror;

autoIndentCode(editor);
dispatch(updateJSCollectionBody(editor.getValue(), jsCollection.id));
AnalyticsUtil.logEvent("PRETTIFY_CODE_MANUAL_TRIGGER");
},
label: "Prettify code",
};

const deleteOption = {
confirmDelete: confirmDelete,
icon: "delete-bin-line" as IconName,
value: "delete",
onSelect: (event?: Event): void => {
confirmDelete
? deleteJSCollectionFromPage(jsCollection.id, jsCollection.name)
: confirmDeletion(true, event);
},
label: confirmDelete
? createMessage(CONFIRM_CONTEXT_DELETE)
: createMessage(CONTEXT_DELETE),
className: "t--apiFormDeleteBtn error-menuitem",
};

const options: ContextMenuOption[] = [renameOption];

if (isChangePermitted) {
options.push(copyOption);
options.push(moveOption);
options.push(prettifyOptions);
}

if (isDeletePermitted) options.push(deleteOption);
const options = useMemo(() => {
const confirmDeletion = (value: boolean, event?: Event) => {
event?.preventDefault?.();
setConfirmDelete(value);
};

const renameOption = {
icon: "input-cursor-move" as IconName,
value: "rename",
onSelect: renameJS,
label: createMessage(CONTEXT_RENAME),
disabled: !isChangePermitted,
};

const copyOption = {
icon: "duplicate" as IconName,
value: "copy",
onSelect: noop,
label: createMessage(CONTEXT_COPY),
children: menuPages.map((page) => {
return {
...page,
onSelect: () =>
copyJSCollectionToPage(jsCollection.id, jsCollection.name, page.id),
};
}),
};

const moveOption = {
icon: "swap-horizontal" as IconName,
value: "move",
onSelect: noop,
label: createMessage(CONTEXT_MOVE),
children:
menuPages.length > 1
? menuPages
.filter((page) => page.id !== pageId) // Remove current page from the list
.map((page) => {
return {
...page,
onSelect: () =>
moveJSCollectionToPage(
jsCollection.id,
jsCollection.name,
page.id,
),
};
})
: [{ value: "No Pages", onSelect: noop, label: "No Pages" }],
};

const prettifyOptions = {
value: "prettify",
icon: "code" as IconName,
subText: prettifyCodeKeyboardShortCut,
onSelect: () => {
const editorElement = document.querySelector(".CodeMirror");

if (
editorElement &&
"CodeMirror" in editorElement &&
editorElement.CodeMirror
) {
const editor = editorElement.CodeMirror as CodeMirror.Editor;

autoIndentCode(editor);
dispatch(updateJSCollectionBody(editor.getValue(), jsCollection.id));
AnalyticsUtil.logEvent("PRETTIFY_CODE_MANUAL_TRIGGER");
}
},
label: "Prettify code",
};

const deleteOption = {
confirmDelete: confirmDelete,
icon: "delete-bin-line" as IconName,
value: "delete",
onSelect: (event?: Event): void => {
confirmDelete
? deleteJSCollectionFromPage(jsCollection.id, jsCollection.name)
: confirmDeletion(true, event);
},
label: confirmDelete
? createMessage(CONFIRM_CONTEXT_DELETE)
: createMessage(CONTEXT_DELETE),
className: "t--apiFormDeleteBtn error-menuitem",
};

const options: ContextMenuOption[] = [renameOption];

if (isChangePermitted) {
options.push(copyOption);
options.push(moveOption);
options.push(prettifyOptions);
}

if (isDeletePermitted) options.push(deleteOption);

return options;
}, [
confirmDelete,
copyJSCollectionToPage,
deleteJSCollectionFromPage,
dispatch,
isChangePermitted,
isDeletePermitted,
jsCollection.id,
jsCollection.name,
menuPages,
moveJSCollectionToPage,
pageId,
renameJS,
setConfirmDelete,
]);

return (
<JSEditorContextMenu
className="t--more-action-menu"
onMenuClose={() => {
setConfirmDelete(false);
}}
onMenuClose={cancelConfirmDelete}
options={options}
/>
);
Expand Down

0 comments on commit c0d393a

Please sign in to comment.