- User Since
- Feb 2 2018, 4:19 AM (28 w, 1 d)
Jul 9 2018
Jul 8 2018
Nice, such a simple fix!
Two things I noticed while testing:
Sorry for the delay!
Jun 24 2018
Jun 23 2018
- resetDragDetection rename
- better way of working with KFileItem/KFileItemList
- lazy initialize thumbnail provider
- check for mDrag closer to its usage
Jun 10 2018
- Remove redundent comments
- Improve variable naming, code readability
Jun 9 2018
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
Jun 8 2018
Jun 3 2018
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
Jun 2 2018
Jun 1 2018
- Remove unnecessary QObject::
- Call QGraphicsWidget instead of QGraphicsItem in drag events
May 27 2018
- Handle drop events in DocumentView
- Fix crash when dragging non-URL mimetypes
- Fix compile error
May 25 2018
I no longer think getting rid of currentChanged is the right way to go about this, so I plan to have another look at it.
May 24 2018
May 23 2018
If I see this correctly, the only time the buttons weren't updated was when the hover did not change, but the selection did. E.g. when Ctrl + clicking, or ⇧ + arrow. Oops, just realised this was cleared up in the Test Plan.
If so, then the patch LGTM.
Thanks for the feedback. I will wait until D13028 is finalised before doing anything. I also think waiting until we can put the mimedata code in a central place is a good idea. I think Document::mimeData could be a good spot?
May 21 2018
- Remove include that I missed
- Use KIO::PreviewJob to generate pixmap thumbnail
- Add (possibly edited) image data to the drag mimedata
May 20 2018
May 19 2018
May 18 2018
May 17 2018
May 16 2018
Patch works as advertised.
All except LibreOffice Writer worked as expected (pasted the bitmap). Writer presumably used the URL since the original image was pasted.
It might be confusing in some situations, but I don't see how we could solve the issue without removing the URL from the mime data, which is a bad idea.
May 15 2018
May 14 2018
I think we should change the current Copy action to copy the bitmap, but only in View mode (if we're in compare mode, then only the selected image). There is no ambiguity with multiple selection here.
I think when viewing a single image, Copy is naturally associated with copying the bitmap (not the file), so one can paste in a browser or image editor. But in Browse, the images are more like files, so copying there should behave more like a file (just like dragging and dropping).
May 13 2018
As far as I understood, the change only affected Herald rules (which we don't have) and forwarding to mailinglists (which we don't do, and which I'd find quite questionable).
As long as nobody of us gets too annoyed with our current way of working, I'd say we simply carry on as usual (but I believe there are also more fine-grained filtering options available using Phab's mail headers, should the need arise to distinguish between subscriber/reviewer notifications and bulk Gwenview mail).
May 9 2018
May 8 2018
May 7 2018
- Simplify patch to prevent duplicate calls
May 3 2018
I'm understanding this a bit better now. QIcon itself doesn't contain light/dark (palette) information. Only when you call QIcon::pixmap does it use the palette to generate either a light or dark icon (in this scenario). The palette used is the same as the QApplication palette, unless a custom palette is set with KIconLoader::setCustomPalette().
- Fix HiDPI by changing how we generate the icon
May 2 2018
May 1 2018
In fact, if this method of forcing light icons (and the corresponding KF5 version bump) is all good, I would prefer not landing this patch but instead convert the HUD to standard widgets (keeping the general HUD framework).
E.g. I would:
- Change back to loadIcon and don't scale
- Bump KF5 version to 5.39 to get setCustomPalette and resetPalette
Apr 29 2018
- Change back to QIcon
Apr 28 2018
Apr 27 2018
Weirdly I'm now getting the cherry pick error when using arc patch. Something about dependent patches seems to confuse Arcanist.
Easy review :)
Works as advertised, and code in apropriate place.
Apr 26 2018
Hadn't considered disabling the collapse behaviour entirely, and agree this is the best decision. The functionality doesn't make sense in this situation.
Well, I've had a play around, and found the following problems:
- If you switch to the Start Page before quitting, the sizes aren't saved.
- There's a pre-existing issue where collapsing the sidebar by dragging the splitter to the left doesn't toggle the sidebar off, and further toggling shows and hides a narrow sliver of the splitter. This patch makes this issue worse by saving the collapsed splitter state to config. I think the best solution to this would be to detect the collapse and 1) toggle the sidebar off, and 2) make sure the collapse splitter sizes aren't saved.
Apr 25 2018
Well the patch LGTM. Only one small quibble.
I suppose I will accept it!