Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/client/packages/design-system/ads/src/List/List.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ function ListItem(props: ListItemProps) {

return (
<StyledListItem
className={clsx(ListItemClassName, props.className)}
className={clsx(ListItemClassName, props.className, "t--ide-list-item")}
data-disabled={props.isDisabled || false}
data-isblockdescription={isBlockDescription}
data-rightcontrolvisibility={rightControlVisibility}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
import React, { useMemo } from "react";
import React, { useEffect, useMemo, useRef, useState } from "react";

import { Spinner, Tooltip } from "../..";
import { Spinner } from "../../Spinner";
import { Tooltip, type TooltipProps } from "../../Tooltip";
import { useEditableText } from "../../__hooks__";

import * as Styled from "./EditableEntityName.styles";

import type { EditableEntityNameProps } from "./EditableEntityName.types";
import clsx from "clsx";

export const isEllipsisActive = (element: HTMLElement | null) => {
return element && element.clientWidth < element.scrollWidth;
};

export const EditableEntityName = (props: EditableEntityNameProps) => {
const {
Expand All @@ -16,13 +22,17 @@ export const EditableEntityName = (props: EditableEntityNameProps) => {
isFixedWidth,
isLoading,
name,
normalizeName = false,
onExitEditing,
onNameSave,
showEllipsis = false,
size = "small",
validateName,
} = props;

const inEditMode = canEdit ? isEditing : false;
const [showTooltip, setShowTooltip] = useState(false);
const longNameRef = useRef<HTMLDivElement | null>(null);

const [
inputRef,
Expand All @@ -36,6 +46,7 @@ export const EditableEntityName = (props: EditableEntityNameProps) => {
onExitEditing,
validateName,
onNameSave,
normalizeName,
);

// When in loading state, start icon becomes the loading icon
Expand All @@ -58,27 +69,59 @@ export const EditableEntityName = (props: EditableEntityNameProps) => {
paddingTop: "4px",
paddingBottom: "4px",
top: "-5px",
placeholder: "Name",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Incorrect use of 'placeholder' in style object.

The 'placeholder' property isn't a valid CSS style property and should be moved to the input attributes.

- placeholder: "Name",

This should be added to the inputProps object instead:

 inputProps = useMemo(
   () => ({
     ["data-testid"]: inputTestId,
     onKeyUp: handleKeyUp,
     onChange: handleTitleChange,
     autoFocus: true,
+    placeholder: "Name",
     style: {
       backgroundColor: "var(--ads-v2-color-bg)",
       paddingTop: "4px",
       paddingBottom: "4px",
       top: "-5px",
     },
   }),
   [handleKeyUp, handleTitleChange, inputTestId],
 );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
placeholder: "Name",
- placeholder: "Name",
+ // Removed placeholder from the style object.
- // ... rest of the code in the style object ...
+ inputProps = useMemo(
+ () => ({
+ ["data-testid"]: inputTestId,
+ onKeyUp: handleKeyUp,
+ onChange: handleTitleChange,
+ autoFocus: true,
+ placeholder: "Name",
+ style: {
+ backgroundColor: "var(--ads-v2-color-bg)",
+ paddingTop: "4px",
+ paddingBottom: "4px",
+ top: "-5px",
+ },
+ }),
+ [handleKeyUp, handleTitleChange, inputTestId],
+ );

},
}),
[handleKeyUp, handleTitleChange, inputTestId],
);

useEffect(
function handleShowTooltipOnEllipsis() {
if (showEllipsis) {
setShowTooltip(!!isEllipsisActive(longNameRef.current));
}
},
[editableName, showEllipsis],
);

// Tooltip can either show the validation error or the name incase of long names
const tooltipProps: TooltipProps = useMemo(
() =>
validationError
? {
key: "validation-error",
content: validationError,
placement: "bottom",
visible: true,
isDisabled: false,
mouseEnterDelay: 0,
showArrow: true,
}
: {
key: "entity-name",
content: name,
placement: "topLeft",
isDisabled: !showTooltip,
mouseEnterDelay: 1,
showArrow: false,
},
[name, showTooltip, validationError],
);

return (
<Styled.Root data-size={size}>
{startIcon}
<Tooltip
content={validationError}
placement="bottom"
visible={Boolean(validationError)}
>
<Tooltip {...tooltipProps}>
<Styled.Text
aria-invalid={Boolean(validationError)}
className={clsx("t--entity-name", { editing: inEditMode })}
data-isediting={inEditMode}
data-isfixedwidth={isFixedWidth}
inputProps={inputProps}
inputRef={inputRef}
isEditable={inEditMode}
kind={size === "small" ? "body-s" : "body-m"}
ref={showEllipsis ? longNameRef : null}
>
{editableName}
</Styled.Text>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,8 @@ export interface EditableEntityNameProps {
onNameSave: (name: string) => void;
/** Function to validate the name. */
validateName: (name: string) => string | null;
/** Whether a name should be normalized on renaming */
normalizeName?: boolean;
/** Used for showing ellipsis for longer names */
showEllipsis?: boolean;
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@ import {
import * as Styled from "./EntityContextMenu.styles";

interface Props {
dataTestid?: string;
dataTestId?: string;
children?: React.ReactNode[] | React.ReactNode;
tooltipContent?: React.ReactNode;
}

export const EntityContextMenu = (props: Props) => {
const {
children,
dataTestid = DEFAULT_DATA_TEST_ID,
dataTestId = DEFAULT_DATA_TEST_ID,
tooltipContent = DEFAULT_TOOLTIP_CONTENT,
} = props;

Expand All @@ -40,7 +40,7 @@ export const EntityContextMenu = (props: Props) => {
>
<Button
className={EntityClassNames.CONTEXT_MENU}
data-testid={dataTestid}
data-testid={dataTestId}
isIconButton
kind="tertiary"
startIcon="more-2-fill"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ const EntityGroup = <T,>({
group.renderList(item)
) : (
<ListItem
dataTestId={`entity-group-item-${(item as ListItemProps)?.title}`}
key={(item as ListItemProps)?.title || `item-${index}`}
{...(item as ListItemProps)}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export const EntityItem = (props: EntityItemProps) => {
canEdit,
isEditing,
isLoading,
normalizeName = false,
onEditComplete,
onNameSave,
validateName,
Expand All @@ -34,8 +35,10 @@ export const EntityItem = (props: EntityItemProps) => {
isFixedWidth
isLoading={isLoading}
name={props.title}
normalizeName={normalizeName}
onExitEditing={onEditComplete}
onNameSave={onNameSave}
showEllipsis
size="medium"
validateName={validateName}
/>
Expand All @@ -44,6 +47,7 @@ export const EntityItem = (props: EntityItemProps) => {
canEdit,
isEditing,
isLoading,
normalizeName,
onEditComplete,
onNameSave,
props.title,
Expand All @@ -65,7 +69,7 @@ export const EntityItem = (props: EntityItemProps) => {
{...rest}
className={clx("t--entity-item", props.className)}
customTitleComponent={customTitle}
data-testid={`t--entity-item-${props.title}`}
dataTestId={`t--entity-item-${props.title}`}
id={"entity-" + props.id}
onDoubleClick={doubleClickOverride}
rightControl={rightControl}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,7 @@ export interface EntityItemProps
onNameSave: (newName: string) => void;
// Provide a function validate the new name
validateName: (newName: string) => string | null;
// Whether entity name should be normalized on renaming
normalizeName?: boolean;
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,50 +28,47 @@ const nameEditorConfig = {
validateName: () => null,
};

const names = {
"1": "Parent 1",
"1.1": "Child 1.1",
"1.1.1": "Child 1.1.1",
"1.1.2": "Child 1.1.2",
"1.2": "Child 1.2",
"2": "Parent 2",
};

const Tree: EntityListTreeProps["items"] = [
{
id: "1",
isExpanded: true,
isSelected: false,
name: "Parent 1",
children: [
{
id: "1.1",
isExpanded: false,
isSelected: true,
name: "Child 1.1",
children: [
{
id: "1.1.1",
isExpanded: false,
isSelected: false,
name: "Child 1.1.1",
},
{
id: "1.1.2",
isDisabled: true,
isExpanded: false,
isSelected: false,
name: "Child 1.1.2",
},
],
},
{
id: "1.2",
isExpanded: false,
isSelected: false,
name: "Child 1.2",
},
],
},
{
id: "2",
isExpanded: false,
isSelected: false,
name: "Parent 2",
},
];

Expand Down Expand Up @@ -111,7 +108,7 @@ const EntityItemComponent = (props: { item: EntityListTreeItem }) => {
onClick={noop}
onDoubleClick={() => onItemEdit(item.id)}
startIcon={<Icon name="apps-line" />}
title={names[item.id as keyof typeof names] || item.id}
title={item.name}
/>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,8 @@ import type {

const mockOnItemExpand = jest.fn();

const name = {
"1": "Parent 1",
"1.1": "Child 1.1",
"1.1.1": "Child 1.1.1",
"1.1.2": "Child 1.1.2",
"1.2": "Child 1.2",
"2": "No Children Parent",
"1-1": "Child",
};

const ItemComponent = ({ item }: { item: EntityListTreeItem }) => {
return <div>{name[item.id as keyof typeof name] || item.id}</div>;
return <div>{item.name}</div>;
};

const defaultProps: EntityListTreeProps = {
Expand All @@ -30,12 +20,14 @@ const defaultProps: EntityListTreeProps = {
isExpanded: false,
isSelected: false,
isDisabled: false,
name: "Parent 1",
children: [
{
id: "1-1",
isExpanded: false,
isSelected: false,
isDisabled: false,
name: "Child 1.1",
children: [],
},
],
Expand All @@ -52,7 +44,7 @@ describe("EntityListTree", () => {

it("calls onItemExpand when expand icon is clicked", () => {
render(<EntityListTree {...defaultProps} />);
const expandIcon = screen.getByTestId("t--entity-item-expand-icon");
const expandIcon = screen.getByTestId("t--entity-collapse-toggle");

fireEvent.click(expandIcon);
expect(mockOnItemExpand).toHaveBeenCalledWith("1");
Expand All @@ -67,13 +59,14 @@ describe("EntityListTree", () => {
isExpanded: false,
isSelected: false,
isDisabled: false,
name: "No Children Parent",
children: [],
},
],
};

render(<EntityListTree {...props} />);
const expandIcon = screen.queryByTestId("t--entity-item-expand-icon");
const expandIcon = screen.queryByTestId("t--entity-collapse-toggle");

expect(
screen.getByRole("treeitem", { name: "No Children Parent" }),
Expand All @@ -90,12 +83,14 @@ describe("EntityListTree", () => {
isExpanded: true,
isSelected: false,
isDisabled: false,
name: "Parent 1",
children: [
{
id: "1-1",
isExpanded: false,
isSelected: false,
isDisabled: false,
name: "Child 1.1",
children: [],
},
],
Expand All @@ -105,12 +100,14 @@ describe("EntityListTree", () => {

render(<EntityListTree {...props} />);

expect(screen.getByRole("treeitem", { name: "Child" })).toBeInTheDocument();
expect(
screen.getByRole("treeitem", { name: "Child 1.1" }),
).toBeInTheDocument();
});

it("does not render nested EntityListTree when item is not expanded", () => {
render(<EntityListTree {...defaultProps} />);

expect(screen.queryByRole("treeitem", { name: "Child" })).toBeNull();
expect(screen.queryByRole("treeitem", { name: "Child 1.1" })).toBeNull();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,11 @@ export function EntityListTree(props: EntityListTreeProps) {
>
{item.children && item.children.length ? (
<CollapseWrapper
data-icon={
Comment thread
hetunandu marked this conversation as resolved.
item.isExpanded ? "arrow-down-s-line" : "arrow-right-s-line"
}
data-itemid={item.id}
data-testid="t--entity-item-expand-icon"
data-testid="t--entity-collapse-toggle"
onClick={handleOnExpandClick}
>
<Icon
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ export interface EntityListTreeItem {
isSelected: boolean;
isDisabled?: boolean;
id: string;
name: string;
}

export interface EntityListTreeProps {
Expand Down
Loading