OMICRONEnergy / oscd-designer

Apache License 2.0
3 stars 6 forks source link

Don't remove entire connectivity nodes when moving containers #57

Open ca-d opened 10 months ago

ca-d commented 10 months ago

Because we want the entire movement of a process level container or a piece of conducting equipment to consist in a single editor action (single undo/redo), we've chosen to

  1. remove entire connectivity nodes when moving containers and
  2. forbid short-cut connections of equipment.

If we were to give up that constraint, we could instead dispatch all removeTerminal edits immediately before dispatching the move or rotate edits themselves. This would eliminate the need for the two safeguards named above, making moving and rotating connected elements more convenient while necessitating more ctrl+z key presses in order to get back to the fully connected state before the move or rotation.

danyill commented 10 months ago

I didn't quite realise that was the trade-off.

My feeling is that the "Ctrl+Z" problem is somehow a fundamental usability issue in open-scd-core and there are things we can do other than writing difficult code to prevent multiple events. I'd like to discuss this some time. I have this issue with my own plugins is that I am often dispatching many events and the work to dispatch say a single event is either impractical or complex.

For instance:

The present behaviour where we can see some "nodes" changed is unlikely to be useful in a user facing fashion for everything except the "smallest" undo:

image

Almost none of that seems very useful to me. Without timestamps I can't even try and associate it with my memory of what I was doing.

So to try and answer the question I think that the "single editor event" restriction spawns unreasonable complexity for plugin authors and the solution for this is improved data capturing and user interface. Because of that I think we should let this restriction go (in general) and perhaps discuss the way undo/redo work to see if we can find agreement/a better API.

(fyi @jakobvogelsang)

JakobVogelsang commented 10 months ago

Hope I can add to this discussion @danyill

I personally like the fact that you as a plugin author have the ability to "design" the history looks by concatenating all edits you - the author of the plugin - feel is one fundamental step. For me that worked perfectly fine up until now. To construct such a fundamental step is then a matter of code structure. With some effort you can make this readable in the code.

What I do miss is a sort of title key in the event detail. The history as is functionally ok, but not readable. I think this is something we should tackle very soon, as this is probably introducing a breaking change. Every month we wait we need to touch more code to adjust.

A second thing that IMO would be nice to have is @danyill proposal to introduce timestamps in the history. I do believe those would make the live of the user easier. This one is easier to introduce, but we need to make some decision upfront such as

  1. What do we do with the timestamp in undo
  2. What do we do with the timestamp on redo

Some improvement that do not have anything to do with the API could also be (but I am not sure how easy to implement):

  1. Have a button in the UI saying ...undo till where you can choose the step in the history to jump to (only makes sence with a title ?and timestamp).
  2. Allow to jump into a complex action as a user and chose some step in there to jump to when you want to undo. By doing so, we then would need to "unfold" the complex action, and this would then be a permanent change. (highly complex tom implement and rather a future far future implementation)

I would propose to have an in digital person meeting @danyill what do you think and then write some tickets on top of that.

danyill commented 10 months ago

It would be good to discuss although I am kind of hijacking this issue... :airplane: :bomb:

One of the biggest deficiencies (at least in my plugins) is pushing out too many events at the moment and it being impractical for users to undo/redo - it's just too unclear what is being achieved. So what ends up happening is that a Menu > Save occurs before "each important work step" which seems like it should be unnecessary.

One option could be to open a sidebar (rather than a dialog) so that the user could see the impact of the undo/redo on the edits and get visual feedback on when they had "done enough" but that could be restrictive for plugins.

But if we had a timestamps then perhaps undo/redo could be a slider (x seconds, y minutes ago), a total of nodes and an updating (as I slide) list of actions and their descriptions (with your proposal) might be just the ticket (?).

danyill commented 10 months ago

I guess in ye olden days we would "checkpoint" a document, being a user facing way of saying "keep changes up to this point" and allow reverting to it. This forces the user to choose "meaningful points"

I personally like the fact that you as a plugin author have the ability to "design" the history looks by concatenating all edits you - the author of the plugin - feel is one fundamental step. For me that worked perfectly fine up until now. To construct such a fundamental step is then a matter of code structure. With some effort you can make this readable in the code.

Fair enough, it can be done but I think perhaps it causes unnecessary friction (as per this issue).

Do you think a better option could be allowing a group of edits to be tagged (by the plugin provider) with the same identifier and a description, so that they are grouped as a single item for the purposes of undo and redo -- a bit like console.time(someLabel) and console.timeEnd(someLabel).

ca-d commented 10 months ago

I like that last suggestion best of the ones offered so far, although it may still introduce quite some complexity to the API from the API user's point of view. Going with adding titles to what we have (@JakobVogelsang's suggestion) would definitely be the simpler solution.

This discussion is very valuable, I'd love for it to be moved over to open-scd-core. What do you think about that?

As regards the specific issue at hand, do you guys think it would be fine to just make the connections disappear individually and make the user undo several times to get them all back?

danyill commented 10 months ago

As regards the specific issue at hand, do you guys think it would be fine to just make the connections disappear individually and make the user undo several times to get them all back?

Yes, totally fine as discussed.