[PATCH] KDesktop drag & drop (BR26456)

[David Faure]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Wednesday 07 May 2003 00:22, Waldo Bastian wrote:
> Hi David,
> 
> First concept patch for fixing the drag&drop positions in kdesktop.
Looks good.

One minor thing: the comment at the top of KIO::pasteDataAsync doesn't apply there.

One nitpick: why tempFile.dataStream()->writeRawBytes() instead of just tempFile.file()->writeBlock()?
(The former just calls the latter, but I don't see the point in creating and using a QDataStream)
Ah I see the current code has this already, they could both be fixed.

One problem with pasteDataAsync: who deletes the tempfile?
This is a general problem, not only for this code... Here we could create
a QObject that takes care of it, connected to the job's result() signal.
(KDE 4 solution: make KTempFile a QObject and add a slot to it, that closes and deletes it).

I note that your patch also adds undo/redo support to the paste operation,
that's great.

> KIO::CopyJob * copyJob = static_cast<KIO::CopyJob *>(job);
pasteDataAsync could return a CopyJob, no?

> It still needs a little work, e.g. when the rename dialog gets triggered and
> the user selects a different filename it should re-emit this aboutToCreate
> signal witht the new name choosen by the user.

Well, CopyJob already emis a signal in such a case: renamed() (line 2259 for me).

> Dragging links and dragging text-fragments to the desktop works ok now.
> 
> Still todo:
> * slotPaste used to store a position as well. That's commented out at the
> moment, not sure if it is still needed actually since the RMB menu doesn't
> seem to contain paste to start with.
Hum. Where did it go? ....
Ah I see. I commented out your "insert the undo and paste actions into
m_actionCollection" since Dirk noticed it gives an invalid write when closing
kdesktop, but I didn't fix the code to find the actions in the right collections.
Fixed now.

> * Not sure if the API is optimal, e.g. at some point we might wish to maintain
> the relative locations if a drag contains multiple items
Yes (although your code already handles this better than the old one, since you
use x+=20, whereas the old code would only care about the first icon and dump
the rest somewhere else :)

Hum. You're using KIO::CopyInfo out of job.cpp, which is fine, but what about
putting that struct into a bigger one, in KonqIconViewWidget? Then you can
add a QPoint for each file, and you don't need the first QPoint arg in the
KonqIconViewWidget signal. This means more processing in slotAboutToCreate
though. The decoding of the positions from the drag object isn't a one-liner though, 
it requires duplicating Qt's code :(
I guess KonqOperations::doDrop should take care of it though, with this new solution.
Only that one knows about the QDropEvent, so it can decode it, and store the positions
in the DropInfo structure.

> * When dropping multiple items it should probably space the items a little bit
> more intelligent around.
Many bug reports about kdesktop seem to indicate that we should redesign the
whole "icon placement" thing...

> Totally unrelated: I noticed that the "Create New" menu has entries for
> Harddisk/DVD player/Floppy. Shouldn't those be removed? We now have automatic
> device detection and if you really want to keep a way to create such links,
> the control center might be a better place. It's also a bit misleading, it
> doesn't create a new harddisk at all, the best it can do is creating a link
> to an already existing harddisk ;-)

Yup that's why "Application" became "Link To Application" .... This should
have been "Link to harddisk" or so. I'm fine with removing them.
Another suggestion, from BR 58139, is to use submenus, with all devices in one,
all documents in another, and my suggestion is to keep "New Directory" where it is.

- -- 
David FAURE, faure at kde.org, sponsored by TrollTech to work on KDE,
Konqueror (http://www.konqueror.org), and KOffice (http://www.koffice.org).
How to write a Makefile.am for KDE/Qt code:
http://developer.kde.org/documentation/other/makefile_am_howto.html
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)

iD8DBQE+uNQF72KcVAmwbhARAgWQAJ9pZ/MDlNomO+nRPtjTRcHMQFQ/xACdHeEo
/EwS2iHCNQzoYAvnI/xj8kU=
=TNN7
-----END PGP SIGNATURE-----