D8850: Support drag and drop between shared folder view containments
This revision was automatically updated to reflect the committed changes. Closed by commit R119:cc9c3d32a381: Support drag and drop between shared folder view containments (authored by mwolff, committed by amantia). REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8850?vs=23065=23066 REVISION DETAIL https://phabricator.kde.org/D8850 AFFECTED FILES containments/desktop/plugins/folder/foldermodel.cpp containments/desktop/plugins/folder/positioner.cpp To: amantia, mwolff, #plasma, hein Cc: broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D8850: Support drag and drop between shared folder view containments
amantia updated this revision to Diff 23065. amantia added a comment. Rebased REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8850?vs=22868=23065 BRANCH master REVISION DETAIL https://phabricator.kde.org/D8850 AFFECTED FILES containments/desktop/plugins/folder/foldermodel.cpp containments/desktop/plugins/folder/positioner.cpp To: amantia, mwolff, #plasma, hein Cc: broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D8850: Support drag and drop between shared folder view containments
mwolff accepted this revision. mwolff added a comment. This revision is now accepted and ready to land. thanks for the clarification BRANCH master REVISION DETAIL https://phabricator.kde.org/D8850 To: amantia, mwolff, #plasma, hein Cc: broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D8850: Support drag and drop between shared folder view containments
hein added a comment. lgtm, but Phab says "You can not accept this revision because you have already accepted it." REVISION DETAIL https://phabricator.kde.org/D8850 To: amantia, mwolff, #plasma, hein Cc: broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D8850: Support drag and drop between shared folder view containments
hein accepted this revision. REVISION DETAIL https://phabricator.kde.org/D8850 To: amantia, mwolff, #plasma, hein Cc: broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D8850: Support drag and drop between shared folder view containments
amantia updated this revision to Diff 22868. amantia added a comment. Guard screenmapper usage CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8850?vs=22797=22868 BRANCH master REVISION DETAIL https://phabricator.kde.org/D8850 AFFECTED FILES containments/desktop/plugins/folder/foldermodel.cpp containments/desktop/plugins/folder/positioner.cpp To: amantia, mwolff, #plasma, hein Cc: broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D8850: Support drag and drop between shared folder view containments
amantia marked an inline comment as done. REVISION DETAIL https://phabricator.kde.org/D8850 To: amantia, mwolff, #plasma, hein Cc: broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D8850: Support drag and drop between shared folder view containments
mwolff requested changes to this revision.
mwolff added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> foldermodel.cpp:1071
> +m_dropTargetPositions.insert(url.fileName(), dropPos);
> +m_screenMapper->addMapping(mappableUrl(url), m_screen,
> ScreenMapper::DelayedSignal);
> +}
unguarded
> foldermodel.cpp:1123
> +if (targetUrl.toString().startsWith(url.toString())) {
> +m_screenMapper->addMapping(targetUrl.toString(),
> m_screen, ScreenMapper::DelayedSignal);
> +} else if
> (targetUrl.toString().startsWith(dropTargetUrl.toString())) {
unguarded
> foldermodel.cpp:1131
> +url.setPath(filePath.remove(0, destPath.length()));
> +m_screenMapper->addMapping(url.toString(), m_screen,
> ScreenMapper::DelayedSignal);
> +}
unguarded
REVISION DETAIL
https://phabricator.kde.org/D8850
To: amantia, mwolff, #plasma, hein
Cc: broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed,
jensreuterberg, abetts, sebas, apol, mart
D8850: Support drag and drop between shared folder view containments
amantia added inline comments. INLINE COMMENTS > mwolff wrote in foldermodel.cpp:1131 > unguarded same here REVISION DETAIL https://phabricator.kde.org/D8850 To: amantia, mwolff, #plasma, hein Cc: broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D8850: Support drag and drop between shared folder view containments
amantia added inline comments. INLINE COMMENTS > mwolff wrote in foldermodel.cpp:1123 > unguarded It is guarded (line 1117) REVISION DETAIL https://phabricator.kde.org/D8850 To: amantia, mwolff, #plasma, hein Cc: broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D8850: Support drag and drop between shared folder view containments
amantia updated this revision to Diff 22797. amantia added a comment. Guard m_screenMapper usage CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8850?vs=22682=22797 BRANCH master REVISION DETAIL https://phabricator.kde.org/D8850 AFFECTED FILES containments/desktop/plugins/folder/foldermodel.cpp containments/desktop/plugins/folder/positioner.cpp To: amantia, mwolff, #plasma, hein Cc: broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D8850: Support drag and drop between shared folder view containments
hein added inline comments.
INLINE COMMENTS
> foldermodel.cpp:1122
> +if (targetUrl.toString().startsWith(url.toString())) {
> +m_screenMapper->addMapping(targetUrl.toString(), m_screen,
> ScreenMapper::DelayedSignal);
> +} else if
> (targetUrl.toString().startsWith(dropTargetUrl.toString())) {
The code in this patch doesn't test for m_screenMapper being set. I don't think
FolderModel should require one to work.
BRANCH
master
REVISION DETAIL
https://phabricator.kde.org/D8850
To: amantia, mwolff, #plasma, hein
Cc: broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed,
jensreuterberg, abetts, sebas, apol, mart
D8850: Support drag and drop between shared folder view containments
mwolff accepted this revision.
mwolff added a comment.
This revision is now accepted and ready to land.
lgtm in general, but a unit test for the positioner would be good to have
INLINE COMMENTS
> foldermodel.cpp:1053
> +if (isDropBetweenSharedViews(mimeData->urls(), dropTargetFolderUrl))
> {
> +/* QMimeData operates on local URLs, but the dir lister and thus
> screen mapper and positioner may
> + * use a fancy scheme like desktop:/ instead. Ensure we always use
> the latter to properly map URLs,
broken indent
> positioner.cpp:403
> +// find the next blank space
> +while (!isBlank(to) && from != to) {
> +to++;
can we get a unit test for this?
BRANCH
master
REVISION DETAIL
https://phabricator.kde.org/D8850
To: amantia, mwolff, #plasma, hein
Cc: broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed,
jensreuterberg, abetts, sebas, apol, mart
D8850: Support drag and drop between shared folder view containments
amantia updated this revision to Diff 22682. amantia added a comment. - Improve path detection as suggested by Milian - fix d for multiple items. There is only one issue, but I think it is acceptable this way: when moving two or more items that are not near each other to a new screen, they will be layed out in a row (or multiple rows) instead of keeping their original relative positions to each other, like when it happens for moving around on the same screen. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8850?vs=22679=22682 BRANCH master REVISION DETAIL https://phabricator.kde.org/D8850 AFFECTED FILES containments/desktop/plugins/folder/foldermodel.cpp containments/desktop/plugins/folder/positioner.cpp To: amantia, mwolff, #plasma, hein Cc: broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D8850: Support drag and drop between shared folder view containments
mwolff requested changes to this revision. mwolff added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > foldermodel.cpp:1126 > +// the targetUrl file:// path to a desktop:/ path for mapping > +auto destPath = dropTargetUrl.path(); > +auto filePath = targetUrl.path(); don't we need to check something like `targetUrl.toString().startsWith(dropTargetUrl.toString())` like above? I.e. including schema and so forth? This could otherwise match for `file://foo/bar` vs. `sftp://host/foo/bar`, no? REVISION DETAIL https://phabricator.kde.org/D8850 To: amantia, mwolff, #plasma, hein Cc: broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D8850: Support drag and drop between shared folder view containments
amantia updated this revision to Diff 22679. amantia added a comment. Drop the items to the right screen when dragging from outside of the folder view. I admit it is not the nicest code for finding the final URL (that will be returned by KDirModel), ideas are welcome to improve it. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8850?vs=22677=22679 BRANCH master REVISION DETAIL https://phabricator.kde.org/D8850 AFFECTED FILES containments/desktop/plugins/folder/foldermodel.cpp To: amantia, mwolff, #plasma, hein Cc: broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D8850: Support drag and drop between shared folder view containments
amantia updated this revision to Diff 22677. amantia added a comment. - More stricter detection for moving between screens to fix issue when a file is moved/copied out from within a folder of the desktop. - Enable the code only when the folder view is used in a containment Bugs to fix: - d from dolphin to a secondary screen drops the item to the first screen - d multiple items between the screen positions them in wrong place CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8850?vs=22657=22677 BRANCH master REVISION DETAIL https://phabricator.kde.org/D8850 AFFECTED FILES containments/desktop/plugins/folder/foldermodel.cpp To: amantia, mwolff, #plasma, hein Cc: broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D8850: Support drag and drop between shared folder view containments
amantia retitled this revision from "WIP: support drag and drop between shared folder view containments" to "Support drag and drop between shared folder view containments". amantia edited the summary of this revision. amantia added reviewers: Plasma, hein. REVISION DETAIL https://phabricator.kde.org/D8850 To: amantia, mwolff, #plasma, hein Cc: broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
