Skip to content

feat: add button to delete conversations #55

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

Merged
merged 5 commits into from
May 6, 2025

Conversation

Miyou
Copy link
Collaborator

@Miyou Miyou commented May 6, 2025

Important

Add delete conversation feature with UI and API integration, including confirmation dialog and updated e2e tests.

  • Feature:
    • Add delete button to ConversationList.tsx for each conversation, triggering a confirmation dialog.
    • Implement DeleteConversationConfirmationDialog.tsx for confirming deletion.
    • Integrate deleteConversation method in ApiClient in api.ts to handle API requests for deletion.
  • UI:
    • Add Trash icon to ConversationList.tsx for delete action.
    • Use useState to manage dialog state in ConversationList.tsx.
  • API:
    • Add deleteConversation method to ApiClient in api.ts.
    • Update ApiContext.tsx to include deleteConversation in context.
  • Tests:
    • Update e2e/conversation.spec.ts to use getByPlaceholder for message input.

This description was created by Ellipsis for ec7196a. You can customize this summary. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Caution

Changes requested ❌

Reviewed everything up to 439918e in 2 minutes and 12 seconds. Click for details.
  • Reviewed 209 lines of code in 4 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/components/ConversationList.tsx:13
  • Draft comment:
    File name typo - consider renaming 'DeleteConversationConfrimationDialog' to 'DeleteConversationConfirmationDialog' for clarity.
  • Reason this comment was not posted:
    Marked as duplicate.
2. src/components/ConversationList.tsx:13
  • Draft comment:
    Filename typo: 'DeleteConversationConfrimationDialog' likely intended to be 'DeleteConversationConfirmationDialog'.
  • Reason this comment was not posted:
    Marked as duplicate.
3. src/components/ConversationList.tsx:13
  • Draft comment:
    Typo: The import path './DeleteConversationConfrimationDialog' contains a typographical error ('Confrimation' should be 'Confirmation'). Please fix the filename to ensure consistency and avoid potential issues.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% This is a real typo in a newly added import path. Typos in import paths will cause build failures and runtime errors. However, this would be caught immediately by the build system when trying to import a non-existent file. The rules specifically say not to comment on things that would be obviously caught by the build. The typo is real and could cause issues. Maybe it's worth pointing out to save the developer time debugging build errors? No - the rules are very clear that we should not comment on issues that would be caught by the build system. Import errors are one of the specifically mentioned examples. Delete this comment. While it correctly identifies a typo, import path issues will be caught by the build system and don't need manual review comments.

Workflow ID: wflow_W6zjcq0RZkj6O1rA

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed 1fe5df9 in 31 seconds. Click for details.
  • Reviewed 12 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/components/DeleteConversationConfrimationDialog.tsx:52
  • Draft comment:
    Good fix: Adding a return prevents further code from executing when an error occurs, ensuring proper error handling.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative and does not provide any actionable feedback or suggestions for improvement. It simply states a fact about the code change without offering any guidance or questions for the PR author.
2. src/components/DeleteConversationConfrimationDialog.tsx:52
  • Draft comment:
    Adding a return here prevents the subsequent deletion logic from running if an error occurs. This ensures that after setting the error state and resetting isDeleting, the function exits immediately. Good fix.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative, explaining the behavior of the code without suggesting any changes or asking for confirmation. It doesn't align with the rules for useful comments.
3. src/components/DeleteConversationConfrimationDialog.tsx:1
  • Draft comment:
    The file name 'DeleteConversationConfrimationDialog.tsx' contains a typographical error. The word 'Confrimation' should be spelled as 'Confirmation' to maintain consistency with the component name inside the file.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_uCQUtJ2DiBk8suKj

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed ae711c0 in 28 seconds. Click for details.
  • Reviewed 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/components/ConversationList.tsx:13
  • Draft comment:
    Fixed typo in file name.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative and does not provide any actionable feedback or suggestions for improvement. It simply states that a typo in a file name was fixed, which does not align with the rules provided.
2. src/components/ConversationList.tsx:13
  • Draft comment:
    Corrected spelling of the DeleteConversationConfirmationDialog import path.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative, as it only states that a spelling correction was made to an import path. It does not provide any actionable feedback or suggestions for improvement.
3. src/components/ConversationList.tsx:190
  • Draft comment:
    It appears that 'convState.pendingTool.tooluse.tool' might contain a typographical error ('tooluse'). Consider verifying whether it should be 'toolUse' or another valid property name.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_zfpmcTvi7QlG06QE

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed 86e66e9 in 1 minute and 7 seconds. Click for details.
  • Reviewed 28 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/components/ConversationList.tsx:205
  • Draft comment:
    Good update: replacing with a for the delete action improves accessibility. Including type and aria-label is great. Ensure stopPropagation is maintained to prevent parent onClick triggers.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
2. src/components/ConversationList.tsx:205
  • Draft comment:
    Good improvement replacing the non-interactive with a . The use of 'aria-label' and 'type' enhances accessibility, and stopPropagation prevents accidental parent clicks. Consider ensuring that the button has visible focus styles for keyboard navigation.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_LD688Rqq1it2hLOi

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed ec7196a in 38 seconds. Click for details.
  • Reviewed 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. e2e/conversation.spec.ts:125
  • Draft comment:
    Using getByPlaceholder('Send a message...') assumes the placeholder text remains constant. Ensure it's unlikely to change to prevent flaky tests.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment suggests ensuring that the placeholder text is unlikely to change to prevent flaky tests. This is a request for confirmation or assurance, which violates the rule against asking the PR author to ensure behavior is intended or tested.
2. e2e/conversation.spec.ts:125
  • Draft comment:
    Using getByPlaceholder improves specificity, but ensure the placeholder remains unique to prevent future test breakage.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_yuBaBtfGCQQ0RqHr

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@@ -122,7 +122,7 @@ test.describe('Conversation Flow', () => {
const message = 'Hello. We are testing, just say exactly "Hello world" without anything else.';

// Type a message
await page.getByRole('textbox').fill(message);
await page.getByPlaceholder('Send a message...').fill(message);
Copy link
Member

Choose a reason for hiding this comment

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

Should probably add a test label, will do in follow-up commit.

@ErikBjare ErikBjare merged commit 24751b0 into master May 6, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants