Refine the CreateObject component

[adecler]

Issues addressed by this PR

Closes #151

Test files

Just play with the CreateOBject component and the global menu

Changelog

• New Input selection menu for the CreateObject component
• Listing properties in text representation of generated constructors
• Improved behaviour of the WPF menus (used in Dynamo)
• Default value for all inputs of CreateObject component

Additional comments

See #147 for a full discussion regarding those new features

[epignatelli]

Thanks @adecler! That's veeery very nice 😄

Can I be anal and ask for 2 changes?

• Can we avoid triggering the auto-close of the Override Inputs menu after you click on a property? And maybe rename it to something more human-like rather than machine-like? Maybe Add/Remove Inputs?
• Can we remove Name as well? I don't see why it should be an exception to the properties. We still need to remove the input every time we want to create an object if the properties is not relevant (which is 80% of the cases).

I'll do some further tests in the meanwhile!

[epignatelli]

❤️

[al-fisher]

First testing - this is really nice @adecler 😸 - I think the syntax we have gone with works well! Thanks

On @epignatelli comment above:

Can we remove Name as well? I don't see why it should be an exception to the properties. We still need to remove the input every time we want to create an object if the properties is not relevant (which is 80% of the cases).

I personally would give the exception to Name. Is the inherited property that is used most frequently.

Will be useful to have defaults set though for the properties, although @adecler I think you mentioned this was not trivial to reflect?
In that case - perhaps we can hard code defaults for Properties with Primative types. i.e. string would be "", int 0, double 0.0
Is that is a good idea?

[epignatelli]

Maybe I have the wrong picture, but I thought that the cases where we are using Name are just a few. Where is it exactly used? Is it used a lot? If not, my case was that it would be most beneficial to support the most frequent case.

Using a default value is definitely a nice option as well!

[al-fisher]

A point on the way the Method signature is rendered in the menu:
We are including Guid, FragmentSet etc. in the menu description - which are not the actual component inputs.

[adecler]

I have added default values. Know that you will need BHoM/BHoM_Engine#1308 for this to work properly so important to merge both together (although it doesn't actually depends on it in the regular sense of the word)

[adecler]

Alright guys, I think I have addressed all your requests.

The only one I haven't touched is the question of including/excluding Name in the initial list of inputs. Here's my 2 cents on it:

• Initially I had decided to keep it because it is the most likely to be used and it is easier to remove an input than to go in the menu to add one.
• Now that we have default values, it makes even more sense to keep it since you can just ignore it.

[epignatelli]

Thanks @adecler this looks amazing to me!

I still have some other comments, but I will raise a separate issue for them, so we can merge and iterate again!

Edit:
Issues raised from this PR: #154, #155, #156, #157

[epignatelli]

Adding do-not-merge because dependencies have not been merged yet. Feel free to remove the label and merge when they will.

[al-fisher]

Gosh - sorry @adecler - I am not getting the same behaviour as yourself and @epignatelli with the new menu on the Create Component.
See below - Add/Remove Support for instance is having no effect.
I certainly think I am not doing anything wrong here, but will certainly recheck all repos etc. in the morning. Can sanity check together on call tomorrow too perhaps.
Other than this!! (🤦‍♂ sorry!!) - all looking really good with latest subtle tweaks!

[adecler]

@al-fisher , we'll have to debug on your machine because it's all working fine for me even reproducing your exact example (down to where you click)

[al-fisher]

As expected - this was user error on my part! Thanks @adecler 😄
As discussed will complete review and aim to merge today now - to pick up the newly raised issues as additions.

[al-fisher]

Approving based on testing and behaviours of the Add/Remove menus working really well! Not withstanding @epignatelli’s further improvement issues.
Thanks @adecler!

Leaving do-not-merge label on though as currently building Excel_Toolkit against this PR branch gives the below errors for me. I say for me - as worth you double checking @adecler after this morning! 😄

BHoM_UI and Excel_Toolkit do seem to build fine both on master for me though.

The type or namespace name 'ISelector' could not be found (are you missing a using directive or an assembly reference?) Excel_UI C:\BHoMGitHub\Excel_Toolkit\Excel_UI\UI\Templates\SelectorMenuUtil.cs 42

The type or namespace name 'SelectorMenu<,>' could not be found (are you missing a using directive or an assembly reference?) Excel_UI C:\BHoMGitHub\Excel_Toolkit\Excel_UI\UI\Templates\SelectorMenu_RibbonXml.cs 39

@awakeman FYI

[adecler]

@al-fisher, thanks for checking that. I forgot Excel toolkit is not exactly interfacing with the BHoM_UI the same way.

I have created a PR that fixes th eproblem here: https://github.com/BHoM/Excel_Toolkit/pull/169.
It doesn't clash with the existing PR so all should be fine.

[al-fisher]

Thanks @adecler - approved the raised Excel PR also now. Cheers