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: ADS Entity Item #38442

Merged
merged 20 commits into from
Jan 6, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
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
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ export const InlineDescriptionWrapper = styled.div`
export const RightControlWrapper = styled.div`
height: 100%;
line-height: normal;
display: flex;
align-items: center;
`;

export const ContentTextWrapper = styled.div`
Expand All @@ -60,6 +62,7 @@ export const ContentTextWrapper = styled.div`
overflow: hidden;
flex: 1;
min-width: 0;
flex-shrink: 0;

& .${ListItemTextOverflowClassName} {
overflow: hidden;
Expand Down Expand Up @@ -115,7 +118,6 @@ export const StyledListItem = styled.div<{
padding-left: var(--ads-v2-spaces-3);

${({ size }) => Sizes[size]}

&[data-isblockdescription="true"] {
height: 54px;
}
Expand All @@ -128,6 +130,7 @@ export const StyledListItem = styled.div<{
${RightControlWrapper} {
display: none;
}

&:hover ${RightControlWrapper} {
display: block;
}
Expand All @@ -138,6 +141,7 @@ export const StyledListItem = styled.div<{
}

/* disabled style */

&[data-disabled="true"] {
cursor: not-allowed;
opacity: var(--ads-v2-opacity-disabled);
Expand All @@ -153,6 +157,7 @@ export const StyledListItem = styled.div<{
}

/* Focus styles */

&:focus-visible {
outline: var(--ads-v2-border-width-outline) solid
var(--ads-v2-color-outline);
Expand Down
17 changes: 11 additions & 6 deletions app/client/packages/design-system/ads/src/List/List.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ function ListItem(props: ListItemProps) {
} = props;
const isBlockDescription = descriptionType === "block";

const listItemhandleKeyDown = (e: React.KeyboardEvent) => {
const listItemHandleKeyDown = (e: React.KeyboardEvent) => {
if (!props.isDisabled && props.onClick) {
switch (e.key) {
case "Enter":
Expand All @@ -108,20 +108,25 @@ function ListItem(props: ListItemProps) {
}
};

const handleDoubleClick = () => {
if (!props.isDisabled && props.onDoubleClick) {
props.onDoubleClick();
}
};

return (
<StyledListItem
className={clsx(ListItemClassName, props.className)}
data-disabled={props.isDisabled || false}
data-isblockdescription={isBlockDescription}
data-rightcontrolvisibility={rightControlVisibility}
data-selected={props.isSelected}
onClick={handleOnClick}
ankitakinger marked this conversation as resolved.
Show resolved Hide resolved
onKeyDown={listItemHandleKeyDown}
size={size}
tabIndex={props.isDisabled ? -1 : 0}
// tabIndex={props.isDisabled ? -1 : 0}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Restore the tabIndex for keyboard accessibility

Commenting out the tabIndex prevents keyboard navigation to list items, which is a critical accessibility requirement.

-      // tabIndex={props.isDisabled ? -1 : 0}
+      tabIndex={props.isDisabled ? -1 : 0}
📝 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
// tabIndex={props.isDisabled ? -1 : 0}
tabIndex={props.isDisabled ? -1 : 0}

>
<ContentTextWrapper
onClick={handleOnClick}
onKeyDown={listItemhandleKeyDown}
>
<ContentTextWrapper onDoubleClick={handleDoubleClick}>
{startIcon}
{props.customTitleComponent ? (
props.customTitleComponent
Expand Down
24 changes: 19 additions & 5 deletions app/client/packages/design-system/ads/src/List/List.types.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,17 @@ import type { ReactNode } from "react";

export type ListSizes = Extract<Sizes, "md" | "lg">;

export interface ListItemProps {
interface BaseListItemProps {
/** The icon to display before the list item title. */
startIcon?: ReactNode;
/** The control to display at the end. */
rightControl?: ReactNode;
/** */
/** Control the visibility trigger of right control */
rightControlVisibility?: "hover" | "always";
/** callback for when the list item is clicked */
onClick: () => void;
/** callback for when the list item is double-clicked */
onDoubleClick?: () => void;
/** Whether the list item is disabled. */
isDisabled?: boolean;
/** Whether the list item is selected. */
Expand All @@ -20,8 +22,6 @@ export interface ListItemProps {
size?: ListSizes;
/** Whether to show the list item in error state */
hasError?: boolean;
/** The title/label of the list item */
title: string;
/** Description text to be shown alongside the title */
description?: string;
/** `inline` type will show the description beside the title. `block` type will show the description
Expand All @@ -32,10 +32,24 @@ export interface ListItemProps {
className?: string;
/** id for the list item */
id?: string;
}

interface ListItemWithTitle extends BaseListItemProps {
/** The title/label of the list item */
title: string;
customTitleComponent?: never;
hetunandu marked this conversation as resolved.
Show resolved Hide resolved
}

interface ListItemWithCustomTitleComponent extends BaseListItemProps {
title?: never;
/** customTitleComponent for the list item to use input component for name editing */
customTitleComponent?: ReactNode | ReactNode[];
customTitleComponent: ReactNode | ReactNode[];
}

export type ListItemProps =
| ListItemWithTitle
| ListItemWithCustomTitleComponent;

export interface ListProps {
items: ListItemProps[];
className?: string;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/* eslint-disable no-console */
import React from "react";
import type { Meta, StoryObj } from "@storybook/react";

import { EntityItem } from "./EntityItem";
import type { EntityItemProps } from "./EntityItem.types";
import { ExplorerContainer } from "../ExplorerContainer";
import { Flex, Button, Icon, Callout } from "../../..";

const meta: Meta<typeof EntityItem> = {
title: "ADS/Templates/Entity Explorer/Entity Item",
component: EntityItem,
};

export default meta;

const Template = (props: EntityItemProps) => {
const { hasError, isDisabled, isSelected, name, nameEditorConfig } = props;
const [isEditing, setIsEditing] = React.useState(false);

const onEditComplete = () => {
setIsEditing(false);
};
const onNameSave = (name: string) => console.log("Name saved" + name);

const onClick = () => console.log("Add clicked");

const rightControl = (
<Button isIconButton kind="tertiary" startIcon="comment-context-menu" />
);

const startIcon = <Icon name="apps-line" />;

return (
<Flex bg="white" overflow="hidden" width="400px">
<ExplorerContainer borderRight="STANDARD" height="500px" width="255px">
<Flex flexDirection="column" gap="spaces-2" p="spaces-3">
<EntityItem
onDoubleClick={() => {
setIsEditing(true);
}}
{...{
startIcon,
name,
hasError,
isSelected,
isDisabled,
nameEditorConfig: {
...nameEditorConfig,
isEditing,
onEditComplete,
onNameSave,
},
onClick,
rightControl,
}}
/>
<Callout>Double click the name to edit it</Callout>
</Flex>
</ExplorerContainer>
</Flex>
);
};

export const InEditingMode = Template.bind({}) as StoryObj;

InEditingMode.args = {
name: "EntityName",
nameEditorConfig: {
canEdit: true,
validateName: () => null,
},
};

export const RenamingError = Template.bind({}) as StoryObj;

RenamingError.args = {
name: "EntityName",
nameEditorConfig: {
canEdit: true,
validateName: () => "This is a sample error",
},
};

export const SelectedState = Template.bind({}) as StoryObj;

SelectedState.args = {
name: "EntityName",
isSelected: true,
nameEditorConfig: {
canEdit: true,
validateName: () => null,
},
};

export const DisabledState = Template.bind({}) as StoryObj;

DisabledState.args = {
name: "EntityName",
isDisabled: true,
nameEditorConfig: {
canEdit: true,
validateName: () => null,
},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import styled from "styled-components";
import { Text } from "../../..";

export const EntityEditableName = styled(Text)`
height: 32px;
line-height: 32px;
flex: 1;
min-width: 3ch;
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import React, { useMemo } from "react";
import { ListItem, Tooltip } from "../../..";

import type { EntityItemProps } from "./EntityItem.types";
import { EntityEditableName } from "./EntityItem.styled";
import { useNameEditor } from "./useNameEditor";

export const EntityItem = (props: EntityItemProps) => {
const { canEdit, isEditing, onEditComplete, onNameSave, validateName } =
props.nameEditorConfig;

const [
inputRef,
editableName,
validationError,
handleKeyUp,
handleTitleChange,
] = useNameEditor(
canEdit ? isEditing : false,
props.name,
onEditComplete,
validateName,
onNameSave,
);

const inputProps = useMemo(
() => ({
onChange: handleTitleChange,
onKeyUp: handleKeyUp,
style: {
paddingTop: 0,
paddingBottom: 0,
height: "32px",
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not completely ok with adding height separately for the inputs. Maybe we can have a conversation with Momcilo regarding this later. Not a blocker for this PR.

top: 0,
},
}),
[handleKeyUp, handleTitleChange],
);

const customTitle = useMemo(() => {
return (
<Tooltip
content={validationError}
placement="bottomLeft"
showArrow={false}
visible={Boolean(validationError)}
>
<EntityEditableName
inputProps={inputProps}
inputRef={inputRef}
isEditable={isEditing}
kind="body-m"
>
{editableName}
</EntityEditableName>
</Tooltip>
);
}, [editableName, inputProps, inputRef, isEditing, validationError]);

return <ListItem {...props} customTitleComponent={customTitle} />;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import type { ListItemProps } from "../../../List";

export interface EntityItemProps
extends Omit<
ListItemProps,
"title" | "customTitleComponent" | "description" | "descriptionType"
> {
name: string;
/** Control the name editing behaviour */
nameEditorConfig: {
// Set editable based on user permissions
canEdit: boolean;
// State to control the editable state of the input
isEditing: boolean;
// Shows a loading spinner in place of the startIcon
isLoading: boolean;
// Called to request the editing mode to end
onEditComplete: () => void;
// Called to save the new name
onNameSave: (newName: string) => void;
// Provide a function validate the new name
validateName: (newName: string) => string | null;
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { EntityItem } from "./EntityItem";
Loading
Loading