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

feat: support multiline for EditableText #2683

Merged
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
edb0a8f
add multiline and support having a textarea instead of input
gabriel-amram Dec 24, 2024
ba0691d
feat: allow multilines for editable text. No breaking changes to exis…
gabriel-amram Dec 24, 2024
3cf410b
feat: allow multilines for editable text. No breaking changes to exis…
gabriel-amram Dec 24, 2024
4aac9b0
linting
gabriel-amram Dec 24, 2024
884570f
l
gabriel-amram Dec 24, 2024
0a560dd
Merge branch 'gabrielam/add-multiline-to-editable-text' of https://gi…
gabriel-amram Dec 24, 2024
fb6e211
fix some styling and calculating height issues
gabriel-amram Dec 25, 2024
701c3a7
don't change the ref explicitly, but use the state
gabriel-amram Dec 26, 2024
9eb4a4e
shift-enter creates the new line, while simple enter defocuses
gabriel-amram Dec 26, 2024
cbb1812
added and fixed tests
gabriel-amram Dec 26, 2024
0440768
Merge branch 'master' into gabrielam/add-multiline-to-editable-text
gabriel-amram Dec 26, 2024
61a9790
CR comments
gabriel-amram Dec 29, 2024
5431d50
Merge branch 'gabrielam/add-multiline-to-editable-text' of https://gi…
gabriel-amram Dec 29, 2024
dd72841
reduce rendering by using inline
gabriel-amram Dec 30, 2024
2e29cde
reduce renders, cache the textarea attrs
gabriel-amram Dec 30, 2024
f7aace2
remove comments
gabriel-amram Dec 30, 2024
7be9a14
use pre-wrap for line breaks
gabriel-amram Dec 31, 2024
c2210dc
sync with origin
gabriel-amram Dec 31, 2024
3c6e3b9
Merge branch 'master' into gabrielam/add-multiline-to-editable-text
gabriel-amram Dec 31, 2024
a4c5cb6
minor cr notes
gabriel-amram Dec 31, 2024
f9269c4
Merge branch 'gabrielam/add-multiline-to-editable-text' of https://gi…
gabriel-amram Dec 31, 2024
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
13 changes: 12 additions & 1 deletion packages/core/src/components/EditableText/EditableText.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,24 @@ export interface EditableTextProps extends VibeComponentProps, EditableTypograph
/** Sets the Text weight
*/
weight?: TextWeight;
/** Controls whether a textarea or a simple input would be rendered, allowing multi-lines
gabriel-amram marked this conversation as resolved.
Show resolved Hide resolved
*/
multiline?: boolean;
}

const EditableText: VibeComponent<EditableTextProps, HTMLElement> & {
types?: typeof TextTypeEnum;
weights?: typeof TextWeightEnum;
} = forwardRef(
(
{ type = "text2", weight = "normal", "data-testid": dataTestId, id, ...editableTypographyProps }: EditableTextProps,
{
type = "text2",
weight = "normal",
"data-testid": dataTestId,
id,
multiline = false,
gabriel-amram marked this conversation as resolved.
Show resolved Hide resolved
...editableTypographyProps
}: EditableTextProps,
ref
) => {
return (
Expand All @@ -38,6 +48,7 @@ const EditableText: VibeComponent<EditableTextProps, HTMLElement> & {
clearable
type={type}
weight={weight}
multiline={multiline}
{...editableTypographyProps}
/>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,12 @@ Editable text can be used with any of the <StorybookLink page="Text/Text">Text</

<Canvas of={EditableTextStories.Types} />

### Multiline

Editable text can be used to allow multiline input.

<Canvas of={EditableTextStories.Multiline} />

## Related components

<RelatedComponents componentsNames={[TEXT, HEADING, EDITABLE_HEADING]} />
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,13 @@
}
}
}

.multilineContainer {
gabriel-amram marked this conversation as resolved.
Show resolved Hide resolved
display: flex;
flex-direction: row;

.editableText {
width: 400px;
max-height: 200px;
gabriel-amram marked this conversation as resolved.
Show resolved Hide resolved
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -102,3 +102,25 @@ export const Types = {
}
}
};

const multilineValue = "This is a multiline\nhere's the second line";
gabriel-amram marked this conversation as resolved.
Show resolved Hide resolved
export const Multiline = {
render: () => (
<div className={styles.multilineContainer}>
<EditableText
type={EditableText.types.TEXT1}
weight={EditableText.weights.NORMAL}
multiline
value={multilineValue}
className={styles.editableText}
talkor marked this conversation as resolved.
Show resolved Hide resolved
/>
</div>
),
parameters: {
docs: {
liveEdit: {
scope: { styles }
}
}
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,17 @@ describe("EditableText", () => {
expect(input).toBeInTheDocument();
});

it("should render a textarea in edit mode with multiline", () => {
render(<EditableText multiline value="Editable text" />);

const component = screen.getByRole("button");
fireEvent.click(component);

const input = screen.queryByRole("input");
expect(input).toBeInTheDocument();
expect(input.tagName).toBe("TEXTAREA");
gabriel-amram marked this conversation as resolved.
Show resolved Hide resolved
});

it("should not render an input when 'readOnly' is false when clicked", () => {
render(<EditableText value="Editable test" readOnly />);

Expand Down Expand Up @@ -98,6 +109,29 @@ describe("EditableText", () => {
expect(onChange).toHaveBeenCalledWith(newValue);
});

it("should call onChange with new value when changed in a multiline editable component", async () => {
const value = "Editable test";
const newValue = "New Editable test";
render(<EditableText value={value} onChange={onChange} multiline />);

const component = screen.getByRole("button");
fireEvent.click(component);

const input = screen.getByRole("input");
fireEvent.change(input, {
target: { value: newValue }
});

expect(input).toHaveValue(newValue);

await waitFor(() => {
fireEvent.keyDown(input, { key: "Enter" });
});
expect(within(await screen.findByRole("button")).getByText(newValue)).toBeInTheDocument();
expect(onChange).toHaveBeenCalledTimes(1);
expect(onChange).toHaveBeenCalledWith(newValue);
});

it("should not call onChange when value isn't changed in an editable component", async () => {
const value = "Editable test";
render(<EditableText value={value} onChange={onChange} />);
Expand All @@ -118,6 +152,24 @@ describe("EditableText", () => {
expect(onChange).not.toBeCalled();
});

it("should not call onChange when value changed but Shift+Enter was clicked for multiline in an editable component", async () => {
const value = "Editable test";
render(<EditableText value={value} onChange={onChange} multiline />);

const component = screen.getByRole("button");
fireEvent.click(component);

const input = screen.getByRole("input");

expect(input).toHaveValue(value);

await waitFor(() => {
fireEvent.keyDown(input, { key: "Enter", shiftKey: true });
});

expect(onChange).not.toBeCalled();
});

it("should not call onChange when value of an editable component is cleared", async () => {
const value = "Editable test";
render(<EditableText value={value} onChange={onChange} />);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
overflow: hidden;
position: relative;

.input {
.input,.textarea {
display: inline-block;
max-width: 100%;
min-width: 64px;
Expand All @@ -26,6 +26,12 @@
}
}

.textarea {
max-height: inherit;
gabriel-amram marked this conversation as resolved.
Show resolved Hide resolved
resize: none;
box-sizing: border-box;
gabriel-amram marked this conversation as resolved.
Show resolved Hide resolved
}

.typography {
border: 1px solid transparent;
padding: 0 var(--spacing-xs);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ export interface EditableTypographyProps extends VibeComponentProps, EditableTyp
type?: TextType | HeadingType;
/** Sets the Text/Heading weight */
weight?: TextWeight | HeadingWeight;
/** Controls whether a textarea or a simple input would be rendered, allowing multi-lines */
multiline?: boolean;
talkor marked this conversation as resolved.
Show resolved Hide resolved
}

const EditableTypography: VibeComponent<EditableTypographyProps, HTMLElement> = forwardRef(
Expand All @@ -64,7 +66,8 @@ const EditableTypography: VibeComponent<EditableTypographyProps, HTMLElement> =
onEditModeChange,
tooltipProps,
type,
weight
weight,
multiline = false
}: EditableTypographyProps,
ref
) => {
Expand All @@ -74,6 +77,7 @@ const EditableTypography: VibeComponent<EditableTypographyProps, HTMLElement> =
const [isEditing, setIsEditing] = useState(isEditMode || false);
const [inputValue, setInputValue] = useState(value);
const [inputWidth, setInputWidth] = useState(0);
const [inputHeight, setInputHeight] = useState<number | string>(0);

const prevValue = usePrevious(value);

Expand Down Expand Up @@ -128,8 +132,12 @@ const EditableTypography: VibeComponent<EditableTypographyProps, HTMLElement> =
handleInputValueChange();
}

function handleKeyDown(event: React.KeyboardEvent<HTMLInputElement>) {
function handleKeyDown(event: React.KeyboardEvent<HTMLInputElement | HTMLTextAreaElement>) {
if (event.key === keyCodes.ENTER) {
if (multiline && event.shiftKey) {
return;
}

handleInputValueChange();
}
if (event.key === keyCodes.ESCAPE) {
Expand All @@ -138,15 +146,56 @@ const EditableTypography: VibeComponent<EditableTypographyProps, HTMLElement> =
}
}

function handleChange(event: React.ChangeEvent<HTMLInputElement>) {
function handleChange(event: React.ChangeEvent<HTMLInputElement | HTMLTextAreaElement>) {
setInputValue(event.target.value);

if (multiline) {
resizeTextarea();
}
}

const toggleKeyboardEditMode = useKeyboardButtonPressedFunc(toggleEditMode);

function focus() {
if (inputRef.current) {
inputRef.current?.focus();

if (multiline) {
resizeTextarea();
gabriel-amram marked this conversation as resolved.
Show resolved Hide resolved
}
}
}

/* Dynamically resizes the textarea to fit its content */
function resizeTextarea() {
gabriel-amram marked this conversation as resolved.
Show resolved Hide resolved
console.log("*************************resize called, why?", multiline, inputRef.current);
gabriel-amram marked this conversation as resolved.
Show resolved Hide resolved
if (inputRef.current) {
// Temporarily set the height to "auto" to accurately measure the scroll height of the content inside the textarea.
setInputHeight("auto");
gabriel-amram marked this conversation as resolved.
Show resolved Hide resolved

requestAnimationFrame(() => {
gabriel-amram marked this conversation as resolved.
Show resolved Hide resolved
const textarea = inputRef.current as HTMLTextAreaElement;

if (!textarea) {
return;
}

const computedStyle = window.getComputedStyle(textarea);
gabriel-amram marked this conversation as resolved.
Show resolved Hide resolved

// Calculate the appropriate height by taking into account the scrollable content inside the textarea,
// as well as the styles applied to it, such as padding and border widths.
const lineHeight = parseFloat(computedStyle.lineHeight) || 16;
const paddingTop = parseFloat(computedStyle.paddingTop) || 0;
const paddingBottom = parseFloat(computedStyle.paddingBottom) || 0;
const borderTopWidth = parseFloat(computedStyle.borderTopWidth) || 0;
const borderBottomWidth = parseFloat(computedStyle.borderBottomWidth) || 0;

const newHeight = textarea.scrollHeight + borderTopWidth + borderBottomWidth;
const minHeight = lineHeight + paddingTop + paddingBottom + borderTopWidth + borderBottomWidth;
gabriel-amram marked this conversation as resolved.
Show resolved Hide resolved

// Ensure we at least have 1 line
setInputHeight(Math.max(newHeight, minHeight));
});
}
}

Expand All @@ -160,22 +209,30 @@ const EditableTypography: VibeComponent<EditableTypographyProps, HTMLElement> =
if (!typographyRef.current) {
return;
}

const { width } = typographyRef.current.getBoundingClientRect();
setInputWidth(width);
}, [inputValue, isEditing]);

return (
<div
ref={mergedRef}
id={id}
aria-label={ariaLabel}
data-testid={dataTestId}
className={cx(styles.editableTypography, className)}
role={isEditing ? null : "button"}
onClick={onTypographyClick}
onKeyDown={toggleKeyboardEditMode}
>
{isEditing && (
function getEditableElement() {
gabriel-amram marked this conversation as resolved.
Show resolved Hide resolved
if (multiline) {
return (
<textarea
ref={inputRef}
className={cx(styles.textarea, typographyClassName)}
value={inputValue}
onChange={handleChange}
onKeyDown={handleKeyDown}
onBlur={handleBlur}
aria-label={ariaLabel}
placeholder={placeholder}
style={{ width: inputWidth, height: inputHeight }}
role="input"
gabriel-amram marked this conversation as resolved.
Show resolved Hide resolved
rows={1}
gabriel-amram marked this conversation as resolved.
Show resolved Hide resolved
/>
);
} else {
return (
<input
ref={inputRef}
className={cx(styles.input, typographyClassName)}
Expand All @@ -188,7 +245,22 @@ const EditableTypography: VibeComponent<EditableTypographyProps, HTMLElement> =
style={{ width: inputWidth }}
role="input"
/>
)}
);
}
}

return (
<div
talkor marked this conversation as resolved.
Show resolved Hide resolved
ref={mergedRef}
id={id}
aria-label={ariaLabel}
data-testid={dataTestId}
className={cx(styles.editableTypography, className)}
role={isEditing ? null : "button"}
gabriel-amram marked this conversation as resolved.
Show resolved Hide resolved
onClick={onTypographyClick}
onKeyDown={toggleKeyboardEditMode}
>
{isEditing && getEditableElement()}
<TypographyComponent
ref={typographyRef}
aria-hidden={isEditing}
Expand All @@ -201,8 +273,16 @@ const EditableTypography: VibeComponent<EditableTypographyProps, HTMLElement> =
tooltipProps={tooltipProps}
weight={weight}
type={type}
ellipsis={!multiline}
>
{inputValue || placeholder}
{(multiline
? inputValue.split("\n").map((line, index) => (
<React.Fragment key={index}>
{line}
{index < inputValue.split("\n").length - 1 && <br />}
</React.Fragment>
))
gabriel-amram marked this conversation as resolved.
Show resolved Hide resolved
: inputValue) || placeholder}
</TypographyComponent>
</div>
);
Expand Down
Loading