Tue, Jun 19
We are pretty close now, just some final polishing 👍
Sun, Jun 10
- Remove redundent comments
- Improve variable naming, code readability
Sat, Jun 9
ThumbnailProvider worked nicely. I was initially put off by the complicated use in ThumbnailView which is why I didn't try it to begin with.
- Clean up KFileItemList code
- Change to using ThumbnailProvider to generate drag pix
- Fix stuck cursor after dragging an image
Fri, Jun 8
Mon, Jun 4
Sun, Jun 3
Thanks for the update ;)
I kept everything in DocumentView because I wanted to support dragging a video.
- Don't drag if document failed to load
- Update to use MimeTypeUtils::selectionMimeData
- Respect mimimum drag distance
- Use DragPixmapGenerator
Sat, Jun 2
@rkflx Thanks for reviewing and accepting the patch! This is my second patch and your feedback and your advices are encouraging me to keep contributing :-).
Excellent. I tested everything again, and the issue with the folders not appearing first anymore is now solved.
I made the changes you requested in your inline comments.
I updated the summary. Is it a suitable commit message?
Ah, one more thing: You might want to update the summary a bit to reflect the current state of the patch, as it will become the commit message. Simply click on Edit Revision on the top.
I created a separate Diff for the bug fix at D13289.
Great, LGTM now except for two minor comments.
Rebase on D13252
Thanks for your detailed feedback! I have made the changes you have requested in your inline comments.
I have also added the ability to sort folders first but I will create a separate revision for that.
Thanks, LGTM now :)
Fri, Jun 1
- Remove unnecessary QObject::
- Call QGraphicsWidget instead of QGraphicsItem in drag events
Thanks again for the patch, works great for folders with only images in them and even title/summary/test plan are looking excellent.
Thu, May 31
Wed, May 30
I wonder whether it would be feasible to ship something like kipi-plugins.desktop via upstream R480 Kipi Plugins? (Just an idea in case someone wants to investigate that, I don't have time to work in it right now.)
@faridb Thanks for your patches, I'll have a look on this one in the next days.
Fixed the coding style issue
Thanks, this seems like a much cleaner and more appropriate approach to me. Just one more minor style nitpick below, then I'm satisfied!
Made the changes to use mInstallPluginAction instead of the action's text.
It also solved the bug I was having with the Plugins menu.
That might be more reliable and semantically correct, so let's try it out and see if we like it!
Thanks! One more thought: instead of comparing the action's text, could we compare the action itself, to see if it's mInstallPluginAction?
I think you forgot to specify reviewers, so I added them for you.