TheCyberHUB icon indicating copy to clipboard operation
TheCyberHUB copied to clipboard

feat: notes | Multiple Categories deletion feature

Open Jazzxe opened this issue 9 months ago • 25 comments

#561

Fixes Issue

[FEATURE] Notes App, Manage Selecting Multiple Notes. #561

Changes proposed

Added the checkbox next to notes that when checked allocates the state to selected, triggers the button delete all to appear. Deletion happens onClick and after confirmation in a popup window.

Screenshots

Screenshot 2024-05-01 003132 Screenshot 2024-05-01 003140 Screenshot 2024-05-01 003155 Screenshot 2024-05-01 003414

Note to reviewers

Code of Conduct

  • [ ✓] By submitting this pull request, I confirm I've read and complied with the CoC 🖖

Check List (Check all the applicable boxes)

  • [✓ ] My code follows the code style of this project.
  • [ ✓] My change requires changes to the documentation.
  • [ ✓] I have updated the documentation accordingly.
  • [✓ ] All new and existing tests passed.
  • [✓ ] This PR does not contain plagiarized content.
  • [ ✓] The title of my pull request is a short description of the requested changes.

You can also join our Discord community. Feel free to check out other cool repositories of the Thecyberworld. Join the Thecyberworld GitHub Organisation by raising an issue (you will be sent an invitation).

Jazzxe avatar Apr 30 '24 23:04 Jazzxe

is checkbox will be default?

we can add a option of edit on top of the screen, then maybe we can see those opttions like checkbox in all notes

@ArkadiK94 what to do you think

kabir0x23 avatar May 01 '24 07:05 kabir0x23

@kabir0x23 maybe on holding the ctrl button on the keyboard, the user could pick many notes, and in this time will appear the "delete notes" button 🤔 without checkboxs

But your idea is easier to implement, in my opinion, so we can go with it.

ArkadiK94 avatar May 01 '24 08:05 ArkadiK94

@kabir0x23 yes at the moment the checkbox appears by default, your approach makes total sense but as I implemented the delete all button to appear only after selection, adding and edit button then selecting the element in order to see the delete all will be a bit confusing to users I think.

However @ArkadiK94 in my opinion , it is unusual to use "crtl + lmb" in order to select multiple things on a web application, I think users wont think about it in order to use it as it is not a built habit.

How about redesigning for it to be less visually space-occupying this way.

Screenshot 2024-05-01 143730

Jazzxe avatar May 01 '24 13:05 Jazzxe

@Jazzxe Instead of edit button, you can at the bottom add toggle "on | off" button. When "on" you can show immediately the "delete notes" even when the user didn't select any note. With appropriate label.

@kabir0x23 what do you think?

ArkadiK94 avatar May 01 '24 14:05 ArkadiK94

@Jazzxe Instead of edit button, you can at the bottom add toggle "on | off" button. When "on" you can show immediately the "delete notes" even when the user didn't select any note. With appropriate label.

@kabir0x23 what do you think?

nice idea, we can ready any simple base for now

later we can set a new value in noites, isTrashed: true

then we can show all of the deelted into trash route

kabir0x23 avatar May 02 '24 14:05 kabir0x23

@Jazzxe you committed the deletion of file_upload file to the git, needs to add it back. your antivirus deleted it but you should not commit and stage it. Can you add it back or do you need help with it?

ArkadiK94 avatar May 02 '24 18:05 ArkadiK94

@ArkadiK94 okay I'll work on the proposed idea . However, I know how to add only If I open another pull request , if you have any better solution I would want some guidance. Also about the deleted file, can you explain what is it exactly, from my basic understanding I can see it is a backdoor threat which is obviously something I do not want.

Jazzxe avatar May 02 '24 23:05 Jazzxe

@Jazzxe Yes, you don't want or need it to have on your local computer for this reason the antivirus deleted it. However, we use it in the website so next time you should not commit it, you need to commit only your changes. I will fix this for now, I only add this file back and commit the change again.

ArkadiK94 avatar May 03 '24 04:05 ArkadiK94

@Jazzxe the file contains malicious data templates, it's only templates not real malicious data, it's like dummy data,

that's why antivirus removed it.

ArkadiK94 avatar May 03 '24 04:05 ArkadiK94

@Jazzxe I restored the file :)

ArkadiK94 avatar May 03 '24 16:05 ArkadiK94

@ArkadiK94 @kabir0x23 About the toggle button what do you think its label should be and do you prefer the square or round checkbox design ?

Jazzxe avatar May 03 '24 17:05 Jazzxe

@Jazzxe In my opinion, "multiselect" and round

ArkadiK94 avatar May 03 '24 19:05 ArkadiK94

@Jazzxe I will change it to draft tell us when it will be ready for review

ArkadiK94 avatar May 08 '24 20:05 ArkadiK94

I was busy last week didn't have much time to work on this, I added the multiselect in the bottom of the list as you suggested, it looks like this in thoggled status , untoggled "Delete All" , and checkboxes are not visible: Screenshot 2024-05-09 030022

Jazzxe avatar May 09 '24 02:05 Jazzxe

@Jazzxe "Multi Select" and is it only a button? What do you think about making it a switch button? Screenshot_20240509_081938_Firefox.jpg

ArkadiK94 avatar May 09 '24 05:05 ArkadiK94

I don't know it that would be appropriate for this feature. How about only the color of the label and icon changes depends on status ? meaning the icon and "multiselect" will render green / red when its toggled / toggled off ?

Jazzxe avatar May 09 '24 22:05 Jazzxe

@Jazzxe do you think it is enough indication for the user to understand that this is turned on or off?

ArkadiK94 avatar May 10 '24 08:05 ArkadiK94

@Jazzxe do you think it is enough indication for the user to understand that this is turned on or off?

we can keep it with funcnality and with any simple way, i will work on design later

kabir0x23 avatar May 10 '24 08:05 kabir0x23

@Jazzxe Hmm, is it confusing where are the categories and where are the notes? because you implemented the deletion for categories and not notes...

https://github.com/thecyberworld/TheCyberHUB/assets/76536506/b36f100a-c99b-4e34-ae32-9a0a82c4d6fc

ArkadiK94 avatar May 11 '24 15:05 ArkadiK94

@Jazzxe it is good on one hand because we need such functionality for categories but also for notes...

ArkadiK94 avatar May 11 '24 15:05 ArkadiK94

@Jazzxe better to make it clickable when we click in every place on the category name.

https://github.com/thecyberworld/TheCyberHUB/assets/76536506/ec7cf03c-0319-4ddb-92fc-9b8cd76b468d

ArkadiK94 avatar May 11 '24 15:05 ArkadiK94

@Jazzxe When I select category click on "MultiSelect" Then click it again. I don't see that the categories selected but when I click on delete they are deleted.

https://github.com/thecyberworld/TheCyberHUB/assets/76536506/8f0d6e0f-876c-4749-b619-f08b6aa665a8

ArkadiK94 avatar May 11 '24 15:05 ArkadiK94

I think I got confused yes, because it says "Notes" on the left side of the table, but now I understand that you call those categories which have notes. And even in the first screenshots I provided it was clear I was working on the left side. So do you want to keep the feature for the "categories" and add it also to the "notes" ? Screenshot 2024-05-11 164659

Jazzxe avatar May 11 '24 15:05 Jazzxe

I think I got confused yes, because it says "Notes" on the left side of the table, but now I understand that you call those categories which have notes. And even in the first screenshots I provided it was clear I was working on the left side. So do you want to keep the feature for the "categories" and add it also to the "notes" ?

Yes, in this pr, it will be only for categories, in next pr, you can do for notes as well.

If you think we should change the title or add another one to make the sections more clear I would be happy to hear.

ArkadiK94 avatar May 11 '24 16:05 ArkadiK94

@Jazzxe I see that you resolved some comments. How is it going?

ArkadiK94 avatar May 15 '24 10:05 ArkadiK94

@Jazzxe any update?

ArkadiK94 avatar May 18 '24 15:05 ArkadiK94

see pr #858

ArkadiK94 avatar Jun 11 '24 21:06 ArkadiK94