danyill
commented
1 year ago I think the divider looks good and
Here's a few more things:
-
If I try to open a new file in the publisher plugin, and a file with the same file name is open, nothing happens (this just a local testing using core).
-
I notice that with the attached file the update dataset button is missing: XAT_Prot1_Demo.scd.zip
We seem to use
?disabled=${!!findCtrlBlockSubscription(this.selectedGseControl)but this doesn't work so well is there is no subscriptions. I think somehow this is the wrong method to call.
Jakob clarified:
The missing icon with subscribed DataSets is on purpose as well. The reason here is that changing a DataSet would require to unsubscribe all the data. I don‘t think this is something users want to do. But maybe not showing the option is not good UI. Instead we can disable events of this button or show an alarm explaining why an edit is not possible right now
Perhaps we could either show it as disabled, and include a tool tip and a log message if someone tries to tap it? (How to best show additional context on mobile? Perhaps in openscd-core we can show a snackbar eventually?)
-
Adding FCDOs and FCDAs doesn't refresh automatically but can be forced by changing e.g. the desc toggle bar.
-
If I create a new GOOSE in the GOOSE editor and then try and delete it I get an error:
controlBlocks.ts:10 Uncaught (in promise) TypeError: Cannot read properties of null (reading 'getAttribute') at findCtrlBlockSubscription (controlBlocks.ts:10:44) at GseControlEditor.renderElementEditorContainer (gse-control-editor.ts:145:29) at GseControlEditor.render (gse-control-editor.ts:237:45) at GseControlEditor.update (lit-element.ts:160:24) at GseControlEditor.update (gse-control-editor.ts:49:11) at GseControlEditor.performUpdate (reactive-element.ts:1327:14) at GseControlEditor.scheduleUpdate (reactive-element.ts:1259:17) at GseControlEditor.__enqueueUpdate (reactive-element.ts:1231:25) -
It would be nice to disable spellchecking for some fields using the
spellcheckattribute - only obvious one here is the MAC-Address:
JakobVogelsang
Thank you for the opportunity to provide some early feedback. This feedback is based on ac3b4b90
As always, feel free to use what you think has value and let the rest go :wink:
Some of these comments may be about features or functionality yet to be implemented or where I haven't understood the direction. Either way, happy to discuss.
Overall this is looking amazing :+1: and could easily be the cleanest, simplest editor I've seen anywhere. I had a look at both the mobile and desktop experiences. I did not look at any code.
I get errors after an
npm ion Node 18:I resolved this using the advice here https://github.com/mobxjs/mobx/issues/3532#issuecomment-1304080708
When editing the dataset, re-ordering items by dragging and dropping would be a very nice user convenience.see #3In the DataSet view a divider between the dataset and the top portion may make it clearer where the save button belongs (the top, not the bottom). Why do we need a save button?DoneIn the DataSet view, if I have already clicked on an attribute or a DO when I click add, a helpful user convenience might be that they are taken to the same view within the data model (so the user doesn't have difficult when they decide I just need to add another attribute to this).#4~~Similarly if I choose to add a data attribute or data object should it be immediately below any item which is already activated? Users will get it wrong once and then they'll like it, but if they have drag and drop they probably won't mind. Otherwise and perhaps anyway any filter needs to be removed or somehow extended so the user action is clear after the add action is complete (i.e. so you can see the new FCDA/FCDO in the list). EDIT: It occurs to me that this is only so helpful unless 2 is also done because you can only put it either before or after the selected item. If you choose after, you can't insert at the very first item location and if you choose before you can't insert after the very last item...~~ #4
I really like: the new dataset button, the delete dataset button, the delete FCDA/FCDO. The view feels clean and uncomplicated. To improve it more, however the delete buttons the add new could be controlled by "hover CSS" along the linnnes of:Although we've done this elsewhere, it's not terribly mobile-responsive. Perhaps a better UI would be a hover for desktop but a long-press on tablet/mobile, bringing up the delete option (perhaps using the Navigator/vibrate API).#5~~I am not sure what
PUBLISHER.SELECTBUTTON.DATASETis intended to do. But on mobile being able to scope to an IED up front would prevent keyboard interaction from being required (basically a restatement of #1144 as a general principle for design!). Oh I see, almost what I had in mind. To make it really easy we could consider two dialogs when pressing this button -- first select the IED then select the dataset (I am somewhat of the mind that we have overused long lists!). If I understand correctly this button should be not shown on desktop?~~ #6For the dataset view if there are no IEDs perhaps the search bar of the filtered-list shouldn't even show? Same applies to other views (Report/GOOSE/SampledValue).
Perhaps, similar to what I did recently for ExtRefs, the identity string of the parent should be used to remove duplicated information, e.g. the following:The idea being that this is adding context/namespace/location/path and the content is described using the first line. This also applied to the second line in the DataSet#7It would be nice to see some "Pro mode" edit buttons at some stage.
I know it doesn't affect Macs, but finding a way to remove the vertical scrollbar would be nice. :stuck_out_tongue_winking_eye:
I couldn't add FCDO or FCDA items (using either the oscd-core demo or open-scd, I guess that means this functionality is not yet available?).
I like the functionality to exchange a dataset and think it works very nicely but I am not sure about the "refresh" icon being used in this context.
I'd suggest one of these:
Perhaps swap horizontal is close to what you have in mind?
For the VLAN-ID input on the GOOSE editor. I think the user shouldn't have to enter the hexadecimal value in capitals -- as previously commented somewhere, I'd also like to be able to see the decimal value. I'd like to see the VLAN-ID prefixed somehow with
0x(or in brackets after VLAN-ID, the word "hexadecimal") and perhaps on the secondary text the decimal value. This is not quite as good as being able to enter either decimal or hex but might be more readily implementable:If the APPID is not entered correctly, we need some help text, again, perhaps using the secondary text (e.g. 'Must be 4 hexadecimal characters).
Similarly, for the MAC address the ability to press a "return to default button" might be good for those of us who are never going to remember the first 4 octets if we forget them.
How do we get out of validation problems at the moment? Currently, if I click on a different dataset, I still have a validation error in the GOOSE editor for the new dataset (which is, not in fact, invalid). What I would expect is that if I click on another dataset without completing my entry (or pressing the save button), then clicking another dataset would remove any unsaved changes and restore the original entries. This may be functionality which is not yet implemented.