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

refactor(webviews): chat history tab with pagination #7020

Open
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

abeatrix
Copy link
Contributor

@abeatrix abeatrix commented Feb 8, 2025

This PR refactors the chat history tab to match prompts tab with several UX improvements:

  • Add pagination support with configurable items per page
  • Add export and delete all functionality in the header
  • Improve search functionality with a cleaner command palette style interface
  • Remove period-based grouping of chat history items
  • Add confirmation dialog for delete all action
  • Make history items more compact and consistent
  • Add keyboard navigation support
  • Improve empty state messaging and layout
  • Aligns with Prompt Library tab layout
  • Unify e2e test for restoring and deleting chat history

image

image

Fixed bugs:

  • Delete All and Export buttons show up when chat history is empty

Test plan

Updated unit test and e2e test.

  • Verify pagination works correctly with different page sizes
  • Test search functionality filters correctly
  • Confirm export and delete all actions work as expected
  • Check keyboard navigation and accessibility
  • Verify empty state renders appropriately
  • Test history item interactions and delete functionality

After

20 chats per page

image

Before

With 400+ chat in my history the list gets too long for scrolling.

image

This PR refactors the chat history tab to match prompts tab with several UX improvements:

- Add pagination support with configurable items per page
- Add export and delete all functionality in the header
- Improve search functionality with a cleaner command palette style interface
- Remove period-based grouping of chat history items
- Add confirmation dialog for delete all action
- Make history items more compact and consistent
- Add keyboard navigation support
- Improve empty state messaging and layout

## Test plan
- Verify pagination works correctly with different page sizes
- Test search functionality filters correctly
- Confirm export and delete all actions work as expected
- Check keyboard navigation and accessibility
- Verify empty state renders appropriately
- Test history item interactions and delete functionality
- Add strict boolean check for human message text filtering
- Move filtering logic to avoid duplicate filtering operations
- Fix empty state to only show when there are no filtered chats and no search text
- Simplify filtered chats logic by combining conditions
- Remove unnecessary newline in component
@abeatrix abeatrix requested a review from a team February 13, 2025 19:30
@abeatrix abeatrix marked this pull request as ready for review February 13, 2025 19:31
// Ensure the chat view is ready before we start typing
await expect(sidebarTabHistoryButton).toBeVisible()
await expect(sidebarChat.getByRole('button', { name: 'Export' })).toBeVisible()
await expect(sidebarChat.getByRole('button', { name: 'Delete all' })).toBeVisible()
Copy link
Contributor

@julialeex julialeex Feb 26, 2025

Choose a reason for hiding this comment

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

Noice Changes! ^o^
Some suggestions

  • Can we add some space between the export button and delete all button?
  • Can we make the two buttons align with the search bar?
  • The export button didn't work when I tested this locally.
  • The text colors are white for light Color themes, which are hard to see.
  • Can we make the buttons slightly smaller?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@julialeex can i get your re-review again?

Copy link
Contributor

@julialeex julialeex Mar 6, 2025

Choose a reason for hiding this comment

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

@abeatrix The export button still does not work for me

@alexromano
Copy link
Contributor

while we're here, can we make the chat titles editable by the user?

@abeatrix
Copy link
Contributor Author

while we're here, can we make the chat titles editable by the user?

@alexromano yea that was on my mind as well and I think this is a good building block for that 😃 I can do that in a follow up since I don't think that's urgent?

@abeatrix abeatrix requested a review from julialeex March 1, 2025 03:33
@abeatrix abeatrix requested review from a team and removed request for arafatkatze March 5, 2025 17:48
@julialeex
Copy link
Contributor

julialeex commented Mar 6, 2025

Personally I prefer the old UI for the export and delete buttons, it looks cleaner. Curious about other people's thoughts.

If we do change the UI back to the old one, I think we can change the All Prompts button to match this style as well for consistency.

Screenshot 2025-03-06 at 1 42 00 PM

@abeatrix
Copy link
Contributor Author

abeatrix commented Mar 6, 2025

Member

personally don't like those buttons at all 😆 But this is to match the design on the current Prompt list

@julialeex
Copy link
Contributor

Would be nice to add the time period to the conversation tabs. Talked to @abeatrix and it seems like we will add it in another PR.

abeatrix added 2 commits March 6, 2025 14:00
The changes include:

-   Adding an `extensionAPI` prop to the `HistoryTab` component to allow access to the extension's API.
-   Passing the `extensionAPI` to the `HistoryTabWithData` component.
-   Implementing the `downloadChatHistory` function to handle the download of the chat history.
-   Adding a button to trigger the download.
-   Adding a test for the download functionality.
-   Removing the background from the history tab's CSS.
history.chat[chat.id] = chat
await localStorage.setChatHistory(authStatus, history)
this.changeNotifications.next()
// Don't save empty chats
Copy link
Contributor

Choose a reason for hiding this comment

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

awesome 🚀

I do not see any tests for this class. If that's not a problem, please add something 🙏

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.

4 participants