netcreateorg / netcreate-itest

Developing the 2.0 version of NetCreate
https://github.com/netcreateorg/netcreate-2018
Other
1 stars 1 forks source link

The type field appears twice in the edges table #269

Closed jdanish closed 2 months ago

jdanish commented 2 months ago

By default it appears to the right, but we had previously forced it to appear between source and target so that it reads as a sentence (a connects to b), and my guess is that the naturally ordered one is also appearing hence 2. The second one needs to be removed.

Screenshot 2024-09-18 at 2 35 47 PM
benloh commented 2 months ago

@jdanish Ah. So this is a slightly deeper issue.

Right now type is not considered a built-in field, so it gets added after the built-in fields do. But yes, we had swapped field orders to make things feel more natural, so that's why it's showing twice.

So do we: A. Make type a built-in/required field so that we can use this custom order. or B. Leave type as an optional field, and provide special order-handling for type? or C. Leave type as an optional field, and follow the natural order, e.g. if type is the first custom field, it'll show after target?

If we make type built-in, we can still hide the type field on a per-project basis. That seems like the better solution, but it might be weird to require type?

jdanish commented 2 months ago

If we make type built-in, we can still hide the type field on a per-project basis. That seems like the better solution, but it might be weird to require type?

I am leaning this way and it feels naturally to always have a type for nodes and edges since the key is front and center to the project and process.

benloh commented 2 months ago

@jdanish so we're adding type as bulit-in to BOTH nodes and edges? Nodes and Edges can have their own set of required fields.

jdanish commented 2 months ago

Yeah, I think we consistently assume there is a type on both and I cannot imagine a context in which we don't have one, though if it arises, being able to hide it is fine. I haven't tested but I bet if you delete type we run into trouble with color and key, right? So requiring makes sense.

benloh commented 2 months ago

It ends up this is a pretty fundamental change in how nodes and edges are treated. So I just want to make sure that we get this right.

Currently type is an optional field, so it displays in the attributes tab, which is where all of the customizable/optional fields live.

If we are bumping type to be a required/built-in field, do we also want to move type out of the attributes tab? Or do we just leave it in as a special case? If we move it out of the attributes tab, is it OK to put type right below the node label? screenshot_1713 screenshot_1712

For edges, we can even move the type popup next to the "reverse direction" button so that it reads like a sentence again.

jdanish commented 2 months ago

For edges, we can even move the type popup next to the "reverse direction" button so that it reads like a sentence again.

Yes please!!

If we are bumping type to be a required/built-in field, do we also want to move type out of the attributes tab?

Leaning toward yes, but waiting on kalani confirmation.

jdanish commented 2 months ago

Confirmed with kalani

benloh commented 2 months ago

Implemented with #272