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.

Sign up for GitHub

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

fix up convo title links #6434

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from
+91 −234
Draft

fix up convo title links #6434

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
348ad77
fix up convo title links
rbren Jan 23, 2025
8fc6176
fix cursor pointer
rbren Jan 23, 2025
e941830
remove enter logic
rbren Jan 23, 2025
e39501e
handle enter again
rbren Jan 23, 2025
b21e9f8
rm navlink
rbren Jan 23, 2025
d916df6
delint
rbren Jan 23, 2025
26ad4ec
fix active status
rbren Jan 23, 2025
34876bf
delint
rbren Jan 23, 2025
9a63dfe
delint more
rbren Jan 23, 2025
7c7d5cc
fix tests
rbren Jan 23, 2025
8f92ba8
delint
rbren Jan 23, 2025
f884c2b
fix nav
rbren Jan 23, 2025
474faa4
Merge branch 'main' into rb/convo-titles
rbren Jan 24, 2025
49c92e9
delint
rbren Jan 24, 2025
10056d1
fix test
openhands-agent Jan 24, 2025
7e14183
change test name
rbren Jan 24, 2025
baf0bfc
use form for titles
openhands-agent Jan 24, 2025
7af3f51
Revert "use form for titles"
rbren Jan 31, 2025
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
119 changes: 33 additions & 86 deletions frontend/__tests__/components/features/conversation-panel/conversation-card.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { ConversationCard } from "#/components/features/conversation-panel/conve
import { clickOnEditButton } from "./utils";

describe("ConversationCard", () => {
const onClick = vi.fn();
const onDelete = vi.fn();
const onChangeTitle = vi.fn();
const onDownloadWorkspace = vi.fn();
Expand All @@ -18,6 +17,7 @@ describe("ConversationCard", () => {
it("should render the conversation card", () => {
render(
<ConversationCard
conversationID="deadbeef"
onDelete={onDelete}
onChangeTitle={onChangeTitle}
isActive
Expand All @@ -30,14 +30,14 @@ describe("ConversationCard", () => {

const card = screen.getByTestId("conversation-card");
const title = within(card).getByTestId("conversation-card-title");

expect(title).toHaveValue("Conversation 1");
expect(title).toHaveTextContent("Conversation 1");
within(card).getByText(expectedDate);
});

it("should render the selectedRepository if available", () => {
const { rerender } = render(
<ConversationCard
conversationID="deadbeef"
onDelete={onDelete}
onChangeTitle={onChangeTitle}
isActive
Expand All @@ -53,6 +53,7 @@ describe("ConversationCard", () => {

rerender(
<ConversationCard
conversationID="deadbeef"
onDelete={onDelete}
onChangeTitle={onChangeTitle}
isActive
Expand All @@ -69,6 +70,7 @@ describe("ConversationCard", () => {
const user = userEvent.setup();
render(
<ConversationCard
conversationID="deadbeef"
onDelete={onDelete}
onChangeTitle={onChangeTitle}
isActive
Expand All @@ -94,6 +96,7 @@ describe("ConversationCard", () => {
const user = userEvent.setup();
render(
<ConversationCard
conversationID="deadbeef"
onDelete={onDelete}
isActive
onChangeTitle={onChangeTitle}
Expand All @@ -114,31 +117,11 @@ describe("ConversationCard", () => {
expect(onDelete).toHaveBeenCalled();
});

test("clicking the selectedRepository should not trigger the onClick handler", async () => {
const user = userEvent.setup();
render(
<ConversationCard
onDelete={onDelete}
isActive
onChangeTitle={onChangeTitle}
title="Conversation 1"
selectedRepository="org/selectedRepository"
lastUpdatedAt="2021-10-01T12:00:00Z"
/>,
);

const selectedRepository = screen.getByTestId(
"conversation-card-selected-repository",
);
await user.click(selectedRepository);

expect(onClick).not.toHaveBeenCalled();
});

test("conversation title should call onChangeTitle when changed and blurred", async () => {
test("conversation title should call onChangeTitle when changed and enter is hit", async () => {
const user = userEvent.setup();
render(
<ConversationCard
conversationID="deadbeef"
onDelete={onDelete}
isActive
title="Conversation 1"
Expand All @@ -148,29 +131,33 @@ describe("ConversationCard", () => {
/>,
);

const title = screen.getByTestId("conversation-card-title");
expect(title).toBeDisabled();
let title = screen.getByTestId("conversation-card-title");
expect(title).toHaveTextContent("Conversation 1");

await clickOnEditButton(user);
const titleInput = screen.getByTestId("conversation-card-title-input");

expect(title).toBeEnabled();
expect(titleInput).toBeEnabled();
expect(screen.queryByTestId("context-menu")).not.toBeInTheDocument();
// expect to be focused
expect(document.activeElement).toBe(title);
expect(document.activeElement).toBe(titleInput);

await user.clear(title);
await user.type(title, "New Conversation Name ");
await user.tab();
await user.clear(titleInput);
await user.type(titleInput, "New Conversation Name ");
// hit enter key
const enterEvent = new KeyboardEvent('keyup', { key: 'Enter' });
titleInput.dispatchEvent(enterEvent);

expect(onChangeTitle).toHaveBeenCalledWith("New Conversation Name");
expect(title).toHaveValue("New Conversation Name");
expect(title).toBeDisabled();
title = screen.getByTestId("conversation-card-title");
expect(title).toHaveTextContent("New Conversation Name");
});

it("should reset title and not call onChangeTitle when the title is empty", async () => {
const user = userEvent.setup();
render(
<ConversationCard
conversationID="deadbeef"
onDelete={onDelete}
isActive
onChangeTitle={onChangeTitle}
Expand All @@ -182,63 +169,21 @@ describe("ConversationCard", () => {

await clickOnEditButton(user);

const title = screen.getByTestId("conversation-card-title");
const title = screen.getByTestId("conversation-card-title-input");

await user.clear(title);
await user.tab();
const enterEvent = new KeyboardEvent('keyup', { key: 'Enter' });
title.dispatchEvent(enterEvent);

expect(onChangeTitle).not.toHaveBeenCalled();
expect(title).toHaveValue("Conversation 1");
});

test("clicking the title should not trigger the onClick handler", async () => {
const user = userEvent.setup();
render(
<ConversationCard
onDelete={onDelete}
isActive
onChangeTitle={onChangeTitle}
title="Conversation 1"
selectedRepository={null}
lastUpdatedAt="2021-10-01T12:00:00Z"
/>,
);

const title = screen.getByTestId("conversation-card-title");
await user.click(title);

expect(onClick).not.toHaveBeenCalled();
});

test("clicking the delete button should not trigger the onClick handler", async () => {
const user = userEvent.setup();
render(
<ConversationCard
onDelete={onDelete}
isActive
onChangeTitle={onChangeTitle}
title="Conversation 1"
selectedRepository={null}
lastUpdatedAt="2021-10-01T12:00:00Z"
/>,
);

const ellipsisButton = screen.getByTestId("ellipsis-button");
await user.click(ellipsisButton);

const menu = screen.getByTestId("context-menu");
const deleteButton = within(menu).getByTestId("delete-button");

await user.click(deleteButton);

expect(onClick).not.toHaveBeenCalled();
});

it("should call onDownloadWorkspace when the download button is clicked", async () => {
const user = userEvent.setup();
render(
<ConversationCard
onClick={onClick}
conversationID="deadbeef"
onDelete={onDelete}
onChangeTitle={onChangeTitle}
onDownloadWorkspace={onDownloadWorkspace}
Expand All @@ -263,7 +208,7 @@ describe("ConversationCard", () => {
const user = userEvent.setup();
const { rerender } = render(
<ConversationCard
onClick={onClick}
conversationID="deadbeef"
onChangeTitle={onChangeTitle}
title="Conversation 1"
selectedRepository={null}
Expand All @@ -282,7 +227,7 @@ describe("ConversationCard", () => {

rerender(
<ConversationCard
onClick={onClick}
conversationID="deadbeef"
onDelete={onDelete}
title="Conversation 1"
selectedRepository={null}
Expand All @@ -299,7 +244,7 @@ describe("ConversationCard", () => {
it("should not render the ellipsis button if there are no actions", () => {
const { rerender } = render(
<ConversationCard
onClick={onClick}
conversationID="deadbeef"
onDelete={onDelete}
onChangeTitle={onChangeTitle}
onDownloadWorkspace={onDownloadWorkspace}
Expand All @@ -313,7 +258,7 @@ describe("ConversationCard", () => {

rerender(
<ConversationCard
onClick={onClick}
conversationID="deadbeef"
onDelete={onDelete}
onDownloadWorkspace={onDownloadWorkspace}
title="Conversation 1"
Expand All @@ -326,7 +271,7 @@ describe("ConversationCard", () => {

rerender(
<ConversationCard
onClick={onClick}
conversationID="deadbeef"
onDownloadWorkspace={onDownloadWorkspace}
title="Conversation 1"
selectedRepository={null}
Expand All @@ -338,7 +283,7 @@ describe("ConversationCard", () => {

rerender(
<ConversationCard
onClick={onClick}
conversationID="deadbeef"
title="Conversation 1"
selectedRepository={null}
lastUpdatedAt="2021-10-01T12:00:00Z"
Expand All @@ -352,6 +297,7 @@ describe("ConversationCard", () => {
it("should render the 'STOPPED' indicator by default", () => {
render(
<ConversationCard
conversationID="deadbeef"
onDelete={onDelete}
isActive
onChangeTitle={onChangeTitle}
Expand All @@ -367,6 +313,7 @@ describe("ConversationCard", () => {
it("should render the other indicators when provided", () => {
render(
<ConversationCard
conversationID="deadbeef"
onDelete={onDelete}
isActive
onChangeTitle={onChangeTitle}
Expand Down
Loading