From 4c4694f689b03e7bebfef1a59895e05109765318 Mon Sep 17 00:00:00 2001 From: Alex Golovanov Date: Fri, 27 Sep 2024 12:14:46 +0300 Subject: [PATCH 01/22] perf: do not mutate history for the same url --- app/client/src/pages/Editor/IDE/hooks.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/app/client/src/pages/Editor/IDE/hooks.ts b/app/client/src/pages/Editor/IDE/hooks.ts index 6c8e0e7fecf..1e2997c6e5c 100644 --- a/app/client/src/pages/Editor/IDE/hooks.ts +++ b/app/client/src/pages/Editor/IDE/hooks.ts @@ -175,11 +175,13 @@ export const useIDETabClickHandlers = () => { const tabClickHandler = useCallback( (item: EntityItem) => { const navigateToUrl = tabsConfig.itemUrlSelector(item, basePageId); - history.push(navigateToUrl, { - invokedBy: NavigationMethod.EditorTabs, - }); + if (navigateToUrl !== history.location.pathname) { + history.push(navigateToUrl, { + invokedBy: NavigationMethod.EditorTabs, + }); + } }, - [segment, basePageId], + [tabsConfig, basePageId], ); const closeClickHandler = useCallback( From 295a041c0f635583d7c50182aab8c5e481fc3469 Mon Sep 17 00:00:00 2001 From: Alex Golovanov Date: Fri, 27 Sep 2024 12:16:03 +0300 Subject: [PATCH 02/22] chore: add usehooks-ts --- app/client/package.json | 1 + app/client/yarn.lock | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/app/client/package.json b/app/client/package.json index 500d3c2a328..90db7bb6489 100644 --- a/app/client/package.json +++ b/app/client/package.json @@ -237,6 +237,7 @@ "typescript": "^5.5.4", "unescape-js": "^1.1.4", "url-search-params-polyfill": "^8.0.0", + "usehooks-ts": "^3.1.0", "uuid": "^9.0.0", "validate-color": "^2.2.4", "webfontloader": "^1.6.28", diff --git a/app/client/yarn.lock b/app/client/yarn.lock index cbd1c58cd4b..13f67708c7a 100644 --- a/app/client/yarn.lock +++ b/app/client/yarn.lock @@ -234,6 +234,12 @@ __metadata: languageName: unknown linkType: soft +"@appsmith/hooks@workspace:packages/hooks": + version: 0.0.0-use.local + resolution: "@appsmith/hooks@workspace:packages/hooks" + languageName: unknown + linkType: soft + "@appsmith/utils@workspace:^, @appsmith/utils@workspace:packages/utils": version: 0.0.0-use.local resolution: "@appsmith/utils@workspace:packages/utils" @@ -13488,6 +13494,7 @@ __metadata: typescript: ^5.5.4 unescape-js: ^1.1.4 url-search-params-polyfill: ^8.0.0 + usehooks-ts: ^3.1.0 uuid: ^9.0.0 validate-color: ^2.2.4 webfontloader: ^1.6.28 @@ -34881,6 +34888,17 @@ __metadata: languageName: node linkType: hard +"usehooks-ts@npm:^3.1.0": + version: 3.1.0 + resolution: "usehooks-ts@npm:3.1.0" + dependencies: + lodash.debounce: ^4.0.8 + peerDependencies: + react: ^16.8.0 || ^17 || ^18 + checksum: 4f850c0c5ab408afa52fa2ea2c93c488cd7065c82679eb1fb62cba12ca4c57ff62d52375acc6738823421fe6579ce3adcea1e2dc345ce4f549c593d2e51455b3 + languageName: node + linkType: hard + "util-deprecate@npm:^1.0.1, util-deprecate@npm:^1.0.2, util-deprecate@npm:~1.0.1": version: 1.0.2 resolution: "util-deprecate@npm:1.0.2" From 144edb79f7388d3150ef881e810572199e75ca6f Mon Sep 17 00:00:00 2001 From: Alex Golovanov Date: Wed, 2 Oct 2024 22:09:00 +0300 Subject: [PATCH 03/22] perf: correct state styles & fix spacing --- .../packages/design-system/ads/src/Text/Text.styles.tsx | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/app/client/packages/design-system/ads/src/Text/Text.styles.tsx b/app/client/packages/design-system/ads/src/Text/Text.styles.tsx index ec4a7b1f8b9..00be975f470 100644 --- a/app/client/packages/design-system/ads/src/Text/Text.styles.tsx +++ b/app/client/packages/design-system/ads/src/Text/Text.styles.tsx @@ -191,11 +191,8 @@ export const StyledEditableInput = styled.input` border: 1px solid transparent; border-radius: var(--ads-v2-border-radius); outline: none; - padding: 0; margin: 0; position: absolute; - left: -3px; - top: -3px; width: 100%; padding: var(--ads-v2-spaces-1); @@ -205,6 +202,6 @@ export const StyledEditableInput = styled.input` &:focus, &:active { - border-color: var(--ads-v2-colors-control-field-default-border); + border-color: var(--ads-v2-colors-control-field-active-border); } `; From 8f09c4492bc06416af6607d99e48e314f38d0a3d Mon Sep 17 00:00:00 2001 From: Alex Golovanov Date: Wed, 2 Oct 2024 22:10:02 +0300 Subject: [PATCH 04/22] perf: add input ref & remove redundant callback prop --- app/client/packages/design-system/ads/src/Text/Text.tsx | 8 ++------ .../packages/design-system/ads/src/Text/Text.types.ts | 4 ++-- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/app/client/packages/design-system/ads/src/Text/Text.tsx b/app/client/packages/design-system/ads/src/Text/Text.tsx index 493555010f2..63b83fc0af8 100644 --- a/app/client/packages/design-system/ads/src/Text/Text.tsx +++ b/app/client/packages/design-system/ads/src/Text/Text.tsx @@ -14,9 +14,9 @@ function Text({ className, color, inputProps, + inputRef, isEditable, kind, - onChange, renderAs, ...rest }: TextProps) { @@ -35,11 +35,7 @@ function Text({ {...rest} > {isEditable && typeof children === "string" ? ( - + ) : ( children )} diff --git a/app/client/packages/design-system/ads/src/Text/Text.types.ts b/app/client/packages/design-system/ads/src/Text/Text.types.ts index ec105aeaf1d..a2751ecee63 100644 --- a/app/client/packages/design-system/ads/src/Text/Text.types.ts +++ b/app/client/packages/design-system/ads/src/Text/Text.types.ts @@ -36,13 +36,13 @@ export type TextProps = { isStriked?: boolean; /** whether the text is editable or not */ isEditable?: boolean; - /** onChange event for editable text */ - onChange?: (event: React.ChangeEvent) => void; /** input component props while isEditable is true */ inputProps?: Omit< React.InputHTMLAttributes, "value" | "onChange" >; + /** ref for input component */ + inputRef?: React.RefObject; } & React.HTMLAttributes & React.HTMLAttributes & React.HTMLAttributes & From 6924378cf574ef9c71a1ddbcf31316fb3243763d Mon Sep 17 00:00:00 2001 From: Alex Golovanov Date: Wed, 2 Oct 2024 22:10:46 +0300 Subject: [PATCH 05/22] chore: fix typo --- app/client/src/ce/constants/messages.ts | 2 +- app/client/src/pages/Editor/JSEditor/JSObjectNameEditor.tsx | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/client/src/ce/constants/messages.ts b/app/client/src/ce/constants/messages.ts index 8407a185902..6339878a6ad 100644 --- a/app/client/src/ce/constants/messages.ts +++ b/app/client/src/ce/constants/messages.ts @@ -353,7 +353,7 @@ export const ENTITY_EXPLORER_ACTION_NAME_CONFLICT_ERROR = (name: string) => export const ACTION_ID_NOT_FOUND_IN_URL = "No correct API id or Query id found in the url."; -export const JSOBJECT_ID_NOT_FOUND_IN_URL = +export const JS_OBJECT_ID_NOT_FOUND_IN_URL = "No correct JS Object id found in the url."; export const DATASOURCE_CREATE = (dsName: string) => diff --git a/app/client/src/pages/Editor/JSEditor/JSObjectNameEditor.tsx b/app/client/src/pages/Editor/JSEditor/JSObjectNameEditor.tsx index f82860f134b..24d4594594c 100644 --- a/app/client/src/pages/Editor/JSEditor/JSObjectNameEditor.tsx +++ b/app/client/src/pages/Editor/JSEditor/JSObjectNameEditor.tsx @@ -10,7 +10,7 @@ import { } from "ee/selectors/entitiesSelector"; import { ACTION_NAME_PLACEHOLDER, - JSOBJECT_ID_NOT_FOUND_IN_URL, + JS_OBJECT_ID_NOT_FOUND_IN_URL, createMessage, } from "ee/constants/messages"; import EditableText, { @@ -66,7 +66,7 @@ export function JSObjectNameEditor(props: JSObjectNameEditorProps) { From 6f6bc3de2f98015776e5610d3857d898a88c8230 Mon Sep 17 00:00:00 2001 From: Alex Golovanov Date: Wed, 2 Oct 2024 22:50:51 +0300 Subject: [PATCH 06/22] feat: hook for common name editing needs --- app/client/src/utils/hooks/useNameEditor.ts | 72 +++++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 app/client/src/utils/hooks/useNameEditor.ts diff --git a/app/client/src/utils/hooks/useNameEditor.ts b/app/client/src/utils/hooks/useNameEditor.ts new file mode 100644 index 00000000000..830eca2e91f --- /dev/null +++ b/app/client/src/utils/hooks/useNameEditor.ts @@ -0,0 +1,72 @@ +import { useCallback } from "react"; +import { useSelector, useDispatch, shallowEqual } from "react-redux"; +import { isNameValid, removeSpecialChars } from "utils/helpers"; +import type { AppState } from "ee/reducers"; + +import { getUsedActionNames } from "selectors/actionSelectors"; +import { + ACTION_INVALID_NAME_ERROR, + ACTION_NAME_CONFLICT_ERROR, + createMessage, +} from "ee/constants/messages"; +import type { ReduxAction } from "ee/constants/ReduxActionConstants"; + +interface NameSaveActionParams { + name: string; + id: string; +} + +interface UseNameEditorProps { + nameSaveAction: ( + params: NameSaveActionParams, + ) => ReduxAction; + nameErrorMessage?: (name: string) => string; +} + +/** + * Provides a unified way to validate and save entity names. + */ +export function useNameEditor(props: UseNameEditorProps) { + const dispatch = useDispatch(); + + const { nameErrorMessage = ACTION_NAME_CONFLICT_ERROR, nameSaveAction } = + props; + + const isNew = + new URLSearchParams(window.location.search).get("editName") === "true"; + + const usedEntityNames = useSelector( + (state: AppState) => getUsedActionNames(state, ""), + shallowEqual, + ); + + const validateName = useCallback( + (entityName: string) => + (name: string): string | null => { + if (!name || name.trim().length === 0) { + return createMessage(ACTION_INVALID_NAME_ERROR); + } else if (name !== entityName && !isNameValid(name, usedEntityNames)) { + return createMessage(nameErrorMessage, name); + } + + return null; + }, + [nameErrorMessage, usedEntityNames], + ); + + const handleNameSave = useCallback( + (entityName: string, entityId: string) => (name: string) => { + if (name !== entityName && validateName(entityName)(name) === null) { + dispatch(nameSaveAction({ id: entityId, name })); + } + }, + [validateName, dispatch, nameSaveAction], + ); + + return { + isNew, + validateName, + handleNameSave, + normalizeName: removeSpecialChars, + }; +} From 195daa7794b9cf6c4d99eb02fdea6f63c1e88dff Mon Sep 17 00:00:00 2001 From: Alex Golovanov Date: Wed, 2 Oct 2024 22:53:01 +0300 Subject: [PATCH 07/22] feat: adde editable tab name --- .../src/IDE/Components/FileTab/FileTab.tsx | 161 ++++++++++++++++++ .../src/IDE/Components/FileTab/index.ts | 1 + .../src/IDE/Components/FileTab/styles.tsx | 61 +++++++ .../pages/Editor/IDE/EditorTabs/constants.ts | 7 + .../src/pages/Editor/IDE/EditorTabs/index.tsx | 95 +++++++---- app/client/src/selectors/ui.tsx | 8 + 6 files changed, 297 insertions(+), 36 deletions(-) create mode 100644 app/client/src/IDE/Components/FileTab/FileTab.tsx create mode 100644 app/client/src/IDE/Components/FileTab/index.ts create mode 100644 app/client/src/IDE/Components/FileTab/styles.tsx diff --git a/app/client/src/IDE/Components/FileTab/FileTab.tsx b/app/client/src/IDE/Components/FileTab/FileTab.tsx new file mode 100644 index 00000000000..a8eb51212c9 --- /dev/null +++ b/app/client/src/IDE/Components/FileTab/FileTab.tsx @@ -0,0 +1,161 @@ +import React, { useEffect, useMemo, useRef, useState } from "react"; + +import clsx from "classnames"; +import { noop } from "lodash"; + +import { Icon, Spinner, Tooltip } from "@appsmith/ads"; +import { sanitizeString } from "utils/URLUtils"; +import { useBoolean, useEventCallback, useOnClickOutside } from "usehooks-ts"; +import { usePrevious } from "@mantine/hooks"; + +import * as Styled from "./styles"; + +interface FileTabProps { + isActive: boolean; + isLoading?: boolean; + title: string; + onClick: () => void; + onClose: (e: React.MouseEvent) => void; + icon?: React.ReactNode; + editorConfig?: { + /** Triggered on enter or click outside */ + onTitleSave: (name: string) => void; + /** Used to normalize title (remove white spaces etc.) */ + titleTransformer: (name: string) => string; + /** Validates title and returns an error message or null */ + validateTitle: (name: string) => string | null; + }; +} + +export const FileTab = ({ + editorConfig, + icon, + isActive, + isLoading = false, + onClick, + onClose, + title, +}: FileTabProps) => { + const { + setFalse: exitEditMode, + setTrue: enterEditMode, + value: isEditing, + } = useBoolean(false); + + const previousTitle = usePrevious(title); + const [editableTitle, setEditableTitle] = useState(title); + const currentTitle = + isEditing || isLoading || title !== editableTitle ? editableTitle : title; + const [validationError, setValidationError] = useState(null); + const inputRef = useRef(null); + + const attemptToSave = () => { + if (editorConfig) { + const { onTitleSave, validateTitle } = editorConfig; + const nameError = validateTitle(editableTitle); + + if (nameError !== null) { + setValidationError(nameError); + } else { + exitEditMode(); + onTitleSave(editableTitle); + } + } + }; + + const handleKeyUp = useEventCallback( + (e: React.KeyboardEvent) => { + if (e.key === "Enter") { + attemptToSave(); + } else if (e.key === "Escape") { + exitEditMode(); + setEditableTitle(title); + } else { + setValidationError(null); + } + }, + ); + + useOnClickOutside(inputRef, () => { + attemptToSave(); + }); + + const handleTitleChange = useEventCallback( + (e: React.ChangeEvent) => { + setEditableTitle( + editorConfig + ? editorConfig.titleTransformer(e.target.value) + : e.target.value, + ); + }, + ); + + const handleEnterEditMode = useEventCallback(() => { + setEditableTitle(title); + enterEditMode(); + }); + + const handleDoubleClick = editorConfig ? handleEnterEditMode : noop; + + const inputProps = useMemo( + () => ({ + onKeyUp: handleKeyUp, + onChange: handleTitleChange, + autofocus: true, + style: { + padding: "0 var(--ads-v2-spaces-1)", + }, + }), + [handleKeyUp, handleTitleChange], + ); + + useEffect(() => { + if (!isEditing && previousTitle !== title) { + setEditableTitle(title); + } + }, [title, previousTitle, isEditing]); + + // this is a nasty hack to re-focus the input after context retention applies the focus + // it will be addressed soon, likely by a focus retention modification + useEffect(() => { + const input = inputRef.current; + if (isEditing && input) { + setTimeout(() => { + input.focus(); + }, 200); + } + }, [isEditing]); + + return ( + + {icon && !isLoading ? ( + {icon} + ) : null} + {isLoading && } + + + + {currentTitle} + + + + {/* not using button component because of the size not matching design */} + + + ); +}; diff --git a/app/client/src/IDE/Components/FileTab/index.ts b/app/client/src/IDE/Components/FileTab/index.ts new file mode 100644 index 00000000000..ab954236e05 --- /dev/null +++ b/app/client/src/IDE/Components/FileTab/index.ts @@ -0,0 +1 @@ +export { FileTab } from "./FileTab"; diff --git a/app/client/src/IDE/Components/FileTab/styles.tsx b/app/client/src/IDE/Components/FileTab/styles.tsx new file mode 100644 index 00000000000..8f0825ff2dd --- /dev/null +++ b/app/client/src/IDE/Components/FileTab/styles.tsx @@ -0,0 +1,61 @@ +import styled from "styled-components"; + +import { Text as ADSText } from "@appsmith/ads"; + +export const Tab = styled.div` + display: flex; + height: 100%; + position: relative; + font-size: 12px; + color: var(--ads-v2-colors-text-default); + cursor: pointer; + gap: var(--ads-v2-spaces-2); + border-top: 1px solid transparent; + border-top-left-radius: var(--ads-v2-border-radius); + border-top-right-radius: var(--ads-v2-border-radius); + align-items: center; + justify-content: center; + padding: var(--ads-v2-spaces-3); + padding-top: 6px; // to accommodate border and make icons align correctly + border-left: 1px solid transparent; + border-right: 1px solid transparent; + border-top: 2px solid transparent; + + &.active { + background: var(--ads-v2-colors-control-field-default-bg); + border-top-color: var(--ads-v2-color-bg-brand); + border-left-color: var(--ads-v2-color-border-muted); + border-right-color: var(--ads-v2-color-border-muted); + } + + & > .tab-close { + position: relative; + right: -2px; + visibility: hidden; + } + + &:hover > .tab-close { + visibility: visible; + } + + &.active > .tab-close { + visibility: visible; + } +`; + +export const IconContainer = styled.div` + height: 12px; + width: 12px; + display: flex; + align-items: center; + justify-content: center; + flex-shrink: 0; + img { + width: 12px; + } +`; + +export const Text = styled(ADSText)` + min-width: 3ch; + padding: 0 var(--ads-v2-spaces-1); +`; diff --git a/app/client/src/pages/Editor/IDE/EditorTabs/constants.ts b/app/client/src/pages/Editor/IDE/EditorTabs/constants.ts index 69720db0251..9b06326a3fe 100644 --- a/app/client/src/pages/Editor/IDE/EditorTabs/constants.ts +++ b/app/client/src/pages/Editor/IDE/EditorTabs/constants.ts @@ -37,3 +37,10 @@ export const TabSelectors: Record< itemUrlSelector: () => "", }, }; + +export const SCROLL_AREA_OPTIONS = { + overflow: { + x: "scroll", + y: "hidden", + }, +} as const; diff --git a/app/client/src/pages/Editor/IDE/EditorTabs/index.tsx b/app/client/src/pages/Editor/IDE/EditorTabs/index.tsx index 9024d599a40..9dbbc6c5514 100644 --- a/app/client/src/pages/Editor/IDE/EditorTabs/index.tsx +++ b/app/client/src/pages/Editor/IDE/EditorTabs/index.tsx @@ -8,20 +8,31 @@ import { EditorEntityTabState, EditorViewMode, } from "ee/entities/IDE/constants"; -import FileTabs from "./FileTabs"; + import Container from "./Container"; import { useCurrentEditorState, useIDETabClickHandlers } from "../hooks"; -import { TabSelectors } from "./constants"; +import { SCROLL_AREA_OPTIONS, TabSelectors } from "./constants"; import { AddButton } from "./AddButton"; import { Announcement } from "../EditorPane/components/Announcement"; import { useLocation } from "react-router"; import { identifyEntityFromPath } from "navigation/FocusEntity"; import { List } from "./List"; import { ScreenModeToggle } from "./ScreenModeToggle"; -import { AddTab } from "./AddTab"; + +import { FileTab } from "IDE/Components/FileTab"; +import { useEventCallback } from "usehooks-ts"; + +import { saveActionName } from "actions/pluginActionActions"; +import { saveJSObjectName } from "actions/jsActionActions"; +import { useNameEditor } from "utils/hooks/useNameEditor"; + +import { + getJsObjectNameSavingStatuses, + getApiNameSavingStatuses, +} from "selectors/ui"; const EditorTabs = () => { - const [showListView, setShowListView] = useState(false); + const [showListView, setShowListView] = useState(true); const isSideBySideEnabled = useSelector(getIsSideBySideEnabled); const ideViewMode = useSelector(getIDEViewMode); const { segment, segmentMode } = useCurrentEditorState(); @@ -29,9 +40,21 @@ const EditorTabs = () => { const tabsConfig = TabSelectors[segment]; const files = useSelector(tabsConfig.tabsSelector, shallowEqual); + const saveStatus = useSelector( + EditorEntityTab.JS === segment + ? getJsObjectNameSavingStatuses + : getApiNameSavingStatuses, + shallowEqual, + ); + const location = useLocation(); const currentEntity = identifyEntityFromPath(location.pathname); + const { handleNameSave, normalizeName, validateName } = useNameEditor({ + nameSaveAction: + EditorEntityTab.JS === segment ? saveJSObjectName : saveActionName, + }); + // Turn off list view while changing segment, files useEffect(() => { setShowListView(false); @@ -46,9 +69,9 @@ const EditorTabs = () => { // scroll to the active tab useEffect(() => { - const activetab = document.querySelector(".editor-tab.active"); - if (activetab) { - activetab.scrollIntoView({ + const activeTab = document.querySelector(".editor-tab.active"); + if (activeTab) { + activeTab.scrollIntoView({ inline: "nearest", }); } @@ -66,22 +89,24 @@ const EditorTabs = () => { } }, [files]); - if (!isSideBySideEnabled) return null; - if (segment === EditorEntityTab.UI) return null; - - const handleHamburgerClick = () => { + const handleHamburgerClick = useEventCallback(() => { if (files.length === 0 && segmentMode !== EditorEntityTabState.Add) return; setShowListView(!showListView); - }; + }); - const onTabClick = (tab: EntityItem) => { + const handleTabClick = (tab: EntityItem) => { setShowListView(false); tabClickHandler(tab); }; - const newTabClickHandler = () => { - setShowListView(false); - }; + const createTabEditorConfig = (title: string, key: string) => ({ + onTitleSave: handleNameSave(title, key), + validateTitle: validateName(title), + titleTransformer: normalizeName, + }); + + if (!isSideBySideEnabled) return null; + if (segment === EditorEntityTab.UI) return null; return ( <> @@ -98,13 +123,8 @@ const EditorTabs = () => { { gap="spaces-2" height="100%" > - - + {files.map((tab) => ( + handleTabClick(tab)} + onClose={() => closeClickHandler(tab.key)} + title={tab.title} + /> + ))} - {files.length > 0 ? : null} {/* Switch screen mode button */} diff --git a/app/client/src/selectors/ui.tsx b/app/client/src/selectors/ui.tsx index 67df1f559e6..3a6e7501d69 100644 --- a/app/client/src/selectors/ui.tsx +++ b/app/client/src/selectors/ui.tsx @@ -21,6 +21,10 @@ export const getDefaultSelectedWidgetIds = (state: AppState) => { export const getIsSavingForApiName = (state: AppState, id: string) => state.ui.apiName.isSaving[id]; +/** Select saving status for all API names */ +export const getApiNameSavingStatuses = (state: AppState) => + state.ui.apiName.isSaving; + /** * Selector to use id and provide the status of error in an API. */ @@ -33,6 +37,10 @@ export const getErrorForApiName = (state: AppState, id: string) => export const getIsSavingForJSObjectName = (state: AppState, id: string) => state.ui.jsObjectName.isSaving[id]; +/** Select saving status for all JS object names */ +export const getJsObjectNameSavingStatuses = (state: AppState) => + state.ui.jsObjectName.isSaving; + /** * Selector to use id and provide the status of error in a JS Object. */ From 70add1234261e7c48e88ac0d3de124765a222225 Mon Sep 17 00:00:00 2001 From: Alex Golovanov Date: Wed, 2 Oct 2024 23:28:17 +0300 Subject: [PATCH 08/22] chore: remove unused files --- app/client/src/IDE/Components/FileTab.tsx | 99 ------------------- .../Editor/IDE/EditorTabs/FileTabs.test.tsx | 81 --------------- .../pages/Editor/IDE/EditorTabs/FileTabs.tsx | 48 --------- 3 files changed, 228 deletions(-) delete mode 100644 app/client/src/IDE/Components/FileTab.tsx delete mode 100644 app/client/src/pages/Editor/IDE/EditorTabs/FileTabs.test.tsx delete mode 100644 app/client/src/pages/Editor/IDE/EditorTabs/FileTabs.tsx diff --git a/app/client/src/IDE/Components/FileTab.tsx b/app/client/src/IDE/Components/FileTab.tsx deleted file mode 100644 index 9d8912d44b8..00000000000 --- a/app/client/src/IDE/Components/FileTab.tsx +++ /dev/null @@ -1,99 +0,0 @@ -import React from "react"; -import styled from "styled-components"; -import clsx from "classnames"; - -import { Flex, Icon } from "@appsmith/ads"; -import { sanitizeString } from "utils/URLUtils"; - -interface FileTabProps { - isActive: boolean; - title: string; - onClick: () => void; - onClose: (e: React.MouseEvent) => void; - icon?: React.ReactNode; -} - -export const StyledTab = styled(Flex)` - position: relative; - height: 100%; - font-size: 12px; - color: var(--ads-v2-colors-text-default); - cursor: pointer; - gap: var(--ads-v2-spaces-2); - border-top: 1px solid transparent; - border-top-left-radius: var(--ads-v2-border-radius); - border-top-right-radius: var(--ads-v2-border-radius); - align-items: center; - justify-content: center; - padding: var(--ads-v2-spaces-3); - padding-top: 6px; // to accomodate border and make icons align correctly - border-left: 1px solid transparent; - border-right: 1px solid transparent; - border-top: 2px solid transparent; - - &.active { - background: var(--ads-v2-colors-control-field-default-bg); - border-top-color: var(--ads-v2-color-bg-brand); - border-left-color: var(--ads-v2-color-border-muted); - border-right-color: var(--ads-v2-color-border-muted); - } - - & > .tab-close { - position: relative; - right: -2px; - visibility: hidden; - } - - &:hover > .tab-close { - visibility: visible; - } - - &.active > .tab-close { - visibility: visible; - } -`; - -export const TabTextContainer = styled.span` - width: 100%; - text-overflow: ellipsis; - white-space: nowrap; - overflow: hidden; -`; - -export const TabIconContainer = styled.div` - height: 12px; - width: 12px; - display: flex; - align-items: center; - justify-content: center; - flex-shrink: 0; - img { - width: 12px; - } -`; - -export const FileTab = ({ - icon, - isActive, - onClick, - onClose, - title, -}: FileTabProps) => { - return ( - - {icon ? {icon} : null} - {title} - {/* not using button component because of the size not matching design */} - - - ); -}; diff --git a/app/client/src/pages/Editor/IDE/EditorTabs/FileTabs.test.tsx b/app/client/src/pages/Editor/IDE/EditorTabs/FileTabs.test.tsx deleted file mode 100644 index 36e8969b209..00000000000 --- a/app/client/src/pages/Editor/IDE/EditorTabs/FileTabs.test.tsx +++ /dev/null @@ -1,81 +0,0 @@ -import React from "react"; -import { fireEvent, render } from "test/testUtils"; -import FileTabs from "./FileTabs"; -import { EditorState, type EntityItem } from "ee/entities/IDE/constants"; -import { PluginType } from "entities/Action"; -import { FocusEntity } from "navigation/FocusEntity"; -import { sanitizeString } from "utils/URLUtils"; - -describe("FileTabs", () => { - const mockTabs: EntityItem[] = [ - { key: "1", title: "File 1", type: PluginType.JS }, - { key: "2", title: "File 2", type: PluginType.JS }, - { key: "3", title: "File 3", type: PluginType.JS }, - { key: "4", title: "File 4", type: PluginType.JS }, - ]; - - const mockNavigateToTab = jest.fn(); - const mockOnClose = jest.fn(); - const activeEntity = { - entity: FocusEntity.API, - id: "File 1", - appState: EditorState.EDITOR, - params: {}, - }; - - it("renders tabs correctly", () => { - const { getByTestId, getByText } = render( - , - ); - - // Check if each tab is rendered with correct content - mockTabs.forEach((tab) => { - const tabElement = getByTestId(`t--ide-tab-${sanitizeString(tab.title)}`); - expect(tabElement).not.toBeNull(); - - const tabTitleElement = getByText(tab.title); - expect(tabTitleElement).not.toBeNull(); - }); - }); - - it("check tab click", () => { - const { getByTestId } = render( - , - ); - const tabElement = getByTestId( - `t--ide-tab-${sanitizeString(mockTabs[0].title)}`, - ); - fireEvent.click(tabElement); - - expect(mockNavigateToTab).toHaveBeenCalledWith(mockTabs[0]); - }); - - it("check for close click", () => { - const { getByTestId } = render( - , - ); - const tabElement = getByTestId( - `t--ide-tab-${sanitizeString(mockTabs[1].title)}`, - ); - const closeElement = tabElement.querySelector( - "[data-testid='t--tab-close-btn']", - ) as HTMLElement; - fireEvent.click(closeElement); - expect(mockOnClose).toHaveBeenCalledWith(mockTabs[1].key); - }); -}); diff --git a/app/client/src/pages/Editor/IDE/EditorTabs/FileTabs.tsx b/app/client/src/pages/Editor/IDE/EditorTabs/FileTabs.tsx deleted file mode 100644 index 3d46bfba35d..00000000000 --- a/app/client/src/pages/Editor/IDE/EditorTabs/FileTabs.tsx +++ /dev/null @@ -1,48 +0,0 @@ -import React from "react"; - -import { - EditorEntityTabState, - type EntityItem, -} from "ee/entities/IDE/constants"; -import { useCurrentEditorState } from "../hooks"; -import { FileTab } from "IDE/Components/FileTab"; -import type { FocusEntityInfo } from "navigation/FocusEntity"; - -interface Props { - tabs: EntityItem[]; - navigateToTab: (tab: EntityItem) => void; - onClose: (actionId?: string) => void; - currentEntity: FocusEntityInfo; - isListActive?: boolean; -} - -const FileTabs = (props: Props) => { - const { currentEntity, isListActive, navigateToTab, onClose, tabs } = props; - const { segmentMode } = useCurrentEditorState(); - - const onCloseClick = (e: React.MouseEvent, id?: string) => { - e.stopPropagation(); - onClose(id); - }; - - return ( - <> - {tabs.map((tab: EntityItem) => ( - navigateToTab(tab)} - onClose={(e) => onCloseClick(e, tab.key)} - title={tab.title} - /> - ))} - - ); -}; - -export default FileTabs; From b98f363f0a08ad5c273a5f59c50c1e33db2bd9e9 Mon Sep 17 00:00:00 2001 From: Alex Golovanov Date: Wed, 2 Oct 2024 23:57:25 +0300 Subject: [PATCH 09/22] fix: replace lock --- app/client/yarn.lock | 6 ------ 1 file changed, 6 deletions(-) diff --git a/app/client/yarn.lock b/app/client/yarn.lock index 8570aead030..906e5c40879 100644 --- a/app/client/yarn.lock +++ b/app/client/yarn.lock @@ -192,12 +192,6 @@ __metadata: languageName: unknown linkType: soft -"@appsmith/hooks@workspace:packages/hooks": - version: 0.0.0-use.local - resolution: "@appsmith/hooks@workspace:packages/hooks" - languageName: unknown - linkType: soft - "@appsmith/utils@workspace:^, @appsmith/utils@workspace:packages/utils": version: 0.0.0-use.local resolution: "@appsmith/utils@workspace:packages/utils" From f2c69246f28d4a42f2b33a22ae8f6577096470fa Mon Sep 17 00:00:00 2001 From: Alex Golovanov Date: Thu, 3 Oct 2024 01:58:27 +0300 Subject: [PATCH 10/22] perf: tweak positioning --- app/client/packages/design-system/ads/src/Text/Text.styles.tsx | 1 + app/client/src/IDE/Components/FileTab/FileTab.tsx | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/client/packages/design-system/ads/src/Text/Text.styles.tsx b/app/client/packages/design-system/ads/src/Text/Text.styles.tsx index 00be975f470..58318c2cbdb 100644 --- a/app/client/packages/design-system/ads/src/Text/Text.styles.tsx +++ b/app/client/packages/design-system/ads/src/Text/Text.styles.tsx @@ -193,6 +193,7 @@ export const StyledEditableInput = styled.input` outline: none; margin: 0; position: absolute; + top: -3px; width: 100%; padding: var(--ads-v2-spaces-1); diff --git a/app/client/src/IDE/Components/FileTab/FileTab.tsx b/app/client/src/IDE/Components/FileTab/FileTab.tsx index 5a570c5a288..14f8f99d8bc 100644 --- a/app/client/src/IDE/Components/FileTab/FileTab.tsx +++ b/app/client/src/IDE/Components/FileTab/FileTab.tsx @@ -103,7 +103,7 @@ export const FileTab = ({ onChange: handleTitleChange, autofocus: true, style: { - padding: "0 var(--ads-v2-spaces-1)", + left: -1, }, }), [handleKeyUp, handleTitleChange], From aa4c87ebf3c5fb167a305f0f10091fdc2731640b Mon Sep 17 00:00:00 2001 From: Alex Golovanov Date: Thu, 3 Oct 2024 02:12:01 +0300 Subject: [PATCH 11/22] fix: stop event propagation on tab close --- app/client/src/IDE/Components/FileTab/FileTab.tsx | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/app/client/src/IDE/Components/FileTab/FileTab.tsx b/app/client/src/IDE/Components/FileTab/FileTab.tsx index 14f8f99d8bc..711a77f0ef2 100644 --- a/app/client/src/IDE/Components/FileTab/FileTab.tsx +++ b/app/client/src/IDE/Components/FileTab/FileTab.tsx @@ -97,11 +97,16 @@ export const FileTab = ({ const handleDoubleClick = editorConfig ? handleEnterEditMode : noop; + const handleOnClose = useEventCallback((e: React.MouseEvent) => { + e.stopPropagation(); + onClose(e); + }); + const inputProps = useMemo( () => ({ onKeyUp: handleKeyUp, onChange: handleTitleChange, - autofocus: true, + autoFocus: true, style: { left: -1, }, @@ -155,7 +160,7 @@ export const FileTab = ({ className="tab-close rounded-[4px] hover:bg-[var(--ads-v2-colors-action-tertiary-surface-hover-bg)] cursor-pointer p-[2px]" data-testid="t--tab-close-btn" name="close-line" - onClick={onClose} + onClick={handleOnClose} /> ); From d6cc17ab37c44766fce3e4de860d166cda4b0bf9 Mon Sep 17 00:00:00 2001 From: Alex Golovanov Date: Thu, 3 Oct 2024 02:17:12 +0300 Subject: [PATCH 12/22] fix: validation reset on escape --- app/client/src/IDE/Components/FileTab/FileTab.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/app/client/src/IDE/Components/FileTab/FileTab.tsx b/app/client/src/IDE/Components/FileTab/FileTab.tsx index 711a77f0ef2..aa7235b6b44 100644 --- a/app/client/src/IDE/Components/FileTab/FileTab.tsx +++ b/app/client/src/IDE/Components/FileTab/FileTab.tsx @@ -70,6 +70,7 @@ export const FileTab = ({ } else if (e.key === "Escape") { exitEditMode(); setEditableTitle(title); + setValidationError(null); } else { setValidationError(null); } From df31a7a6f6e6888d4219207054b94f60eef12e6f Mon Sep 17 00:00:00 2001 From: Alex Golovanov Date: Thu, 3 Oct 2024 12:18:41 +0300 Subject: [PATCH 13/22] perf: replace tw with styled & tweak styles --- .../src/IDE/Components/FileTab/FileTab.tsx | 19 ++++++++++++------- .../src/IDE/Components/FileTab/styles.tsx | 15 +++++++++++---- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/app/client/src/IDE/Components/FileTab/FileTab.tsx b/app/client/src/IDE/Components/FileTab/FileTab.tsx index aa7235b6b44..448a469196f 100644 --- a/app/client/src/IDE/Components/FileTab/FileTab.tsx +++ b/app/client/src/IDE/Components/FileTab/FileTab.tsx @@ -109,7 +109,10 @@ export const FileTab = ({ onChange: handleTitleChange, autoFocus: true, style: { + paddingTop: 0, + paddingBottom: 0, left: -1, + top: -1, }, }), [handleKeyUp, handleTitleChange], @@ -121,8 +124,9 @@ export const FileTab = ({ } }, [title, previousTitle, isEditing]); - // this is a nasty hack to re-focus the input after context retention applies the focus - // it will be addressed soon, likely by a focus retention modification + // TODO: This is a temporary fix to focus the input after context retention applies focus to its target + // this is a nasty hack to re-focus the input after context retention applies focus to its target + // this will be addressed in a future task, likely by a focus retention modification useEffect(() => { const input = inputRef.current; @@ -156,13 +160,14 @@ export const FileTab = ({ - {/* not using button component because of the size not matching design */} - + > + + ); }; diff --git a/app/client/src/IDE/Components/FileTab/styles.tsx b/app/client/src/IDE/Components/FileTab/styles.tsx index 8f0825ff2dd..7fd681a6ce7 100644 --- a/app/client/src/IDE/Components/FileTab/styles.tsx +++ b/app/client/src/IDE/Components/FileTab/styles.tsx @@ -34,10 +34,7 @@ export const Tab = styled.div` visibility: hidden; } - &:hover > .tab-close { - visibility: visible; - } - + &:hover > .tab-close, &.active > .tab-close { visibility: visible; } @@ -59,3 +56,13 @@ export const Text = styled(ADSText)` min-width: 3ch; padding: 0 var(--ads-v2-spaces-1); `; + +export const CloseButton = styled.button` + border-radius: var(--ads-v2-border-radius); + cursor: pointer; + padding: var(--ads-v2-spaces-1); + + &:hover { + background: var(--ads-v2-colors-action-tertiary-surface-hover-bg); + } +`; From 072f1d6b67d604122645195362eef03c980439e3 Mon Sep 17 00:00:00 2001 From: Alex Golovanov Date: Fri, 4 Oct 2024 19:56:15 +0300 Subject: [PATCH 14/22] perf: lifted state into editable component for perf --- .../src/IDE/Components/FileTab/FileTab.tsx | 9 +-- .../src/IDE/Components/FileTab/index.ts | 1 + .../Editor/IDE/EditorTabs/EditableTab.tsx | 64 +++++++++++++++++++ .../src/pages/Editor/IDE/EditorTabs/index.tsx | 46 +++---------- app/client/src/utils/hooks/useNameEditor.ts | 46 +++++++------ 5 files changed, 99 insertions(+), 67 deletions(-) create mode 100644 app/client/src/pages/Editor/IDE/EditorTabs/EditableTab.tsx diff --git a/app/client/src/IDE/Components/FileTab/FileTab.tsx b/app/client/src/IDE/Components/FileTab/FileTab.tsx index 448a469196f..2f99afe1f21 100644 --- a/app/client/src/IDE/Components/FileTab/FileTab.tsx +++ b/app/client/src/IDE/Components/FileTab/FileTab.tsx @@ -10,7 +10,7 @@ import { usePrevious } from "@mantine/hooks"; import * as Styled from "./styles"; -interface FileTabProps { +export interface FileTabProps { isActive: boolean; isLoading?: boolean; title: string; @@ -98,11 +98,6 @@ export const FileTab = ({ const handleDoubleClick = editorConfig ? handleEnterEditMode : noop; - const handleOnClose = useEventCallback((e: React.MouseEvent) => { - e.stopPropagation(); - onClose(e); - }); - const inputProps = useMemo( () => ({ onKeyUp: handleKeyUp, @@ -164,7 +159,7 @@ export const FileTab = ({ aria-label="Close tab" className="tab-close" data-testid="t--tab-close-btn" - onClick={handleOnClose} + onClick={onClose} > diff --git a/app/client/src/IDE/Components/FileTab/index.ts b/app/client/src/IDE/Components/FileTab/index.ts index ab954236e05..3a8ba9834ec 100644 --- a/app/client/src/IDE/Components/FileTab/index.ts +++ b/app/client/src/IDE/Components/FileTab/index.ts @@ -1 +1,2 @@ export { FileTab } from "./FileTab"; +export type { FileTabProps } from "./FileTab"; diff --git a/app/client/src/pages/Editor/IDE/EditorTabs/EditableTab.tsx b/app/client/src/pages/Editor/IDE/EditorTabs/EditableTab.tsx new file mode 100644 index 00000000000..cc88734770b --- /dev/null +++ b/app/client/src/pages/Editor/IDE/EditorTabs/EditableTab.tsx @@ -0,0 +1,64 @@ +import React, { useMemo } from "react"; + +import { FileTab, type FileTabProps } from "IDE/Components/FileTab"; +import { useNameEditor } from "utils/hooks/useNameEditor"; +import { EditorEntityTab } from "ee/entities/IDE/constants"; +import { saveActionName } from "actions/pluginActionActions"; +import { saveJSObjectName } from "actions/jsActionActions"; +import { useCurrentEditorState } from "../hooks"; + +import { + getIsSavingForApiName, + getIsSavingForJSObjectName, +} from "selectors/ui"; +import { useSelector } from "react-redux"; +import { useEventCallback } from "usehooks-ts"; + +interface EditableTabProps extends Omit { + id: string; + onClose: (id: string) => void; +} + +export function EditableTab(props: EditableTabProps) { + const { icon, id, isActive, onClick, onClose, title } = props; + const { segment } = useCurrentEditorState(); + + const { handleNameSave, normalizeName, validateName } = useNameEditor({ + entityId: id, + entityName: title, + nameSaveAction: + EditorEntityTab.JS === segment ? saveJSObjectName : saveActionName, + }); + + const isLoading = useSelector((state) => + EditorEntityTab.JS === segment + ? getIsSavingForJSObjectName(state, id) + : getIsSavingForApiName(state, id), + ); + + const editorConfig = useMemo( + () => ({ + onTitleSave: handleNameSave, + validateTitle: validateName, + titleTransformer: normalizeName, + }), + [handleNameSave, normalizeName, validateName], + ); + + const handleClose = useEventCallback((e: React.MouseEvent) => { + e.stopPropagation(); + onClose(id); + }); + + return ( + + ); +} diff --git a/app/client/src/pages/Editor/IDE/EditorTabs/index.tsx b/app/client/src/pages/Editor/IDE/EditorTabs/index.tsx index e75704c736a..09a923da070 100644 --- a/app/client/src/pages/Editor/IDE/EditorTabs/index.tsx +++ b/app/client/src/pages/Editor/IDE/EditorTabs/index.tsx @@ -25,17 +25,9 @@ import { ScreenModeToggle } from "./ScreenModeToggle"; import { AddTab } from "./AddTab"; import { setListViewActiveState } from "actions/ideActions"; -import { FileTab } from "IDE/Components/FileTab"; import { useEventCallback } from "usehooks-ts"; -import { saveActionName } from "actions/pluginActionActions"; -import { saveJSObjectName } from "actions/jsActionActions"; -import { useNameEditor } from "utils/hooks/useNameEditor"; - -import { - getJsObjectNameSavingStatuses, - getApiNameSavingStatuses, -} from "selectors/ui"; +import { EditableTab } from "./EditableTab"; const EditorTabs = () => { const isSideBySideEnabled = useSelector(getIsSideBySideEnabled); @@ -46,22 +38,10 @@ const EditorTabs = () => { const files = useSelector(tabsConfig.tabsSelector, shallowEqual); const isListViewActive = useSelector(getListViewActiveState); - const saveStatus = useSelector( - EditorEntityTab.JS === segment - ? getJsObjectNameSavingStatuses - : getApiNameSavingStatuses, - shallowEqual, - ); - const location = useLocation(); const dispatch = useDispatch(); const currentEntity = identifyEntityFromPath(location.pathname); - const { handleNameSave, normalizeName, validateName } = useNameEditor({ - nameSaveAction: - EditorEntityTab.JS === segment ? saveJSObjectName : saveActionName, - }); - // Turn off list view while changing segment, files useEffect(() => { dispatch(setListViewActiveState(false)); @@ -104,19 +84,14 @@ const EditorTabs = () => { dispatch(setListViewActiveState(!isListViewActive)); }); - const handleTabClick = (tab: EntityItem) => { + // TODO: this returns a new function every time, needs to be recomposed + const handleTabClick = useEventCallback((tab: EntityItem) => () => { dispatch(setListViewActiveState(false)); tabClickHandler(tab); - }; - - const newTabClickHandler = useEventCallback(() => { - dispatch(setListViewActiveState(false)); }); - const createTabEditorConfig = (title: string, key: string) => ({ - onTitleSave: handleNameSave(title, key), - validateTitle: validateName(title), - titleTransformer: normalizeName, + const handleNewTabClick = useEventCallback(() => { + dispatch(setListViewActiveState(false)); }); if (!isSideBySideEnabled) return null; @@ -148,24 +123,23 @@ const EditorTabs = () => { height="100%" > {files.map((tab) => ( - handleTabClick(tab)} - onClose={() => closeClickHandler(tab.key)} + onClick={handleTabClick(tab)} + onClose={closeClickHandler} title={tab.title} /> ))} diff --git a/app/client/src/utils/hooks/useNameEditor.ts b/app/client/src/utils/hooks/useNameEditor.ts index 830eca2e91f..4dbc0fe6e7f 100644 --- a/app/client/src/utils/hooks/useNameEditor.ts +++ b/app/client/src/utils/hooks/useNameEditor.ts @@ -1,4 +1,3 @@ -import { useCallback } from "react"; import { useSelector, useDispatch, shallowEqual } from "react-redux"; import { isNameValid, removeSpecialChars } from "utils/helpers"; import type { AppState } from "ee/reducers"; @@ -10,6 +9,7 @@ import { createMessage, } from "ee/constants/messages"; import type { ReduxAction } from "ee/constants/ReduxActionConstants"; +import { useEventCallback } from "usehooks-ts"; interface NameSaveActionParams { name: string; @@ -17,6 +17,8 @@ interface NameSaveActionParams { } interface UseNameEditorProps { + entityId: string; + entityName: string; nameSaveAction: ( params: NameSaveActionParams, ) => ReduxAction; @@ -28,9 +30,12 @@ interface UseNameEditorProps { */ export function useNameEditor(props: UseNameEditorProps) { const dispatch = useDispatch(); - - const { nameErrorMessage = ACTION_NAME_CONFLICT_ERROR, nameSaveAction } = - props; + const { + entityId, + entityName, + nameErrorMessage = ACTION_NAME_CONFLICT_ERROR, + nameSaveAction, + } = props; const isNew = new URLSearchParams(window.location.search).get("editName") === "true"; @@ -40,28 +45,21 @@ export function useNameEditor(props: UseNameEditorProps) { shallowEqual, ); - const validateName = useCallback( - (entityName: string) => - (name: string): string | null => { - if (!name || name.trim().length === 0) { - return createMessage(ACTION_INVALID_NAME_ERROR); - } else if (name !== entityName && !isNameValid(name, usedEntityNames)) { - return createMessage(nameErrorMessage, name); - } + const validateName = useEventCallback((name: string): string | null => { + if (!name || name.trim().length === 0) { + return createMessage(ACTION_INVALID_NAME_ERROR); + } else if (name !== entityName && !isNameValid(name, usedEntityNames)) { + return createMessage(nameErrorMessage, name); + } - return null; - }, - [nameErrorMessage, usedEntityNames], - ); + return null; + }); - const handleNameSave = useCallback( - (entityName: string, entityId: string) => (name: string) => { - if (name !== entityName && validateName(entityName)(name) === null) { - dispatch(nameSaveAction({ id: entityId, name })); - } - }, - [validateName, dispatch, nameSaveAction], - ); + const handleNameSave = useEventCallback((name: string) => { + if (name !== entityName && validateName(name) === null) { + dispatch(nameSaveAction({ id: entityId, name })); + } + }); return { isNew, From 1d128f6cf0bbf9e9d5a977af3bed1686ac298239 Mon Sep 17 00:00:00 2001 From: Alex Golovanov Date: Mon, 7 Oct 2024 13:19:39 +0300 Subject: [PATCH 15/22] chore: add usehooks-ts to jest config --- app/client/jest.config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/client/jest.config.js b/app/client/jest.config.js index abe29d26d8d..84c6f71ea69 100644 --- a/app/client/jest.config.js +++ b/app/client/jest.config.js @@ -17,7 +17,7 @@ module.exports = { moduleFileExtensions: ["ts", "tsx", "js", "jsx", "json", "node", "css"], moduleDirectories: ["node_modules", "src", "test"], transformIgnorePatterns: [ - "/node_modules/(?!codemirror|konva|react-dnd|dnd-core|@babel|(@blueprintjs)|@github|lodash-es|@draft-js-plugins|react-documents|linkedom|assert-never|axios)", + "/node_modules/(?!codemirror|konva|react-dnd|dnd-core|@babel|(@blueprintjs)|@github|lodash-es|@draft-js-plugins|react-documents|linkedom|assert-never|axios|usehooks-ts)", ], moduleNameMapper: { "\\.(css|less)$": "/test/__mocks__/styleMock.js", From 95f041e888c15ab8d78c2b06738eaa3e5e6fb97a Mon Sep 17 00:00:00 2001 From: Alex Golovanov Date: Mon, 7 Oct 2024 13:21:31 +0300 Subject: [PATCH 16/22] chore: add forwardRef & rename striked to strikethrough for consistency --- .../ads/src/Text/Text.styles.tsx | 6 ++-- .../design-system/ads/src/Text/Text.tsx | 36 ++++++++++--------- .../design-system/ads/src/Text/Text.types.ts | 14 ++++++-- 3 files changed, 34 insertions(+), 22 deletions(-) diff --git a/app/client/packages/design-system/ads/src/Text/Text.styles.tsx b/app/client/packages/design-system/ads/src/Text/Text.styles.tsx index 58318c2cbdb..55f278fa7b1 100644 --- a/app/client/packages/design-system/ads/src/Text/Text.styles.tsx +++ b/app/client/packages/design-system/ads/src/Text/Text.styles.tsx @@ -125,7 +125,7 @@ export const StyledText = styled.span<{ isBold?: boolean; isItalic?: boolean; isUnderlined?: boolean; - isStriked?: boolean; + isStrikethrough?: boolean; isEditable?: boolean; }>` ${TypographyScales} @@ -160,8 +160,8 @@ export const StyledText = styled.span<{ text-decoration: underline; } - /* Striked style */ - &[data-striked="true"] { + /* Strikethrough style */ + &[data-strikethrough="true"] { text-decoration: line-through; } diff --git a/app/client/packages/design-system/ads/src/Text/Text.tsx b/app/client/packages/design-system/ads/src/Text/Text.tsx index 63b83fc0af8..7413380359b 100644 --- a/app/client/packages/design-system/ads/src/Text/Text.tsx +++ b/app/client/packages/design-system/ads/src/Text/Text.tsx @@ -1,6 +1,6 @@ -import React from "react"; +import React, { forwardRef } from "react"; import clsx from "classnames"; -import type { TextProps } from "./Text.types"; +import type { RenderAsElement, TextProps } from "./Text.types"; import { StyledEditableInput, StyledText } from "./Text.styles"; import { TextClassName } from "./Text.constants"; @@ -9,17 +9,20 @@ TODO: - add segment header style to list of styles */ -function Text({ - children, - className, - color, - inputProps, - inputRef, - isEditable, - kind, - renderAs, - ...rest -}: TextProps) { +const Text = forwardRef(function Text( + { + children, + className, + color, + inputProps, + inputRef, + isEditable, + kind, + renderAs, + ...rest + }: TextProps, + ref, +) { return ( {isEditable && typeof children === "string" ? ( @@ -41,13 +45,13 @@ function Text({ )} ); -} +}); Text.displayName = "Text"; Text.defaultProps = { renderAs: "span", - kind: "span", + kind: "body-m", }; export { Text }; diff --git a/app/client/packages/design-system/ads/src/Text/Text.types.ts b/app/client/packages/design-system/ads/src/Text/Text.types.ts index a2751ecee63..07e606bde8b 100644 --- a/app/client/packages/design-system/ads/src/Text/Text.types.ts +++ b/app/client/packages/design-system/ads/src/Text/Text.types.ts @@ -1,6 +1,7 @@ import type { ReactNode } from "react"; import type React from "react"; +/** Text style variant */ export type TextKind = | "heading-xl" | "heading-l" @@ -14,7 +15,14 @@ export type TextKind = | "action-s" | "code"; -// Text props +/** All possible element types text can be rendered as, matches renderAs prop */ +export type RenderAsElement = + | HTMLHeadingElement + | HTMLLabelElement + | HTMLParagraphElement + | HTMLSpanElement; + +/** Text component props */ export type TextProps = { /** to change the rendering component */ renderAs?: "h1" | "h2" | "h3" | "h4" | "h5" | "h6" | "p" | "span" | "label"; @@ -32,8 +40,8 @@ export type TextProps = { isItalic?: boolean; /** whether the text is underlined or not */ isUnderlined?: boolean; - /** whether the text is striked or not */ - isStriked?: boolean; + /** whether the text is strikethrough or not */ + isStrikethrough?: boolean; /** whether the text is editable or not */ isEditable?: boolean; /** input component props while isEditable is true */ From ede76cdd96a2956fdcd0450f2512cc16a5d33aec Mon Sep 17 00:00:00 2001 From: Alex Golovanov Date: Mon, 7 Oct 2024 18:55:12 +0300 Subject: [PATCH 17/22] chore: move test constants to a file --- app/client/src/IDE/Components/FileTab/constants.ts | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 app/client/src/IDE/Components/FileTab/constants.ts diff --git a/app/client/src/IDE/Components/FileTab/constants.ts b/app/client/src/IDE/Components/FileTab/constants.ts new file mode 100644 index 00000000000..e35359172b3 --- /dev/null +++ b/app/client/src/IDE/Components/FileTab/constants.ts @@ -0,0 +1,5 @@ +export const DATA_TEST_ID = { + INPUT: "t--ide-tab-editable-input", + CLOSE_BUTTON: "t--ide-tab-close-btn", + SPINNER: "t--ide-tab-spinner", +}; From bfadb0bfdd4a519be28a5c72a79b44c0e9f3db45 Mon Sep 17 00:00:00 2001 From: Alex Golovanov Date: Mon, 7 Oct 2024 18:56:15 +0300 Subject: [PATCH 18/22] test: add tab tests --- .../IDE/Components/FileTab/FileTab.test.tsx | 203 ++++++++++++++++++ .../src/IDE/Components/FileTab/FileTab.tsx | 6 +- 2 files changed, 207 insertions(+), 2 deletions(-) create mode 100644 app/client/src/IDE/Components/FileTab/FileTab.test.tsx diff --git a/app/client/src/IDE/Components/FileTab/FileTab.test.tsx b/app/client/src/IDE/Components/FileTab/FileTab.test.tsx new file mode 100644 index 00000000000..0e3c6ce84d3 --- /dev/null +++ b/app/client/src/IDE/Components/FileTab/FileTab.test.tsx @@ -0,0 +1,203 @@ +/* eslint-disable react-perf/jsx-no-new-object-as-prop */ +/* eslint-disable react-perf/jsx-no-jsx-as-prop */ +import "@testing-library/jest-dom"; +import React from "react"; +import { render, fireEvent, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; + +import { Icon } from "@appsmith/ads"; +import { FileTab } from "./FileTab"; +import { DATA_TEST_ID } from "./constants"; + +describe("FileTab", () => { + const mockOnClick = jest.fn(); + const mockOnClose = jest.fn(); + + const title = "test_file"; + const TabIcon = () => ; + + const setup = ( + mockEditorConfig: { + onTitleSave: () => void; + titleTransformer: (title: string) => string; + validateTitle: (title: string) => string | null; + } = { + onTitleSave: jest.fn(), + titleTransformer: jest.fn((title) => title), + validateTitle: jest.fn(() => null), + }, + isLoading = false, + ) => { + const utils = render( + } + isActive + isLoading={isLoading} + onClick={mockOnClick} + onClose={mockOnClose} + title={title} + />, + ); + const tabElement = utils.getByText(title); + + return { + tabElement, + ...utils, + ...mockEditorConfig, + }; + }; + + test("renders component", () => { + const { getByTestId, tabElement } = setup(); + + fireEvent.click(tabElement); + expect(mockOnClick).toHaveBeenCalled(); + + const closeButton = getByTestId(DATA_TEST_ID.CLOSE_BUTTON); + + fireEvent.click(closeButton); + expect(mockOnClose).toHaveBeenCalled(); + }); + + test("renders component in loading state", () => { + const { getByTestId, tabElement } = setup(undefined, true); + + fireEvent.click(tabElement); + expect(mockOnClick).toHaveBeenCalled(); + + const spinner = getByTestId(DATA_TEST_ID.SPINNER); + + fireEvent.click(spinner); + + const closeButton = getByTestId(DATA_TEST_ID.CLOSE_BUTTON); + + fireEvent.click(closeButton); + expect(mockOnClose).toHaveBeenCalled(); + }); + + test("enters edit mode on double click", () => { + const { getByTestId, tabElement } = setup(); + + fireEvent.doubleClick(tabElement); + within(tabElement).getByTestId(DATA_TEST_ID.INPUT); + + const closeButton = getByTestId(DATA_TEST_ID.CLOSE_BUTTON); + + fireEvent.click(closeButton); + expect(mockOnClose).toHaveBeenCalled(); + }); + + test("edit and hit enter", () => { + const { + getByTestId, + getByText, + onTitleSave, + tabElement, + titleTransformer, + validateTitle, + } = setup(); + + const newTitle = "new_title"; + + fireEvent.doubleClick(tabElement); + const inputElement = getByTestId(DATA_TEST_ID.INPUT); + + fireEvent.change(inputElement, { target: { value: newTitle } }); + expect(titleTransformer).toHaveBeenCalledWith(newTitle); + + fireEvent.keyUp(inputElement, { key: "Enter", keyCode: 13 }); + + expect(titleTransformer).toHaveBeenCalledWith(newTitle); + expect(validateTitle).toHaveBeenCalledWith(newTitle); + expect(onTitleSave).toHaveBeenCalledWith(newTitle); + + expect(getByText(newTitle)).toBeInTheDocument(); + }); + + test("edit and click outside", async () => { + const { + getByTestId, + getByText, + onTitleSave, + tabElement, + titleTransformer, + validateTitle, + } = setup(); + + const newTitle = "new_title"; + + fireEvent.doubleClick(tabElement); + const inputElement = getByTestId(DATA_TEST_ID.INPUT); + + fireEvent.change(inputElement, { target: { value: newTitle } }); + expect(titleTransformer).toHaveBeenCalledWith(newTitle); + + await userEvent.click(document.body); + + expect(titleTransformer).toHaveBeenCalledWith(newTitle); + expect(validateTitle).toHaveBeenCalledWith(newTitle); + expect(onTitleSave).toHaveBeenCalledWith(newTitle); + + expect(getByText(newTitle)).toBeInTheDocument(); + }); + + test("edit and hit esc", () => { + const { + getByTestId, + getByText, + queryByText, + tabElement, + titleTransformer, + } = setup(); + + const newTitle = "new_title"; + + fireEvent.doubleClick(tabElement); + const inputElement = getByTestId(DATA_TEST_ID.INPUT); + + fireEvent.change(inputElement, { target: { value: newTitle } }); + expect(titleTransformer).toHaveBeenCalledWith(newTitle); + + fireEvent.keyUp(inputElement, { key: "Esc", keyCode: 27 }); + + const newTab = queryByText(newTitle); + const oldTab = getByText(title); + + expect(newTab).not.toBeInTheDocument(); + expect(oldTab).toBeInTheDocument(); + }); + + test("enter invalid title", () => { + const validationError = "Invalid title"; + + const { + getByTestId, + getByText, + tabElement, + titleTransformer, + validateTitle, + } = setup({ + onTitleSave: jest.fn(), + titleTransformer: jest.fn((title) => title), + validateTitle: jest.fn(() => validationError), + }); + + const invalidTitle = "else"; + + fireEvent.doubleClick(tabElement); + const inputElement = getByTestId(DATA_TEST_ID.INPUT); + + fireEvent.change(inputElement, { target: { value: invalidTitle } }); + expect(titleTransformer).toHaveBeenCalledWith(invalidTitle); + + fireEvent.keyUp(inputElement, { key: "Enter", keyCode: 13 }); + + expect(titleTransformer).toHaveBeenCalledWith(invalidTitle); + expect(validateTitle).toHaveBeenCalledWith(invalidTitle); + + const tooltip = getByText(validationError); + + expect(tooltip).toBeInTheDocument(); + }); +}); diff --git a/app/client/src/IDE/Components/FileTab/FileTab.tsx b/app/client/src/IDE/Components/FileTab/FileTab.tsx index 2f99afe1f21..fd415664d14 100644 --- a/app/client/src/IDE/Components/FileTab/FileTab.tsx +++ b/app/client/src/IDE/Components/FileTab/FileTab.tsx @@ -9,6 +9,7 @@ import { useBoolean, useEventCallback, useOnClickOutside } from "usehooks-ts"; import { usePrevious } from "@mantine/hooks"; import * as Styled from "./styles"; +import { DATA_TEST_ID } from "./constants"; export interface FileTabProps { isActive: boolean; @@ -100,6 +101,7 @@ export const FileTab = ({ const inputProps = useMemo( () => ({ + ["data-testid"]: DATA_TEST_ID.INPUT, onKeyUp: handleKeyUp, onChange: handleTitleChange, autoFocus: true, @@ -142,7 +144,7 @@ export const FileTab = ({ {icon && !isLoading ? ( {icon} ) : null} - {isLoading && } + {isLoading && } From 47ee66b3be496fd017b9e58bd6647235ea988486 Mon Sep 17 00:00:00 2001 From: Alex Golovanov Date: Mon, 7 Oct 2024 19:42:07 +0300 Subject: [PATCH 19/22] chore: correct typing and story --- .../ads/src/Text/Text.stories.tsx | 18 ++++++++++-------- .../design-system/ads/src/Text/Text.types.ts | 5 +---- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/app/client/packages/design-system/ads/src/Text/Text.stories.tsx b/app/client/packages/design-system/ads/src/Text/Text.stories.tsx index 640ed86334e..3eff9aec033 100644 --- a/app/client/packages/design-system/ads/src/Text/Text.stories.tsx +++ b/app/client/packages/design-system/ads/src/Text/Text.stories.tsx @@ -1,4 +1,4 @@ -import React from "react"; +import React, { useMemo } from "react"; import { Text } from "./Text"; import type { Meta, StoryObj } from "@storybook/react"; @@ -25,15 +25,17 @@ export const EditableTextStory: Story = { }, render: function Render(args) { const [text, setText] = React.useState(args.children); + const inputProps = useMemo( + () => ({ + onChange: (e: React.ChangeEvent) => { + setText(e.target.value); + }, + }), + [], + ); return ( - { - // @ts-expect-error type error - setText(e.target.value); - }} - > + {text} ); diff --git a/app/client/packages/design-system/ads/src/Text/Text.types.ts b/app/client/packages/design-system/ads/src/Text/Text.types.ts index 07e606bde8b..76323ede3fe 100644 --- a/app/client/packages/design-system/ads/src/Text/Text.types.ts +++ b/app/client/packages/design-system/ads/src/Text/Text.types.ts @@ -45,10 +45,7 @@ export type TextProps = { /** whether the text is editable or not */ isEditable?: boolean; /** input component props while isEditable is true */ - inputProps?: Omit< - React.InputHTMLAttributes, - "value" | "onChange" - >; + inputProps?: Omit, "value">; /** ref for input component */ inputRef?: React.RefObject; } & React.HTMLAttributes & From d05671f71404ecbfd1f3da72694b50d2dfc4a2d5 Mon Sep 17 00:00:00 2001 From: Alex Golovanov Date: Mon, 7 Oct 2024 22:08:39 +0300 Subject: [PATCH 20/22] fix: revert tab close button name change --- app/client/src/IDE/Components/FileTab/constants.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/client/src/IDE/Components/FileTab/constants.ts b/app/client/src/IDE/Components/FileTab/constants.ts index e35359172b3..4408a5b065a 100644 --- a/app/client/src/IDE/Components/FileTab/constants.ts +++ b/app/client/src/IDE/Components/FileTab/constants.ts @@ -1,5 +1,5 @@ export const DATA_TEST_ID = { INPUT: "t--ide-tab-editable-input", - CLOSE_BUTTON: "t--ide-tab-close-btn", + CLOSE_BUTTON: "t--tab-close-btn", SPINNER: "t--ide-tab-spinner", }; From a47adb561ba842d82b2b31d65e9f35a7f0ed60ce Mon Sep 17 00:00:00 2001 From: Alex Golovanov Date: Mon, 7 Oct 2024 23:10:26 +0300 Subject: [PATCH 21/22] chore: fix a linter error --- app/client/src/pages/Editor/IDE/hooks.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/app/client/src/pages/Editor/IDE/hooks.ts b/app/client/src/pages/Editor/IDE/hooks.ts index a5e3cf834ac..e4eeba31981 100644 --- a/app/client/src/pages/Editor/IDE/hooks.ts +++ b/app/client/src/pages/Editor/IDE/hooks.ts @@ -179,6 +179,7 @@ export const useIDETabClickHandlers = () => { const tabClickHandler = useCallback( (item: EntityItem) => { const navigateToUrl = tabsConfig.itemUrlSelector(item, basePageId); + if (navigateToUrl !== history.location.pathname) { history.push(navigateToUrl, { invokedBy: NavigationMethod.EditorTabs, From b1f6a7c57b3c927965ce1e287e46fdb0908e91ba Mon Sep 17 00:00:00 2001 From: Alex Golovanov Date: Thu, 10 Oct 2024 16:09:32 +0300 Subject: [PATCH 22/22] fix: addressed focusOut related issues --- .../IDE/Components/FileTab/FileTab.test.tsx | 149 ++++++++++-------- .../src/IDE/Components/FileTab/FileTab.tsx | 86 ++++++---- 2 files changed, 135 insertions(+), 100 deletions(-) diff --git a/app/client/src/IDE/Components/FileTab/FileTab.test.tsx b/app/client/src/IDE/Components/FileTab/FileTab.test.tsx index 0e3c6ce84d3..38ff125fe0f 100644 --- a/app/client/src/IDE/Components/FileTab/FileTab.test.tsx +++ b/app/client/src/IDE/Components/FileTab/FileTab.test.tsx @@ -13,8 +13,12 @@ describe("FileTab", () => { const mockOnClick = jest.fn(); const mockOnClose = jest.fn(); - const title = "test_file"; + const TITLE = "test_file"; const TabIcon = () => ; + const KEY_CONFIG = { + ENTER: { key: "Enter", keyCode: 13 }, + ESC: { key: "Esc", keyCode: 27 }, + }; const setup = ( mockEditorConfig: { @@ -36,10 +40,10 @@ describe("FileTab", () => { isLoading={isLoading} onClick={mockOnClick} onClose={mockOnClose} - title={title} + title={TITLE} />, ); - const tabElement = utils.getByText(title); + const tabElement = utils.getByText(TITLE); return { tabElement, @@ -88,92 +92,82 @@ describe("FileTab", () => { expect(mockOnClose).toHaveBeenCalled(); }); - test("edit and hit enter", () => { + test("valid title actions", async () => { const { getByTestId, getByText, onTitleSave, + queryByText, tabElement, titleTransformer, validateTitle, } = setup(); - const newTitle = "new_title"; + // hit enter + const enterTitle = "enter_title"; fireEvent.doubleClick(tabElement); - const inputElement = getByTestId(DATA_TEST_ID.INPUT); - - fireEvent.change(inputElement, { target: { value: newTitle } }); - expect(titleTransformer).toHaveBeenCalledWith(newTitle); - - fireEvent.keyUp(inputElement, { key: "Enter", keyCode: 13 }); - - expect(titleTransformer).toHaveBeenCalledWith(newTitle); - expect(validateTitle).toHaveBeenCalledWith(newTitle); - expect(onTitleSave).toHaveBeenCalledWith(newTitle); - - expect(getByText(newTitle)).toBeInTheDocument(); - }); + fireEvent.change(getByTestId(DATA_TEST_ID.INPUT), { + target: { value: enterTitle }, + }); + expect(titleTransformer).toHaveBeenCalledWith(enterTitle); - test("edit and click outside", async () => { - const { - getByTestId, - getByText, - onTitleSave, - tabElement, - titleTransformer, - validateTitle, - } = setup(); + fireEvent.keyUp(getByTestId(DATA_TEST_ID.INPUT), KEY_CONFIG.ENTER); + expect(titleTransformer).toHaveBeenCalledWith(enterTitle); + expect(validateTitle).toHaveBeenCalledWith(enterTitle); + expect(onTitleSave).toHaveBeenCalledWith(enterTitle); + expect(getByText(enterTitle)).toBeInTheDocument(); - const newTitle = "new_title"; + // click outside + const clickOutsideTitle = "click_outside_title"; fireEvent.doubleClick(tabElement); - const inputElement = getByTestId(DATA_TEST_ID.INPUT); - - fireEvent.change(inputElement, { target: { value: newTitle } }); - expect(titleTransformer).toHaveBeenCalledWith(newTitle); + fireEvent.change(getByTestId(DATA_TEST_ID.INPUT), { + target: { value: clickOutsideTitle }, + }); + expect(titleTransformer).toHaveBeenCalledWith(clickOutsideTitle); await userEvent.click(document.body); + expect(titleTransformer).toHaveBeenCalledWith(clickOutsideTitle); + expect(validateTitle).toHaveBeenCalledWith(clickOutsideTitle); + expect(onTitleSave).toHaveBeenCalledWith(clickOutsideTitle); + expect(getByText(clickOutsideTitle)).toBeInTheDocument(); - expect(titleTransformer).toHaveBeenCalledWith(newTitle); - expect(validateTitle).toHaveBeenCalledWith(newTitle); - expect(onTitleSave).toHaveBeenCalledWith(newTitle); - - expect(getByText(newTitle)).toBeInTheDocument(); - }); - - test("edit and hit esc", () => { - const { - getByTestId, - getByText, - queryByText, - tabElement, - titleTransformer, - } = setup(); - - const newTitle = "new_title"; + // hit esc + const escapeTitle = "escape_title"; fireEvent.doubleClick(tabElement); - const inputElement = getByTestId(DATA_TEST_ID.INPUT); + fireEvent.change(getByTestId(DATA_TEST_ID.INPUT), { + target: { value: escapeTitle }, + }); + expect(titleTransformer).toHaveBeenCalledWith(escapeTitle); - fireEvent.change(inputElement, { target: { value: newTitle } }); - expect(titleTransformer).toHaveBeenCalledWith(newTitle); + fireEvent.keyUp(getByTestId(DATA_TEST_ID.INPUT), KEY_CONFIG.ESC); + expect(queryByText(escapeTitle)).not.toBeInTheDocument(); + expect(getByText(TITLE)).toBeInTheDocument(); - fireEvent.keyUp(inputElement, { key: "Esc", keyCode: 27 }); + // focus out event + const focusOutTitle = "focus_out_title"; - const newTab = queryByText(newTitle); - const oldTab = getByText(title); + fireEvent.doubleClick(tabElement); + fireEvent.change(getByTestId(DATA_TEST_ID.INPUT), { + target: { value: focusOutTitle }, + }); + expect(titleTransformer).toHaveBeenCalledWith(focusOutTitle); - expect(newTab).not.toBeInTheDocument(); - expect(oldTab).toBeInTheDocument(); + fireEvent.keyUp(getByTestId(DATA_TEST_ID.INPUT), KEY_CONFIG.ESC); + expect(queryByText(focusOutTitle)).not.toBeInTheDocument(); + expect(getByText(TITLE)).toBeInTheDocument(); }); - test("enter invalid title", () => { + test("invalid title actions", async () => { const validationError = "Invalid title"; + const invalidTitle = "else"; const { getByTestId, getByText, + queryByText, tabElement, titleTransformer, validateTitle, @@ -183,21 +177,44 @@ describe("FileTab", () => { validateTitle: jest.fn(() => validationError), }); - const invalidTitle = "else"; - + // click outside fireEvent.doubleClick(tabElement); - const inputElement = getByTestId(DATA_TEST_ID.INPUT); + fireEvent.change(getByTestId(DATA_TEST_ID.INPUT), { + target: { value: invalidTitle }, + }); + expect(titleTransformer).toHaveBeenCalledWith(invalidTitle); - fireEvent.change(inputElement, { target: { value: invalidTitle } }); + fireEvent.keyUp(getByTestId(DATA_TEST_ID.INPUT), KEY_CONFIG.ENTER); expect(titleTransformer).toHaveBeenCalledWith(invalidTitle); + expect(validateTitle).toHaveBeenCalledWith(invalidTitle); + expect(getByText(validationError)).toBeInTheDocument(); - fireEvent.keyUp(inputElement, { key: "Enter", keyCode: 13 }); + await userEvent.click(document.body); + expect(queryByText(validationError)).not.toBeInTheDocument(); + expect(getByText(TITLE)).toBeInTheDocument(); + // escape + fireEvent.doubleClick(tabElement); + fireEvent.change(getByTestId(DATA_TEST_ID.INPUT), { + target: { value: invalidTitle }, + }); expect(titleTransformer).toHaveBeenCalledWith(invalidTitle); - expect(validateTitle).toHaveBeenCalledWith(invalidTitle); - const tooltip = getByText(validationError); + fireEvent.keyUp(getByTestId(DATA_TEST_ID.INPUT), KEY_CONFIG.ENTER); + fireEvent.keyUp(getByTestId(DATA_TEST_ID.INPUT), KEY_CONFIG.ESC); + expect(queryByText(validationError)).not.toBeInTheDocument(); + expect(getByText(TITLE)).toBeInTheDocument(); + + // focus out event + fireEvent.doubleClick(tabElement); + fireEvent.change(getByTestId(DATA_TEST_ID.INPUT), { + target: { value: invalidTitle }, + }); + expect(titleTransformer).toHaveBeenCalledWith(invalidTitle); - expect(tooltip).toBeInTheDocument(); + fireEvent.keyUp(getByTestId(DATA_TEST_ID.INPUT), KEY_CONFIG.ENTER); + fireEvent.focusOut(getByTestId(DATA_TEST_ID.INPUT)); + expect(queryByText(validationError)).not.toBeInTheDocument(); + expect(getByText(TITLE)).toBeInTheDocument(); }); }); diff --git a/app/client/src/IDE/Components/FileTab/FileTab.tsx b/app/client/src/IDE/Components/FileTab/FileTab.tsx index fd415664d14..5de08640815 100644 --- a/app/client/src/IDE/Components/FileTab/FileTab.tsx +++ b/app/client/src/IDE/Components/FileTab/FileTab.tsx @@ -5,7 +5,7 @@ import { noop } from "lodash"; import { Icon, Spinner, Tooltip } from "@appsmith/ads"; import { sanitizeString } from "utils/URLUtils"; -import { useBoolean, useEventCallback, useOnClickOutside } from "usehooks-ts"; +import { useBoolean, useEventCallback, useEventListener } from "usehooks-ts"; import { usePrevious } from "@mantine/hooks"; import * as Styled from "./styles"; @@ -50,24 +50,20 @@ export const FileTab = ({ const [validationError, setValidationError] = useState(null); const inputRef = useRef(null); - const attemptToSave = () => { - if (editorConfig) { - const { onTitleSave, validateTitle } = editorConfig; - const nameError = validateTitle(editableTitle); - - if (nameError !== null) { - setValidationError(nameError); - } else { - exitEditMode(); - onTitleSave(editableTitle); - } - } - }; - const handleKeyUp = useEventCallback( (e: React.KeyboardEvent) => { if (e.key === "Enter") { - attemptToSave(); + if (editorConfig) { + const { onTitleSave, validateTitle } = editorConfig; + const nameError = validateTitle(editableTitle); + + if (nameError === null) { + exitEditMode(); + onTitleSave(editableTitle); + } else { + setValidationError(nameError); + } + } } else if (e.key === "Escape") { exitEditMode(); setEditableTitle(title); @@ -78,10 +74,6 @@ export const FileTab = ({ }, ); - useOnClickOutside(inputRef, () => { - attemptToSave(); - }); - const handleTitleChange = useEventCallback( (e: React.ChangeEvent) => { setEditableTitle( @@ -115,24 +107,50 @@ export const FileTab = ({ [handleKeyUp, handleTitleChange], ); - useEffect(() => { - if (!isEditing && previousTitle !== title) { - setEditableTitle(title); - } - }, [title, previousTitle, isEditing]); + useEventListener( + "focusout", + function handleFocusOut() { + if (isEditing && editorConfig) { + const { onTitleSave, validateTitle } = editorConfig; + const nameError = validateTitle(editableTitle); + + exitEditMode(); + + if (nameError === null) { + onTitleSave(editableTitle); + } else { + setEditableTitle(title); + setValidationError(null); + } + } + }, + inputRef, + ); + + useEffect( + function syncEditableTitle() { + if (!isEditing && previousTitle !== title) { + setEditableTitle(title); + } + }, + [title, previousTitle, isEditing], + ); // TODO: This is a temporary fix to focus the input after context retention applies focus to its target // this is a nasty hack to re-focus the input after context retention applies focus to its target // this will be addressed in a future task, likely by a focus retention modification - useEffect(() => { - const input = inputRef.current; - - if (isEditing && input) { - setTimeout(() => { - input.focus(); - }, 200); - } - }, [isEditing]); + useEffect( + function recaptureFocusInEventOfFocusRetention() { + const input = inputRef.current; + + if (isEditing && input) { + setTimeout(() => { + input.focus(); + }, 200); + } + }, + [isEditing], + ); return (