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

#391 Chat Ordering based upon the order of interaction #445

Merged
merged 8 commits into from
Jul 22, 2024

Conversation

arjxn-py
Copy link
Contributor

@arjxn-py arjxn-py commented Jul 8, 2024

This PR modifies the LeftSidebar component to display chats in reverse order, showing the newest chats first instead of the oldest.
Should fix #391

@pmeier
Copy link
Member

pmeier commented Jul 9, 2024

@blakerosenthal Could you check if this change is indeed enough?

@blakerosenthal
Copy link
Contributor

This doesn't quite work, unfortunately. self.chats comes indirectly from database.py:get_chats which doesn't guarantee order. There's no timestamp on orm.Chat either otherwise we could sort in get_chats.

Option 1: add a (created_at) timestamp to Chats and then add an order_by to the table.
Option 2: dig into the messages and pull out either the first or last message timestamp. I kind of like this option because then the UI could show chats by how recently they were used, not when they were created. This could be accomplished with a last_updated field on the chat table as well.

image

@pmeier
Copy link
Member

pmeier commented Jul 10, 2024

I'd prefer option 1. by a big margin.

@arjxn-py
Copy link
Contributor Author

arjxn-py commented Jul 10, 2024

Thanks a lot for confirming on this @blakerosenthal, the ui tests you added were also very helpful to test in #446 🚀
To be honest I also like the latter (option 2) more as I feel it contains more utility for the user.

@arjxn-py arjxn-py changed the title #391 Chat Ordering from Newest to Oldest #391 Chat Ordering based upon the order of interaction Jul 10, 2024
@arjxn-py
Copy link
Contributor Author

arjxn-py commented Jul 10, 2024

Hi @blakerosenthal, happy to have your quick look again on this. Thanks 🙂

@pmeier
Copy link
Member

pmeier commented Jul 22, 2024

I'm ok with limiting the scope of this change to only adapt the order when the page (re-)loaded. But ultimately, we want the ordering change to be dynamic, e.g. if I ask a new question in an old chat, that should automatically move to the top of the list rather than this only happening the next time I load the page. @arjxn-py or @blakerosenthal could one of you open a new issue for this?

Copy link
Member

@pmeier pmeier left a comment

Choose a reason for hiding this comment

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

LGTM, thanks Arjun!

@arjxn-py
Copy link
Contributor Author

could one of you open a new issue for this?

Thanks @pmeier, opened #457 for this, feel free to correct me there if needed.

@pmeier pmeier merged commit 7c5728a into Quansight:main Jul 22, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Chat Ordering Newest to Oldest
3 participants