Brewtarget / brewtarget

Main brewtarget source code repository.
GNU General Public License v3.0
315 stars 135 forks source link

Drag/drop integrated with undo #585

Closed mikfire closed 3 years ago

mikfire commented 3 years ago

The undo functionality didn't work when dropping ingredients from the trees onto the recipe. As this is my primary method of creating recipes, the undo not work was frustrating. I fixed it.

I probably did some bad things by not creating two classes, but I was afraid of creating two different undo queues. This wastes some pointers, but makes sure there is one queue.

This is my second try at this, because git and I are not friends.

matty0ung commented 3 years ago

It's good you're fixing this. I remembered I hadn't made everything undoable, but I hadn't done a systematic search.

There is potentially a more elegant approach. Because Qt already allows undo/redo objects (QUndoCommand) to be grouped together, you could create, say, UndoableAddOrRemoveMultiple that just groups regular UndoableAddOrRemove objects together, in a similar, but more generic, way as is done in MainWindow::droppedRecipeEquipment().

matty0ung commented 3 years ago

Just for fun, if you want to see what this would look like, check out https://github.com/Brewken/brewken/pull/9 which I knocked up this morning. I haven't pushed the change back to Brewtarget, as it doesn't add any functionality, just a different way of doing the same thing but with Qt doing a bit more of the heavy lifting.

mikfire commented 3 years ago

I had toyed with a very similar approach when I realized I couldn't just overload a few things. I was uncertain of the side effects.

Given that the list form is a fairly isolated case -- it is really only needed in four places -- I rather prefer the separate classes. I might take it up later this evening if I am feeling motivated.