Open-Assistant
Open-Assistant copied to clipboard
add delete function
https://github.com/LAION-AI/Open-Assistant/issues/2582
Add a function for delete, note the inference server already have the delete endpoint so this is only a frontend change.
https://user-images.githubusercontent.com/119783413/235826890-b95a43f5-9511-4692-8f60-610ce43fdfe8.mov
:x: pre-commit failed.
Please run pre-commit run --all-files locally and commit the changes.
Find more information in the repository's CONTRIBUTING.md
Right now if the deleted chat is the current chat, there will be a "failed to open chat" problem (because the chat Container still wants to load the deleted chat). Ideally we may want to redirect to the next chat or if no more chat, navigate back to the dashboard. But it's not implemented in this PR yet.
https://user-images.githubusercontent.com/119783413/235828697-1259a0f1-4826-448a-81bd-b457895ac43c.mov
Thank you. I think it would be good to have this as part of a dropdown with extra buttons (so that the user by default sees only Hide and Edit Title, but can click the ... and see Delete and in future Data Opt Out). Would that be possible?
Maybe additionally we want to let the users confirm the delete, not because they might delete by accident but because the data will be removed permanently and we cannot use it for further training.
:x: pre-commit failed.
Please run pre-commit run --all-files locally and commit the changes.
Find more information in the repository's CONTRIBUTING.md
:x: pre-commit failed.
Please run pre-commit run --all-files locally and commit the changes.
Find more information in the repository's CONTRIBUTING.md
Added a dropdown and a confirmation dialog.
https://user-images.githubusercontent.com/119783413/236356690-5bf8cb21-34dc-4a81-b15c-35f7f4a43ee4.mov
@ruoniw Thank you for preparing this ♥, I had to do some final cross-service related changes, and minimal improvements to the mobile experience.
Blocked by https://github.com/LAION-AI/Open-Assistant/pull/3046
@olliestanley we throw a 404 if chat is not found, instead of error. @notmd had to do some hacks to get this working on mobile, the three dot menu would be shown under the mobile menu.
please review!
@AbdBarho I think we should get this and #3046 (will need your review) merged and onto dev ASAP, then we can test on dev and hopefully get onto prod today
@notmd had to do some hacks to get this working on mobile, the three dot menu would be shown under the mobile menu.
Maybe we can remove the 3 dots for now, and show the delete icon directly. I plan to work on a new design later.
Maybe we can remove the 3 dots for now, and show the delete icon directly. I plan to work on a new design later.
It would be unfortunate to have this on prod with delete icon showing directly tbh. Potentially a lot of unnecessary data loss if users click delete when they really just want to hide
Maybe we can remove the 3 dots for now, and show the delete icon directly. I plan to work on a new design later.
It would be unfortunate to have this on prod with delete icon showing directly tbh. Potentially a lot of unnecessary data loss if users click delete when they really just want to hide
I mean we still have a popup to display warning like "Delete chat wont help us train the model"
@AbdBarho thanks for wrapping this up! Looks good to me.
@notmd had to do some hacks to get this working on mobile, the three dot menu would be shown under the mobile menu.
Have you try a higher zIndex?
Have you try a higher zIndex?
Yeah yeah it works now, I just wanted to inform you that there were some hacks involved.