netcreateorg / netcreate-2018

Please report bugs, problems, ideas in the project Issues page: https://github.com/netcreateorg/netcreate-2018/issues
Other
11 stars 2 forks source link

Nodes with unselected type appear black #202

Closed jdanish closed 2 years ago

jdanish commented 2 years ago

In the current build, the key shows that nodes without a selected type should be gray, and the box is gray in the full edit template screen, but if you create a new node and save without selecting a type it is black.

kalanicraig commented 2 years ago

This is likely a factor of the new template not automatically creating a default node definition with grey hex code

On Thu, Feb 3, 2022 at 12:12 PM Joshua Danish @.***> wrote:

In the current build, the key shows that nodes without a selected type should be gray, and the box is gray in the full edit template screen, but if you create a new node and save without selecting a type it is black.

— Reply to this email directly, view it on GitHub https://github.com/netcreateorg/netcreate-2018/issues/202, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACKL4NGOV73E2UP5465JHNTUZKZPJANCNFSM5NPQRYOA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

jdanish commented 2 years ago

I do see that entry in the template, so I am guessing it is just not getting called?

On Feb 3, 2022, at 12:13 PM, Kalani Craig @.***> wrote:

This is likely a factor of the new template not automatically creating a default node definition with grey hex code

On Thu, Feb 3, 2022 at 12:12 PM Joshua Danish @.***> wrote:

In the current build, the key shows that nodes without a selected type should be gray, and the box is gray in the full edit template screen, but if you create a new node and save without selecting a type it is black.

— Reply to this email directly, view it on GitHub https://github.com/netcreateorg/netcreate-2018/issues/202, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACKL4NGOV73E2UP5465JHNTUZKZPJANCNFSM5NPQRYOA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/netcreateorg/netcreate-2018/issues/202#issuecomment-1029212203, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABKGCKS6TH454E6Z4D66EDDUZKZUBANCNFSM5NPQRYOA. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub. You are receiving this because you authored the thread.

benloh commented 2 years ago

Actually this all depends on which branch you're on. It's in flux.

If you are on toml or flatten, the default blanks are still in the editor.

If you are on typeedit, then the default blank selection should be hidden from the Template Editor.

One behavior that we probably still need to work out: what should happen to a type that is not currently supported? A node with a blank type WILL appear as gray. But a node with an unknown type (e.g. a node with a type that has been removed, such as a "Person" node, but "Person" has been removed from the Node Type Options), the color will appear black. We don't currently have a way to designate that color. I suppose we could use the blank color, but black is also a way to give you feedback that a Node is using the wrong type.

One possibly confusing thing: If you open such a node in the Node Editor, the type will appear blank because we can't match the type to the available lables. But if you look for the node in the node table, the node type will be shown.

Popping up a level, the way the tools are currently configured lends itself to a power user cleaning up a database. But you have to kind of know what to look for and how to do it. We can simplify, but then we'd have to make decisions about how to handle data.

For example, if the "Person" type is removed from type options, and the user has not designated a replacement type, should we just replace all "Person" nodes with a blank type? Keeping the "Person" type, even if it's not displayed correctly, would give you a way to recover from that. On the other hand, we can take a much stronger hand-holding approach where we proactively force the user to make a decision, either requiring a remapping or confirming a mapping to blank. This is much more complicated and I probably would have to write a custom UI for that -- the current cheap approach of using the json-editor probably would not be sufficient.

jdanish commented 2 years ago

Yeah, I like the black for "need to re-set because you hosed the type" but this is puzzling. See the attached - I set it to unselected (appears in database as "") first by not picking a type, and then by changing the type and then changing it back to the blank option in the drop-down with the same result. This is the code from prior to the most recent commit 10 minutes ago. The node in question is named "testing" in this screen capture.

Screen Shot 2022-02-03 at 2 03 04 PM
jdanish commented 2 years ago

Hold on -most most recent code seems to fix it. Verifying.

jdanish commented 2 years ago

Newest commit works as expected so closing this.

benloh commented 2 years ago

This actually might raise an issue: the Edit Current Template editor will let you mess with the raw Node and Edge Type options without the transition tools (modifying dataset data, adding a replacement label). The more focused Node Type Editor and Edge Type Editor features are the ones that have the scaffolding. We can't easily insert the fancy Node/Edge Type Option editor into the full Current Template editor. I suppose we can just remove the type options, or note that it's a power user feature?

jdanish commented 2 years ago

Either way, I think some text warning that the full template editor is for power users only is a good idea since it'll be hard to read for a non-tech-y. Beyond that I defer to Kalani since it is her users that are likely to encounter this short-term.

benloh commented 2 years ago

Warning text added 2512b30e5c8a6fcbb3c9699b89bfcef9a5684fce