edemaine / cocreate

Cocreate Shared Whiteboard/Drawing
MIT License
209 stars 27 forks source link

Undo should select #47

Closed edemaine closed 4 years ago

edemaine commented 4 years ago

Undoing/redoing some/all operations (e.g. edits) should probably select those objects that changed, so it's easy to continue changes. (Illustrator does this, for example.)

This at least makes sense when select tool is currently active. I'm not sure what it means when e.g. pen tool is active, in which case e.g. selecting a color would change the current pen color, not the selected objects. So maybe this auto-selection should happen only when select is currently selected. (This is when undo would likely involve edit operations anyway.)

adqm commented 4 years ago

This is interesting to me. I think it's not hard to implement regardless, but it's worth thinking about what is the right metric here, where people won't be surprised by something being selected (or not) after an undo? I see several ways that this could potentially be implemented:

  1. We could use the currently-selected tool as a way to decide whether to select or not, as you suggest above.

  2. We could store whether each action operated on a selection or not as part of the call to undoableAction, and select the relevant objects in the case of undoing an action that acted on a selection (selection information stored as part of the action, regardless of the tool that is selected when the "undo" action is taken).

  3. We could track changes to the selection as undoable actions. GIMP does things this way, actions that change the selection are themselves undoable (i.e., the selection is an active object that is considered as part of the state, rather than a separate thing to keep track of). This is slightly harder to implement if object IDs are not retained after a delete-and-restore operation (I haven't checked to see whether that's the case), but probably still doable.

edemaine commented 4 years ago

I think Option 2 is probably best: store the relevant selection when storing the undoableAction. Then, if an undo/redo operation triggers a (nonempty) selection, we switch the current tool to select.

If this ends up being confusing, we can easily switch to Option 1 using the currently selected tool to sometimes ignore the action. But I think it'll be fairly intuitive...

Option 3 is common in photo editing tools (GIMP and Photoshop do it), but I don't think I've ever seen it in a vector drawing tool (Illustrator, Inkscape, CAD, etc.). Admittedly, it sounds useful, but I fear it'll be too counterintuitive.

I just checked the following behaviors:

adqm commented 4 years ago

Hmmm...I haven't thought about this enough, but I don't think I agree with forcibly changing the tool. If I'm drawing with the pen, for example, and make a stroke I want to roll back, it would be nice to just ctrl+z and then draw the new thing, rather than having to re-select the pen before drawing.

edemaine commented 4 years ago

Ah, that's definitely true. So maybe a mixture of Options 1 and 2 is best, until we define the notion of selection in every tool.

edemaine commented 4 years ago

I implemented this. For the most part, I could compute the correct selection from the undoable operation itself; only the new Duplicate functionality required special care. Selection is modified only in the two modes that it currently makes sense: the select tool and the text tool.