Reordering rows of QTableView with drag and drop

[Rodrigo B.]

@kshegunov Thank you again.

I made the changes you suggest in the code but unfortunately I still get the same error message.

I've placed the project files for download if that is helpful.

I've installed Qt Creator 4.15.2 just two months ago. I'm using the Clang 5.15.2 Clang 64-bit kit.

As for compiler:

$ /usr/bin/clang --version
Apple clang version 12.0.0 (clang-1200.0.32.29)
Target: x86_64-apple-darwin19.6.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

Hopefully that sheds light on it. Thank you.

[kshegunov]

I don't use MacOS, but I'll build it on linux later this evening just to make sure.

[Rodrigo B.]

@kshegunov friendly reminder about this, if you can. Thanks!

[kshegunov]

I'm sorry! I completely forgot.
I just downloaded it. The std::bind version works out of the box when you fix the inheritance error (look down for details).

Otherwise there's an argument order error for the runtime resolution method:

QMetaObject::invokeMethod(this, "selectRow", Qt::QueuedConnection, Q_ARG(int, m_dropRow));

The inheritance error, though, affects both invocations:

class MyTableView: QTableView

inherits privately, it should be:

class MyTableView: public QTableView

[Rodrigo B.]

@kshegunov said in Reordering rows of QTableView with drag and drop:

QMetaObject::invokeMethod(this, "selectRow", Qt::QueuedConnection, Q_ARG(int, m_dropRow));

Thank you, that fixed my compilation errors! I will experiment with it tomorrow to try to solve my original problem and let you know.

[Rodrigo B.]

Hi @kshegunov ,

Now I had time to experiment with your suggestion but I am not seeing how it may solve the problem.

I've updated with GitHub repository with it if anyone wishes to run it.

There are several things I do not understand:

1. The main one is that I don't see the stated desired effect: I wanted the dropping a row into another to insert itself rather than overwrite, but it still overwrites.
2. Another thing that puzzles me is that, when I drag a row in between two rows, it used to insert itself there (as is the standard behavior), but now it also overwrites. That is to say, it seems to have gone in the opposite direction of what was desired.
3. It seems to me your code has the purpose of selecting the row after drag and drop, but I don't see that behavior. There is no selection after the operation.

Would you please shed some light on these questions? Thanks again.

[kshegunov]

@Rodrigo-B said in Reordering rows of QTableView with drag and drop:

Now I had time to experiment with your suggestion but I am not seeing how it may solve the problem.

That's because I'd forgotten a method. I'm sorry, it happens sometimes when you snip pieces from existing code without thinking too much. I've created a pull request for you, so you could check it out.

[Rodrigo B.]

@kshegunov Thank you, I really appreciate that you went over the code and provided a pull request! However, it still doesn't seem to work. When I drop the first row (Lion) on third row (Mouse), I would expect the Lion row to be inserted right below Mouse, and Gazelle to move up and be the first row. Instead, Lion overwrites Gazelle in the second row and remains in the first row. As far as I can tell, everything is being overwritten rather than moved.

In any case, I am starting to see the idea here... modifying variables in dropMimeData so data goes where we want. So if you don't have the time to look into this I will probably be able to mess around and find a solution. Thanks!

[kshegunov]

@Rodrigo-B said in Reordering rows of QTableView with drag and drop:

Instead, Lion overwrites Gazelle in the second row and remains in the first row. As far as I can tell, everything is being overwritten rather than moved.

That's odd, because it works on linux as described.
Video: https://drive.google.com/file/d/14vvAHgGdyRoJkKqgm_tbT8uRwKt_3JhL/view?usp=sharing

[Rodrigo B.]

@kshegunov Wow, that's mindblogging! Here's my video showing that my local code is sync'ed with the repository containing your change, and behaving completely differently:

https://drive.google.com/file/d/11m9d5xOGGhJN-WM-OhRjHAhGK1_t7vMR/view?usp=sharing

Not quite sure how to proceed now other than submitting as a bug...

[mrjj]

Hi
Just as a note. Win 10. Qt5.15.2
seems to work as expected:

[kshegunov]

@Rodrigo-B said in Reordering rows of QTableView with drag and drop:

Wow, that's mindblogging!

I imagine there's something different on MacOS in the way the drag&drop is handled. But as I said I haven't and I don't own a mac and I've never tested that code on it, so I truly have no idea what it may be, sorry.

Not quite sure how to proceed now other than submitting as a bug...

Yes, you're welcome to do that, although if I were you I wouldn't hold my breath.

Note:
Not sure if it's relevant, but I noticed how you start the application (from the run button). Make sure you've made a full rebuild before testing, you may have stale code.

[Rodrigo B.]

@kshegunov said in Reordering rows of QTableView with drag and drop:

Note:
Not sure if it's relevant, but I noticed how you start the application (from the run button). Make sure you've made a full rebuild before testing, you may have stale code.

Unfortunately things don't change after a Clean and Rebuild (nice observation though).

Alright, I will keep messing with it to see if I find out more.

Thanks @mrjj for running it on Windows, and thanks @kshegunov very much for your help!

[Rodrigo B.]

@kshegunov

Update/good news: when I remove your overriding implementation of MyTableView::dropEvent, things work fine (although we lose the selection persistence after the drop, as was the goal of that piece of code).

Actually, if I keep the method but replace the delayed row selection by selectRow(m_dropRow) (as shown below), I get the anomalous behavior. If I remove this line, things work properly but without selecting the moved row afterwards.

It seems the delayed row selection does not work in the Mac (makes sense since probably different OSs deal with events differently). Then the row selection happens before the dropMimeData and changes the targeted row, causing the bizarre behavior we observed.

void dropEvent(QDropEvent *e)
    {
       if (e->source() != this || e->dropAction() != Qt::MoveAction)
          return;

       int dragRow = selectedRow();

       QTableView::dropEvent(e);  // m_dropRow is set by inserted row

       if (m_dropRow > dragRow)
          --m_dropRow;

       selectRow(m_dropRow); // non-delayed selection has the same effect, so QueuedConnection seems not to work on the Mac.

//       QMetaObject::invokeMethod(this,
//                                 std::bind(&MyTableView::selectRow, this, m_dropRow),
//                                 Qt::QueuedConnection);  // Postpones selection
    }

[Rodrigo B.]

Followup question for the community: it seems changes to drag and drop often happen in dropMimeData, as @kshegunov suggested above.

However, that seems like a less than ideal solution because if violates the Model-View paradigm. The behavior of drag and drop seems to be more related to the view than to the model. For example, I might want to use the same model in two different views but wish to see the behavior described in only one of those views.

Would it be possible to obtain the same behavior but overriding view methods only?

[Max Paperno]

Apologies if this is a repeat of known info... the thread seemed to have delved off topic for a bit so maybe I missed it.

@Rodrigo-B said in Reordering rows of QTableView with drag and drop:

However, dropping D on R overwrites R, even if the QTableView's property dragDropOverwriteMode is false.

I always thought this behavior was strange as well.

I work around the issue by returning false from my model's dropMimeData() method in response to Qt::MoveAction from any of the built-in "QItemView" classes. Even if the move succeeded. I've already done the moving inside dropMimeData() and the model has already updated, so the View will reflect it regardless of what we return there. Returning false makes the item views cancel any further action, like removing any rows(*). It's not ideal since the actual QDrag never gets properly accepted, but I also haven't seen that it matters.

If I want to DnD from my own custom views (or I've re-implemented QAbstractItemView::startDrag()) then I can pass some meta data to my model's dropMimeData() which will trigger the correct true/false result of the drop (eg. if I'm only drag/dropping rows, I can pass column = -2 and the model knows to return the actual result instead of always false).

* More specifically, in QAbstractItemView::startDrag()[1] where it waits for the drag.exec() == Qt::MoveAction, it will then not run the internal d->clearOrRemove() method, which is what does the actual removals. Another way to hack it may be to change the accepted drop method by re-implementing the (simpler) QAbstractItemView::dropEvent[2].

HTH,
-Max

[1] https://code.woboq.org/qt5/qtbase/src/widgets/itemviews/qabstractitemview.cpp.html#_ZN17QAbstractItemView9startDragE6QFlagsIN2Qt10DropActionEE
[2] https://code.woboq.org/qt5/qtbase/src/widgets/itemviews/qabstractitemview.cpp.html#_ZN17QAbstractItemView9dropEventEP10QDropEvent

[Max Paperno]

@Rodrigo-B said in Reordering rows of QTableView with drag and drop:

Followup question for the community: it seems changes to drag and drop often happen in dropMimeData, as @kshegunov suggested above.

However, that seems like a less than ideal solution because if violates the Model-View paradigm. The behavior of drag and drop seems to be more related to the view than to the model. For example, I might want to use the same model in two different views but wish to see the behavior described in only one of those views.

Can you post a more concrete example? I think everything your model needs to know to perform the appropriate drop action is passed to dropMimeData(). I'm having trouble thinking of a situation where the "view" would know better what to do with the data than the model would. One could dis/allow certain moves in the view, but there's no way the view can do a move/update if the underlying model can't handle it.

The default QAbstract*Model::dropMimeData() methods are convenient, but relatively simplistic. Re-implementing provides a lot more control.

Cheers,
-Max

[kshegunov]

@Max-Paperno said in Reordering rows of QTableView with drag and drop:

Can you post a more concrete example? I think everything your model needs to know to perform the appropriate drop action is passed to dropMimeData().

I believe @Rodrigo-B's point is that the model is not supposed to perform any appropriate drop actions at all.

I'm having trouble thinking of a situation where the "view" would know better what to do with the data than the model would.

Why? Say you have an object of type X and a widget representing that object for the user to edit. Would you implement the drag-drop handling in the class X or in the widget class? I'd rather do it in the UI, at least to me it looks more in line with what d&d does. If my X class is also possible to be used without UI, then I'd have no drag-drop at all, so it does seem "wrong" to force it to deal with it ...

[Max Paperno]

Right, certainly the UI is the "first stop" with DnD, and it can and does control, to a large extent, how the user can "physically" interact with the data. But ultimately it's up to the model how to react to those results (move, copy, overwrite, edit, etc). If, for example, you don't want the UI to allow the user to replace (move-overwrite) an item by dropping another onto it, then that move can be prohibited in the UI (or like in the OP, replaced with a move-only action). But the model has to do the actual data manipulation, otherwise that breaks the model/view separation. If the model "can't" do what the UI is telling it (for whatever reason, including data integrity), then it shouldn't be forced to. Especially considering that the model should be the one to encode and decode the MIME data since it would be a) most "familiar" with it and b) is a convenient central place to keep that functionality (and not just for views).

Also just to point out that the "drop" part in dropMimeData() (et. al.) makes it sound limited to DnD. Actually the same method can be used to paste clipboard data, for example, or any other kind of import/export action where MIME-encapsulated data makes sense.

Of course one is also free to re-implement the respective DnD methods in the views (or write custom views) and do all the data manipulation of the model from there (using the respective insert/remove/move methods). Including en/decoding the data. But the default behavior is to let the model handle that part.

I might even go out on a limb and say that the main reason for this topic is that the UI is doing something unexpected by trying to be "smart" about what to do with the data after it has been dropped. From my POV it should not try to manipulate the data after the model has done its thing... ever (keep selection maybe, but that has nothing to do with the model). The UI should be telling the model exactly what to do... eg. if dropping a row onto another row is meant to re-arrange the items (not overwrite the recipient), then the view needs to send the appropriate combination of action, row number, and parent. Then the model knows, oh, this row should be moved before this other one, not replaced. The view really does have ultimate control over what to tell the model it wants to do.

Cheers,
-Max

[kshegunov]

@Max-Paperno said in Reordering rows of QTableView with drag and drop:

Right, certainly the UI is the "first stop" with DnD, and it can and does control, to a large extent, how the user can "physically" interact with the data. But ultimately it's up to the model how to react to those results (move, copy, overwrite, edit, etc).

Yet it doesn't. It reacts directly to the raw representation, not to the move, not to the copy. The model is the one unpacking and interpreting the mime-data. Which is the gist of my whole argument.

If, for example, you don't want the UI to allow the user to replace (move-overwrite) an item by dropping another onto it, then that move can be prohibited in the UI (or like in the OP, replaced with a move-only action). But the model has to do the actual data manipulation, otherwise that breaks the model/view separation.

The model should expose an API to all clients to that API to manipulate the data, which it does. However, it also does interprets what's in the mime-encapsulated data. To bring the argument to an absurdity, why not force the model to interpret a TCP protocol? This simply violates the separation of concerns, and I consider it an API bug. Not a significant one, mind you, but still ...

If the model "can't" do what the UI is telling it (for whatever reason, including data integrity), then it shouldn't be forced to.

Yes, but the UI doesn't tell the model "move this row", it tells it, "hey I have this thing, please do what it says inside".

Especially considering that the model should be the one to encode and decode the MIME data since it would be a) most "familiar" with it and b) is a convenient central place to keep that functionality (and not just for views).

Also just to point out that the "drop" part in dropMimeData() (et. al.) makes it sound limited to DnD. Actually the same method can be used to paste clipboard data, for example, or any other kind of import/export action where MIME-encapsulated data makes sense.

Not in my opinion. The moment you start pushing unsanitized user-supplied data over opaque datatypes to a model, that's the moment you break any three-tier validation you may've hoped to achieve. If the data comes through a network channel, you'd not push it directly to the model, be it mime-compatible or not, would you? Neither should you do it with an UI.

Of course one is also free to re-implement the respective DnD methods in the views (or write custom views) and do all the data manipulation of the model from there (using the respective insert/remove/move methods). Including en/decoding the data. But the default behavior is to let the model handle that part.

Which is what it shouldn't, is the point. It's not its place to interpret what comes from whatever place it happens to come.

I might even go out on a limb and say that the main reason for this topic is that the UI is doing something unexpected by trying to be "smart" about what to do with the data after it has been dropped. From my POV it should not try to manipulate the data after the model has done its thing... ever (keep selection maybe, but that has nothing to do with the model).

And it doesn't. In the proposed fix it's actually necessary to override the model's function to prevent it from overwriting the row. The only thing the view tries to achieve here is to keep the selection, it doesn't modify the drop event at all.

The UI should be telling the model exactly what to do... eg. if dropping a row onto another row is meant to re-arrange the items (not overwrite the recipient), then the view needs to send the appropriate combination of action, row number, and parent. Then the model knows, oh, this row should be moved before this other one, not replaced. The view really does have ultimate control over what to tell the model it wants to do.

Heh, isn't that my argument? The default implementation instead of that passes on the mime-data, instead of saying, "hey, I want you to put me a new row there".