-
Notifications
You must be signed in to change notification settings - Fork 4.6k
feat: editable ide tabs #36665
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
feat: editable ide tabs #36665
Changes from 10 commits
4c4694f
295a041
144edb7
8f09c44
6924378
6f6bc3d
195daa7
70add12
2d6bc28
b98f363
f2c6924
aa4c87e
d6cc17a
df31a7a
072f1d6
1d128f6
95f041e
ede76cd
bfadb0b
47ee66b
d05671f
a47adb5
b1f6a7c
14a6c75
dc89acb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,162 @@ | ||
| 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"; | ||
|
alex-golovanov marked this conversation as resolved.
Outdated
|
||
|
|
||
| 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<string | null>(null); | ||
| const inputRef = useRef<HTMLInputElement>(null); | ||
|
|
||
| const attemptToSave = () => { | ||
|
alex-golovanov marked this conversation as resolved.
Outdated
|
||
| if (editorConfig) { | ||
| const { onTitleSave, validateTitle } = editorConfig; | ||
| const nameError = validateTitle(editableTitle); | ||
|
|
||
| if (nameError !== null) { | ||
| setValidationError(nameError); | ||
| } else { | ||
| exitEditMode(); | ||
| onTitleSave(editableTitle); | ||
| } | ||
| } | ||
| }; | ||
|
alex-golovanov marked this conversation as resolved.
Outdated
|
||
|
|
||
| const handleKeyUp = useEventCallback( | ||
| (e: React.KeyboardEvent<HTMLInputElement>) => { | ||
| 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<HTMLInputElement>) => { | ||
| setEditableTitle( | ||
| editorConfig | ||
| ? editorConfig.titleTransformer(e.target.value) | ||
| : e.target.value, | ||
|
alex-golovanov marked this conversation as resolved.
|
||
| ); | ||
| }, | ||
| ); | ||
|
|
||
| const handleEnterEditMode = useEventCallback(() => { | ||
| setEditableTitle(title); | ||
| enterEditMode(); | ||
| }); | ||
|
|
||
| const handleDoubleClick = editorConfig ? handleEnterEditMode : noop; | ||
|
|
||
| const inputProps = useMemo( | ||
| () => ({ | ||
| onKeyUp: handleKeyUp, | ||
| onChange: handleTitleChange, | ||
| autofocus: true, | ||
|
alex-golovanov marked this conversation as resolved.
Outdated
alex-golovanov marked this conversation as resolved.
Outdated
alex-golovanov marked this conversation as resolved.
Outdated
|
||
| 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]); | ||
|
alex-golovanov marked this conversation as resolved.
Outdated
|
||
|
|
||
| return ( | ||
| <Styled.Tab | ||
| className={clsx("editor-tab", isActive && "active")} | ||
| data-testid={`t--ide-tab-${sanitizeString(title)}`} | ||
| onClick={onClick} | ||
| onDoubleClick={handleDoubleClick} | ||
| > | ||
| {icon && !isLoading ? ( | ||
| <Styled.IconContainer>{icon}</Styled.IconContainer> | ||
| ) : null} | ||
| {isLoading && <Spinner size="sm" />} | ||
|
|
||
| <Tooltip content={validationError} visible={Boolean(validationError)}> | ||
| <Styled.Text | ||
| inputProps={inputProps} | ||
| inputRef={inputRef} | ||
| isEditable={isEditing} | ||
| kind="body-s" | ||
| > | ||
| {currentTitle} | ||
| </Styled.Text> | ||
| </Tooltip> | ||
|
|
||
| {/* not using button component because of the size not matching design */} | ||
| <Icon | ||
| 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} | ||
|
alex-golovanov marked this conversation as resolved.
Outdated
|
||
| /> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Enhance maintainability by moving inline styles to styled components. Inline styles within the You can create a styled component for the close icon: // In styles.ts
export const CloseIcon = styled(Icon)`
border-radius: 4px;
padding: 2px;
cursor: pointer;
&:hover {
background-color: var(--ads-v2-colors-action-tertiary-surface-hover-bg);
}
`;Then update your JSX: - <Icon
- className="tab-close rounded-[4px] hover:bg-[var(--ads-v2-colors-action-tertiary-surface-hover-bg)] cursor-pointer p-[2px]"
+ <Styled.CloseIcon
data-testid="t--tab-close-btn"
name="close-line"
onClick={onClose}
/> |
||
| </Styled.Tab> | ||
| ); | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export { FileTab } from "./FileTab"; |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -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; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Remember to use design tokens for font sizes. Class, on line 9, we have the Let's adjust it as follows: - font-size: 12px;
+ font-size: var(--ads-v2-font-size-3);Ensure that 📝 Committable suggestion
Suggested change
|
||||||
| 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 | ||||||
|
alex-golovanov marked this conversation as resolved.
|
||||||
| 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; | ||||||
| } | ||||||
| `; | ||||||
|
alex-golovanov marked this conversation as resolved.
|
||||||
|
|
||||||
| export const IconContainer = styled.div` | ||||||
| height: 12px; | ||||||
| width: 12px; | ||||||
|
alex-golovanov marked this conversation as resolved.
|
||||||
| display: flex; | ||||||
| align-items: center; | ||||||
| justify-content: center; | ||||||
| flex-shrink: 0; | ||||||
| img { | ||||||
| width: 12px; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Ensure consistent icon sizing using container dimensions. Class, on line 51, the Adjust it as follows: - width: 12px;
+ width: 100%;
+ height: 100%;This ensures the icon scales appropriately within its container.
|
||||||
| } | ||||||
| `; | ||||||
|
|
||||||
| export const Text = styled(ADSText)` | ||||||
| min-width: 3ch; | ||||||
| padding: 0 var(--ads-v2-spaces-1); | ||||||
| `; | ||||||
Uh oh!
There was an error while loading. Please reload this page.