umbraco / Umbraco-CMS

Umbraco is a free and open source .NET content management system helping you deliver delightful digital experiences.
https://umbraco.com
MIT License
4.45k stars 2.68k forks source link

Macro Parameters in V8 - Any chance we can just use DataTypes? #7319

Closed hfloyd closed 1 year ago

hfloyd commented 4 years ago

I think Macros, or macro-like functionality still have a place in the Umbraco ecosystem, but I wonder if it is time to do away with special "macro parameters" and just allow usage of any defined Data Types?

Especially in V8, I'm noticing that the macro parameters available are underwhelming. I have had to code my own custom ones to handle simple things like a radio button list of pre-defined options - which could all be done in the UI via DataTypes with pre-values. The "Property Editor" ecosystem is just so rich, it would be nice to be able to tap that to make macros which are truly editor-friendly.

I'm at the point where I am wondering if I should just use DocTypeGridEditor for all the "macros" I want to make available via the Grid, but there is a part of me that cringes at the idea of creating DataTypes for something which is really just a set of options that will be used to return dynamic data.

I feel like this was discussed at some point in the past, but I don't have a reference to any written conversations now.

There might be some deep-core-code reason that DataTypes can't be used, but can anyone verify that?

hfloyd commented 4 years ago

Related:

shearer3000 commented 4 years ago

yes just using the datatypes like you would on a doctype seems very logical to me. Great suggestion :)

nul800sebastiaan commented 4 years ago

We've discussed this at HQ a few times now and the general consensus is: "yes, but". I need to refresh my memory on this one but I am pretty sure not all datatypes can be suitable for a macro parameter, I just can't remember which ones are not suitable.

To me it doesn't make sense to use a Grid datatype in a macro parameter for example, but maybe there is good reasons to do it?

marcemarc commented 4 years ago

Perhaps the only Data Types made available in the UI for Macro Parameters would be those based on 'Property Editors' that have the existing 'IsParameterEditor' flag set in their configuration?

hfloyd commented 4 years ago

@marcemarc 's suggestion sounds reasonable to me.

@nul800sebastiaan I (who do use Macros) can't think of a good architectural reason to have a Grid as a parameter (there are better ways to handle that amount of content), but I guess the larger philosophical issue to consider is - should the CMS put limitations on such (developer) choices to guide (force?) people towards better implementations... or should the CMS be more concerned with delivering maximum flexibility (as long as technical stability is maintained)?

I know that Umbraco has been constantly evolving, and macros were present in the earlier versions, pre-razor/models builder. It just seems to me that macro parameters are very similar to Doctype properties - and passed in as objects which could be strongly-typed (perhaps even utilizing existing PropertyValueConverters?). I've even written my own Helpers to "GetSafeMacroParamString()" and "GetSafeMacroParamContent()", etc.

nul800sebastiaan commented 4 years ago

@hfloyd

should the CMS put limitations on such (developer) choices to guide (force?) people towards better implementations... or should the CMS be more concerned with delivering maximum flexibility (as long as technical stability is maintained)?

I do believe we need to be opinionated in what kind of flexibility we give out of the box. As you hint at already, "as long as technical stability is maintained" - yep, the problem with extreme flexibility is that it comes at an extreme cost of testing and we know historically that people start building things that Umbraco really can't handle all that well.

As for your suggestion to use PropertyValueConverters for macro params, that could work but probably needs to be opt-in as an upgrade would break existing sites if we suddenly give back data in a different shape. I would consider that a separate feature request for now.

marcemarc commented 4 years ago

I had a look at using PropertyValueConverters in the existing extension method that Macros use to get strongly typed parameter values:

Model.GetParameterValue("alias", "fallback") - and I may be misremembering but because it's not aware of the DataType - there is nothing to simple to work out the appropriate PropertyValueConverter to use... or at least I couldn't work it out!

I was wondering if there was a way to introduce DataTypes for Macro Parameters in a soft way without breaking changes - to perhaps have 'Data Type' as an option in the list of ParameterValueEditors when setting up a parameter and 'if picked' allows you to pick from a list of Data Types that 'support' Macro Parameters usage (eg their underlying property editors have the IsParameterValue flag) - but if a normal editor were picked it would work in the same way

Then you'd have a 'different extension method' for pulling the value that supported PropertyValueConverters...

or something...

... not saying it would be easy, but it would maybe make Macros a first class citizen again.

hfloyd commented 4 years ago

@nul800sebastiaan : I agree that we need to manage complexity in terms of the core codebase for testing purposes - and of course, package creators would be responsible for setting the best option for "IsParameterValue".

I also agree that backwards-compatibility needs to be considered carefully.

One idea that came to me after reading @marcemarc 's suggestion: Perhaps we could add an additional option to the macro itself - to indicate "DataType params" or "Legacy Macro params" (the second option would be assumed if there is no selection made - which would indicate a pre-existing macro). Then the "parameters" UI could be different based on that selection. We'd also have a value to check for rendering/data modeling purposes.

My one concern with @marcemarc 's suggestion is that it would add another "layer" which might not be necessary and changes the architecture from the current macro params: example: Standard: Parameter -> type: ContentPicker : DATA New: Parameter -> type: DataType -> Data Type selected : DATA

In terms of using PVCs, I know they are designed to work "automatically" across all doctypes/datatypes, but is there a way to call them directly? Then they could be used somehow as part of Model.GetParameterValue<IPublishedContent>("alias", "fallback") or Model.GetParameterValue<string>("alias", "fallback")? ( I admit I haven't investigated the "Model.GetParameterValue" source code to know whether this makes total sense.)

Shazwazza commented 4 years ago

Just to chime in here ... and perhaps to trigger further ideas

The ideal scenario is something that @mattbrailsford has spoken about on numerous occassions is to 'just' use Element Types for macro parameter editing. This means you get everything out of the box and we don't have to hack a ton of stuff to attempt to get data types working for macro parameters... which would ultimately mean re-creating Entity Types for macros which is going to be a ... unpleasant to do ;)

I actually think the easier way to achieve what everyone is talking about here is to allow Element Types to be used as the macro parameter editor. This requires a lot less hacking and makes everything consistent. This means allowing the macro to choose which Element Type is responsible for macro parameters. This might be possible in a backwards compat way where you can either choose an Element Type or use the current approach. Of course there's gonna have to be some DB updates for that. The trickier part is 3 fold: updating the back office editing bits to allow for the new editor and then how to deal with the saved data results and then how to deal with getting this data into the PartialViewMacroModel along with have the property value converters in place to populate this data.

hfloyd commented 4 years ago

@Shazwazza A Question - By "Entity Types" are you referring to special doctypes (like Nested Content) with the checkbox for "Is an Element type" selected?

Shazwazza commented 4 years ago

Doh! sorry my brain is fuzzy today, yes "Element Types" is what i meant, i'll update the above

hfloyd commented 4 years ago

So, it sounds like what @Shazwazza is suggesting is basically doctype grid editor. If that is truly the way you think macros will be going, then I'll just stop using macros and use DTGE. I was just hoping to avoid making everything a doctype... The naming is challenging for so many items which may end up being somewhat similar. I have a nice naming convention for macros: Articles_Featured, Products_Featured, Events_Featured, Events_Upcoming... Which might get a bit cumbersome in code.

Shazwazza commented 4 years ago

That's not really what I'm suggesting... i'll try to re-explain....

When creating a macro, you can define a whole bunch of macro parameters which is a manual process. You define a name, alias and an editor for a macro parameter. This sounds a lot like creating a property type on an Element Type right? So instead of when you create a macro and having to go create a bunch of individual macro parameters, you simply just choose to use an Element Type to be your macro parameters.

The proposal here is that we use real "Data Types" for macro parameters but this comes with a ton of plumbing. This plumbing is already done with Element Types.

By allowing a macro to just choose an Element Type as it's parameter editor, this makes almost all of the back office editing experience consistent and this does mean you will be using Data Types for macro parameters.

The macro editor could be updated to directly launch the Element Type builder instead of having to pre-create one. It could also auto-prefix this and put it inside a "Macro" folder in the document types tree. It could probably be done so that you don't really have to worry about these things and the experience wouldn't be that much different than indivisually creating macro parameters today, instead you'd just be creating property types.

Does that make more sense or no?

Shazwazza commented 4 years ago

Also note, you would never reference this element type in code, there would be no need for that, it would purely be used for editing macro parameters.

marcemarc commented 4 years ago

@hfloyd you could still devise a neat naming convention for your Macros, and have a 'folder' in the Document Type Tree called 'Macro Definitions', to avoid confusing these Element Types with other Document Types. You might also have a scenario where the same definition is useful for a Macro AND a Nested Content repeating item etc. and with the UI using infinite editing, it might not feel like you've left the Macro Parameter editing page, to go off and create your parameter configuration element as a Doc Type. So yes it would be a lot like DTGE, but with the benefit of inserting into a Rich Text Editor, instead of only using the grid, which for some scenarios is a little sub optimal.

hfloyd commented 4 years ago

Hi guys, I have no problem with doing it this way if that is what makes sense architecturally. I was just bringing up possible objections.

One of the main reasons I have been interested in this topic right now is that I am building a Cloud Baseline which I would like to be as user-friendly and future-proof as possible (it will be continually evolving, with updates pushed to child sites automatically). There will be an ever expanding collection of macros, so I am trying to figure out the best way to implement them now, in light of where macros are going in core. At this moment, with the conversation going this way, my original question for myself was: Should I use "macro" or should I use "doctype grid editor"? It sounds like I should look more into doctype grid editor at this time. If macros ever get upgraded to utilize similar functionality, the transition won't be as difficult, since all the "macro element types" will already exist.

@Shazwazza by "in code" I meant models-builder-generated classes.

leekelleher commented 4 years ago

Posting my "Super Macros" suggestion (from the "CG19 Dream Corner") - for posterity.

make-macros-great-again

I bored various folk at CG19 rambling on about this. Echoing most of what has been said above.

My general suggestion was to replace/merge ElementTypes with SuperMacros - ultimately if Macro Params were richer (backed by DataType configurations), we could do away with ElementTypes (as being a checkbox on ContentTypes). NestedContent (and DTGE, et al) would use these SuperMacros instead.

hfloyd commented 4 years ago

Thanks, @leekelleher for chiming in! It sounds like you have the opposite suggestion to @Shazwazza

(Perhaps your final (red) point is what he would agree with ;-) )

Ps. Nice Super Mario drawing

leekelleher commented 4 years ago

@hfloyd I'll need to re-read all the above comments. :flushed: I suspect overall we have a common goal - to simplify how we create (and reuse) rich modular content.

hfloyd commented 4 years ago

@leekelleher I guess I have always thought about Macros as fundamentally serving a different purpose than something like Nested Content...

At least in my recent site implementations, I have used Nested Content for allowing an Editor to create 0-∞ Content Items on a Content Node, with the understanding that the items are "first-class-data-citizens" - that is, fundamental to the data object (aka "Ingredients" on a Recipe, "Sections" on a page, etc.)

I always think of Macros more as sets of options to pull-in and dynamically display data from other parts of the site (like my examples above - Articles_Featured, Articles_Latest, Products_Featured, Testimonial_Featured, Testimonials_Related, Events_Featured, Events_Upcoming)

But I do understand that other developers use Macros in different ways, and for those who are using Rich Text Editors heavily for Editors to select on-page content, Macros are the only way to allow an Editor to render some content in a structured way (from the developer perspective), for instance, in a box with a header and a description). (I am mostly using the Grid control for page content, so I have DTGE "widgets" to manage formatting general content, and I'd like to use "macros" more for those "pull in dynamic content here" needs - but that's just me)

image

image

leekelleher commented 4 years ago

@hfloyd I agree. My take on these "Super Macros" could serve both purposes.

Maybe the naming isn't quite right - in trying to cover both structured content (backend/CMS) and modular features (frontend/website) - might be the confusing part.

Shazwazza commented 4 years ago

@leekelleher I unfortunately didn't see your chat about super macros. I don't quite understand what you mean by them though, especially this comment: "My general suggestion was to replace/merge ElementTypes with SuperMacros"

We will actually be using Element Types more in the CMS rather than less. For example, the new block editor and the way that future complex editors that will be based on the block editor data structure wil all use Element Types. Each block can actually be a combination of Element Types, one to represent the content and one to represent the options. So not sure how this fits in with SuperMacros, or maybe I'm not quite getting what you mean... or maybe we are actually just talking about the same thing? :)

I don't see why there couldn't be a macro block in the new block editor, in my mind that was a given considering we support that in the current grid editor. The new block editor will essentially be a better version of nested content. I'm unsure why nested content doesn't or can't support macros now but in the future, if macros just used Element Types for their macro parameter editing, it would not only be quite easy but it would also be completely consistent with how items are edited in nested content and in the new block editor.

umbrabot commented 3 years ago

Hiya @hfloyd,

Just wanted to let you know that we noticed that this issue got a bit stale and might not be relevant any more.

We will close this issue for now but we're happy to open it up again if you think it's still relevant (for example: it's a feature request that's not yet implemented, or it's a bug that's not yet been fixed).

To open it this issue up again, you can write @umbrabot still relevant in a new comment as the first line. It would be super helpful for us if on the next line you could let us know why you think it's still relevant.

For example:

@umbrabot still relevant This bug can still be reproduced in version x.y.z

This will reopen the issue in the next few hours.

Thanks, from your friendly Umbraco GitHub bot :robot: :slightly_smiling_face:

FransdeJong commented 3 years ago

@umbrabot still relevant

I'd like to bring this under attention again.

Macro's are still really useful for instance to create complex buttons in the Rich text editor. It would be great if we could have a easier way to reuse the property editors available in Umbraco.

Using element types instead of the macro parameters would make total sense and would be a huge improvement.

graealex commented 3 years ago

Lol on declaring this feature request stale because none of the developers care. Yes, having proper macro parameters matters. Right now, even if you circumvent the lack of a proper GUI by declaring property editors in a package.manifest, half of them will not work properly, i.e. nested content. This used to work in V7, and before that might even had a GUI.

FransdeJong commented 3 years ago

@Shazwazza @filipbech Can you provide us with a status update on this issue? I'm not asking for a ETA like a date but maybe a timeline like possibly in V9 or V10? I would love to keep this on the radar.

hfloyd commented 2 years ago

Anyone with an interest in macros is invited to add to this discussion: https://github.com/umbraco/Umbraco-CMS/discussions/11918

@FransdeJong @graealex @leekelleher @shearer3000 @marcemarc

shearer3000 commented 2 years ago

thanks Heather 😁 the simple take on all of this is that if macros was a brand new feature being added to umbraco today for arguments sake then they would use the core data types not different ones

Myster commented 2 years ago

Both a macro and a Nested Content Item are primarily just a collection of properties. Currently, the best way we have to define a collection of properties is the Element Type, It makes sense we should use that. also, the name is vague enough to encompass macros, nested content, and whatever other uses we come up with next that needs a collection of properties.

Zeegaan commented 1 year ago

I am closing this as it was used as an discussion but @hfloyd has opened one here where you can join in: https://github.com/umbraco/Umbraco-CMS/discussions/11918