Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split up the Logging Mixin into 2 Mixins #1328

Closed
2 tasks
Tracked by #1300
pascalwilbrink opened this issue Oct 9, 2023 · 6 comments · Fixed by #1334
Closed
2 tasks
Tracked by #1300

Split up the Logging Mixin into 2 Mixins #1328

pascalwilbrink opened this issue Oct 9, 2023 · 6 comments · Fixed by #1334
Labels

Comments

@pascalwilbrink
Copy link
Member

pascalwilbrink commented Oct 9, 2023

As a preparation for Milestone 2 for OpenSCD, we need to split up the Logging Mixin into 2 seperate Mixins to make the current OpenSCD work with OpenSCD-Core.

The undo/redo and the History functionality and the editCount should be removed from the Logging Mixin and be placed in a new Mixin.

The new mixin should be called Historing. Both API's (old and new) stay the same!

The Historing Mixin will contain old LogEntries and new LogEntries.
These interfaces and types should be enhanced with a discriminator (https://stackoverflow.com/questions/14425568/interface-type-check-with-typescript#answer-14426274).

The Historing mixin will render all the LogEntries and will be placed as the last Mixin on top of OpenSCD.

Acceptance Criteria:

  • Logging functionality still works
  • Undo/Redo functionality should work with the new Mixin.

Item to use: format_list_bulleted

@pascalwilbrink pascalwilbrink added the Kind: Enhancement New Request label Oct 9, 2023
@trusz trusz mentioned this issue Sep 27, 2023
9 tasks
@pascalwilbrink
Copy link
Member Author

  • Changed old Editing Mixin to transform the edit-event Event to an oscd-edit Event.
  • the Move and Replace edit-event Events will not be supported. Change them to oscd-edit type Remove and osd-edit type Remove
  • The old Editing API does not support different namespaces. The AttributeValue is always null or a String

@pascalwilbrink
Copy link
Member Author

MicrosoftTeams-image (2)

@pascalwilbrink
Copy link
Member Author

@danyill , What do you think of this?
We would like to split up the current "Log" dialog into a "History" dialog and a "Log" dialog. The history dialog will contain all the actions that can trigger an "undo"/"redo". the "Log" dialog will contain a list of log statements / information

@danyill
Copy link
Collaborator

danyill commented Oct 12, 2023

Hiya. My recollection of the last time this was discussed, is that we didn't have agreement on whether we needed the "Log" dialog as it was used fairly rarely (at least in OpenSCD). I was in the camp of "let's remove it and see if it really matters".

I think if a plugin needs to give a message to a user it can do that however it wants and I'm not sure we need to provide "historical messages" in a unified API. However quite a few of the IEC 61850 engineering software tools we use do this (e.g. DIGSI 5, SEL Architect) and so may be it has some value. Additionally for automated actions which may trigger multiple messages then other interfaces like snackbars are not adequate.

To some extent I think the reason I am uncomfortable with it is that it allows logging messages to be used as a way to show something to the user without complicating a workflow but as a result it puts the onus on the end user to "always go and check the log periodically to see if something weird happened which wasn't otherwise indicated" which seems like a user interface failure.

For this proposal, I guess a downside could be a loss of context, the user can't say: "Oh yeah, I was editing this ConnectedAP and that's when this logging error occurred and that makes sense".

@pascalwilbrink
Copy link
Member Author

Thanks for your message 🙂 what we could do in order to remove the downside a bit, is to have the "error" message clickable, open the history dialog and have the "edit" action with the latest timestamp before the "error" message activated.
Or something like this. I see the downside of having 2 views, but maybe some nice workaround can be thought of

@pascalwilbrink
Copy link
Member Author

Decided is to not split up the Mixin just yet. This will cause a lot of refactoring work.
What will be done, is to rename the Logging Mixin to Historing and split up the Log dialog into 2 dialogs:

  • Log dialog: Will contain InfoDetails
  • History dialog: Will contain CommitDetails

After this split, we can still take a look if and/or when we want to split up these mixins

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants