sugarlabs / sugar

Sugar GTK shell
GNU General Public License v3.0
255 stars 241 forks source link

[Feature] Add remove button for a Journal entry #826

Closed Hrishi1999 closed 5 years ago

Hrishi1999 commented 5 years ago

To remove an entry, you need to first select the entry with checkbox and then remove it, or you can open the detail view and delete it. This makes it simple. Similar functionality can be found in Gmail, you can select multiple items and delete, or delete a single item by clicking on the "Trash" button. Added a remove button next to the detail button for each Journal entry item, The feature is demonstrated below: Remove

quozl commented 5 years ago

Thanks. I disagree with the feature as you have implemented it.

In your argument, you have missed the right-click, mouse-over or long-touch menu, which has an erase button.

You have made an erase without confirmation. I disagree with that.

You have also added code to uninstall an activity if a journal object was a bundle file. I disagree with that. Perhaps you added this as part of a fix to https://github.com/sugarlabs/browse-activity/issues/81, but that issue was for deleting the bundle file without uninstalling the activity. Also, you only added it for the delete using your new button, not the other delete methods, which is inconsistent.

The comparison with Gmail is improper. Gmail has a deleted items folder, and the Journal does not. And this is not mail, it is children's work. One accidental touch on a touchscreen and the work is gone. My pet kangaroo ate my homework, teacher! :grinning:

You seem to have devalued journal objects. Perhaps you should be enabling the Save-As feature instead, if you don't want so many things in a journal? I'd like the Save-As feature selectable from My Settings, but that's a different issue.

Also, in future please ensure your commit messages contain all relevant information; your commit message was too brief, and your pull request contained information that could have been in the commit message. See our commit message checklist.

Also, the order of widget creation does not match the order of display, left to right. You fixed this in the append_column calls, but the lines above remain out of order.

Hope that helps.

Hrishi1999 commented 5 years ago

Would keep these points in minds. But is there any scope of improvement? Should I discard it?

quozl commented 5 years ago

I've given my opinion. Let's see what other people say. If you think not enough people have answered your user experience issue, remember that few people watch pull requests in repositories, and you may have more people answer you if you use another place to ask user experience questions.

quozl commented 5 years ago

I'll delete responses that directly attack me by putting words in my mouth. You had much to say of value, but I won't take attacks any more. It was also off-topic for the pull request.