search

hugolabe / Wike

Wikipedia Reader for the GNOME Desktop
https://hugolabe.github.io/Wike/
GNU General Public License v3.0
228 stars 32 forks source link

Add a remove button for history row #160

Closed aunetx closed 7 months ago

aunetx commented 8 months ago

This fixes #101.

I was heavily inspired by your implementation of bookmark remove buttons, however the implementation is a little bit different because of the way the history needs to allow uri duplicates, etc

It seems to work all right for me, maybe some additional testing would be better though (as I may have not though about all the edge cases).

By the way, sorry for my urge to make pull requests for your app, I just love using it and wanted to give it a little bit back!

hugolabe commented 8 months ago

@aunetx I thank you for your collaborations, but I also ask you to go a little slower.

When submitting a PR, especially when it affects the operation of the application, it is important to take the time to test it well and think about all the use cases that could cause problems.

For example, in this case there is a problem when you remove all elements of the same date from the list, since the date header should no longer be displayed. And there could be others...

The time I can dedicate to Wike is unfortunately limited, so I ask you to be patient and wait until I can review and test each PR before sending others.

aunetx commented 8 months ago

@hugolabe hello and sorry for this kind of problem. I know reviewing is painful, as is testing this kind of functionalities; and I admit I went a little bit too fast on this one.

I fixed this bug, I wrongly implemented the date header removal... I won't send any more PR before the other ones are merged, and tell me if there is another problem somewhere else -- although I tried to test all the edge cases I found for the functionalities that were added.