Open-Assistant icon indicating copy to clipboard operation
Open-Assistant copied to clipboard

add delete function

Open ruoniw opened this issue 2 years ago • 15 comments

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

ruoniw avatar May 03 '23 03:05 ruoniw

: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

github-actions[bot] avatar May 03 '23 03:05 github-actions[bot]

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

ruoniw avatar May 03 '23 03:05 ruoniw

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?

olliestanley avatar May 03 '23 07:05 olliestanley

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.

AbdBarho avatar May 03 '23 16:05 AbdBarho

: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

github-actions[bot] avatar May 05 '23 00:05 github-actions[bot]

: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

github-actions[bot] avatar May 05 '23 00:05 github-actions[bot]

Added a dropdown and a confirmation dialog.

https://user-images.githubusercontent.com/119783413/236356690-5bf8cb21-34dc-4a81-b15c-35f7f4a43ee4.mov

ruoniw avatar May 05 '23 00:05 ruoniw

@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 avatar May 05 '23 17:05 AbdBarho

@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

olliestanley avatar May 05 '23 17:05 olliestanley

@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.

notmd avatar May 05 '23 18:05 notmd

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

olliestanley avatar May 05 '23 18:05 olliestanley

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"

notmd avatar May 05 '23 18:05 notmd

@AbdBarho thanks for wrapping this up! Looks good to me.

ruoniw avatar May 05 '23 18:05 ruoniw

@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?

notmd avatar May 05 '23 18:05 notmd

Have you try a higher zIndex?

Yeah yeah it works now, I just wanted to inform you that there were some hacks involved.

AbdBarho avatar May 05 '23 18:05 AbdBarho