Hi @adecler ! I added the status:WIP label to the pr as it is a draft. I was not sure which one the two was the truth (no label or draft state). Let me know if I can review it, in case, I can remove the label!

Thanks @epignatelli , we are starting to have a bit too many labels for it not to be at least a bit confusing :-P. This PR can already be reviewed within the context of what it already promises (i.e. a new component that merges CreateObject and CreateCustom).

This is now ready for review and merging

As a final comment, I think that we can address the comments in a separate pr, especially to unblock the adapter refactoring. Thus, I am approving, but not merging to have a coordination chat before that @adecler

This is looking good - massively going to improve both user and coding experience I think.

One specific comment I have which I think will be good to include in this PR if possible is the way in which the default constructor is represented in the menu.

Is currently showing up as BH.oM.Structure.Elements.Bar(...) as an example.
I think this is likely confusing for new users - as you do not necessarly know what the defining properties of an object are in advance.

Suggest we try and explicitly list out the defining properties as possible inputs. (Knowing that they can actually be removed or left as default).

BH.oM.Structure.Elements.Bar(Node startNode, Node endNode, ISectionProperty sectionProperty, double orientationAngle, BarRelease release, BarFEAType fEAType, Constraint4DOF support, Offset offset)

For bar example will look not too dissimilar to current Engine.Create exposing and mapping directly across many of the object initialiser syntax. a method which we will remove. See screen shot below.

Also as these can get long - I do really like the and "x" more inputs added to the end where needed.

@adecler @epignatelli @alelom - what do you think of the above?

I generally agree we should find a better representation for the (...) constructor.

Suggest we try and explicitly list out the defining properties as possible inputs. (Knowing that they can actually be removed or left as default).

Also as these can get long - I do really like the and "x" more inputs added to the end where needed.

Problem with this is if you end up having a Create constructor and the (...) constructor cut to the same string, I think. So you won't know which one refers to which.

In this cases, I would simply go for the clearest, most explicit description, like using a sentence:
Create BH.oM.Structure.Elements.Bar() with all public properties
which has the advantage of appearing less scary than the other options, or if we want to remain closer to the other options:
BH.oM.Structure.Elements.Bar() - all public properties

In agree with all the above as well.
I guess the problem is that the menu is created when Grasshopper loads. At that time we do not yet mine the properties of each object?

I also agree with @alelom about potentially confusing the custom constructor with the Create methods.

Maybe we can exploit curly braces?
BH.oM.Structure.Elements.Bar() { Node StartNode, Node EndNode, ISectionProperty SectionProperty, double OrientationAngle, BarRelease Release, BarFEAType FEAType, Constraint4DOF Support, Offset Offset }

Problem with this is if you end up having a Create constructor and the (...) constructor cut to the same string, I think.

Don't forget we will be removing any and all Create methods that are simply exposing defining properties only - so there will be, by definition, no conflicts or generating the same string.

I guess the problem is that the menu is created when Grasshopper loads. At that time we do not yet mine the properties of each object?

hmmm ... right. Thanks @epignatelli.
@adecler will this be too much of an overhead at load time? Keen to expose the public properties as parameters here if at all possible.

So there are alternate options above - but personally I do like the idea of not labouring the distinction between creating an object through selecting a Create method or reflected public properties through syntax. Is not important to user here - and may confuse. Interested in thoughts though?

Eduardo is right that gathering the properties would be an overhead. But more importantly to me the distinction between the two type of constructors is actually quite important as they behave differently. One can have some of its input removed, the other cannot. Something you will only see when zooming on the component. Having a clear visual indicator of this difference in the menu is therefore important to avoid guessing work in my opinion. Alessio's point of the final method string looking the same due to the cutting of long parameter chain is also very relevant.

@epignatelli , I like you suggestion on using the context menu to allow adding the base properties instead of providing them by default. Agreed that it would be better to do this on a separate PR though so we can merge this and unblock the PR from @alelom .

Let's catch up in your morning before merging this.

I think that #147 (comment) is a bigger obstacle, because it can scary people - they don't know what to do when they are overwhelmed by all the properties.

For this other issue, I agree that the initialiser provides a different experience compared to the other methods. Despite that, the user does not yet know that when he/she's choosing the entry from the list for the first time.
I do not agree with conforming the initialiser string to the others in all its aspects, because the second time the user gets there, he/she cannot distinguish it from the rest.
I do agree that it should provide the same level of info though, like the available properties. But not at the cost of doubling the loading time.

Agreed @epignatelli - good summary above.

on this:

I do agree that it should provide the same level of info though, like the available properties. But not at the cost of doubling the loading time.

Thinking your idea of curly braces could be a subtle but distinct way of clarifying

To be honest I don't really see the point of listing all the properties explicitly while the idea is always the same: all properties are used as input.

When you are creating an object through the UI you do not necessarily know what the defining properties are.
As a new user, or even as a returning user, I think is important to see side by side the different input options when you are creating your object.
To use our favourite Bar as an example! 😄
Seeing the options of BarbyStartAndEndNode and BarbyLine next to each other is important as can inform design of the workflow and any other objects you might need to create on the canvas.
The menus are actually a really helpful way of exploring what is possible.
Dos that make sense?

It does. So you're saying this helps when people don't know what properties an object is made of and there are multiple ways of building an object.

My concern was that it would not be as obvious that we are dealing with a variable input component anymore (hence the ...) but this is a one time learning aspect versus a full time practical issue so I am with you now.

I like your identification of “one time learning aspect versus a full time practical issue“.
Imagining there are a few ideas like that that we could highlight across the UI as we continue to refine. And therefore design best/most appropriate way to support both.

One idea I just had - is around icons. We obviously only want one component!! (that was this whole task!!) but the wee icons in the search list could subtly reference the difference between variable input component or not. Maybe? An option in addition to the written syntax anyway.

Hmmm, the issue I have with that idea is that the icons in the search are an indicator of the component from the task bar used in each case. This might be confusing if the search shows an icon that is not actually in the task bar. I guess we could make a small change but 1) it might be missed by some people 2) others will consider as a different icon altogether.

Yep fair point - if subtle, then may well defeat the point. Icon are small in the menu here too.