U4-628 - Remove 'show' checkbox from Macro Param editor

Created by Sebastiaan Janssen 19 Aug 2012, 14:55:06 Updated by Mike Chambers 19 Dec 2012, 22:11:54

Relates to: U4-1340

Subtask of: U4-760

v4.everything (including 4.8)

When adding macro params there is a checkbox for 'show'. It is useless and should be removed from the UI. Why would you ever make a parameter and then not prompt for it when inserting the macro? Besides, I'm pretty sure the tickbox hasn't actually made any difference for a long time now.

(also, it was removed from v5 for exactly this reason so hopefully no argument about it for v4)

cheers, doug.

''Originally created on CodePlex by [drobar|http://www.codeplex.com/site/users/view/drobar]'' on 7/27/2012 4:50:44 PM [Codeplex ID: 30952 - Codeplex Votes: 7]

Imported comments

''Comment by [sgay|http://www.codeplex.com/site/users/view/sgay] on 7/27/2012 5:18:21 PM:'' It is not useless. We do not want to show all parameters when inserting a macro. For complex reasons but anyway. But it does not work. It's a two-lines fix. Below.

=== (+4,-2) umbraco/presentation/umbraco/plugins/tinymce3/insertMacro.aspx.cs === @@ -91,7 +91,8 @@ { foreach (MacroProperty mp in m.Properties) {

  • if (!mp.Public)
  • continue; macroAssembly = mp.Type.Assembly; macroType = mp.Type.Type; try

''Comment by [sgay|http://www.codeplex.com/site/users/view/sgay] on 7/27/2012 5:22:51 PM:'' Scenarios/ii:

  1. We use some macros to embed images. These macros have a width and a height but we don't want users to set them. When we render the rich text content, before rendering the macros, we do a find/replace to set width and height to appropriate values for the page we are rendering. => we need the parameters but we don't want to show them to users.
  2. We use a special custom "CacheByCulture" parameter that forces the macro to be cached depending on the current culture. We don't want users to see that parameter as it's not meant to be set by users. Granted, there should be another way to achieve this, but anyway. Showing the parameter to users would be a breaking change for us.

''Comment by [mortenbock|http://www.codeplex.com/site/users/view/mortenbock] on 7/27/2012 5:26:27 PM:'' I see a scenario for using it. You could for example have a macro with a bunch of configuration options. When an end user inserts it to the RTE, then he should only see a few, and the rest will be defaulted by the code. If it is inserted in a template, then the developer might want to use some advanced config options.

However there is another way to acheive the same goal. Create two macros pointing to the same ascx/xslt file, and create different sets of parameters on the two macors. Allow one of them in the RTE, and not the other, which can then be used by devs.

I'm happy either way.

''Comment by [mattbrailsford|http://www.codeplex.com/site/users/view/mattbrailsford] on 7/27/2012 5:33:34 PM:'' I can see mortens point about having a more complex view for developers inserting a macro in a template, but I don't think the show option is required for the examples stephan posted.

We should kind of think of macro params defined on a macro as the "user configurable" options for that macro. For anything, you can just set them on the macro tag anyway. Ie, you can define an attribute on a macro tag in your template (or on staphans case, via code) that doesn't actually exist on the macro definition defined in the back office. As long as the macro can handle the property, it will be made available regardless, so there would be no real need to have it.

That said, and like I say, I can understand mortens use case, so I guess it wouldn't be that big a deal to just make it work. It's just got a very limited use case IMO.

''Comment by [mortenbock|http://www.codeplex.com/site/users/view/mortenbock] on 7/27/2012 5:42:49 PM:'' I guess what the "hidden" properties give you is discoverability for the dev. If the macro comes from a package, you might not know that there is a "startNodeXpath" property that could be set.

But again, it could be solved by having two macros:

MyMacroBasic

  • Header
  • Searchterm

MyMacroAdvanced

  • Header
  • Searchterm
  • StartNodeXPath
  • SomOtherOption

''Comment by [sgay|http://www.codeplex.com/site/users/view/sgay] on 7/27/2012 6:01:37 PM:'' Having two macros won't solve my (a) scenario. Matt's suggestion of adding non-declared parameters could, although I was not aware that it would work. Does it really?

But for (b) scenario?

''Comment by [sgay|http://www.codeplex.com/site/users/view/sgay] on 7/27/2012 6:02:35 PM:'' OTOH I could understand that by default you want to show every parameter, so we'd turn that "show" thing into a "hide" thing.

''Comment by [mortenbock|http://www.codeplex.com/site/users/view/mortenbock] on 7/27/2012 6:11:52 PM:'' Stephan, I think the (a) scenario sounds like a bit of a hack. There must be a lot of other ways for the macro to determine which height/width should be used for an image. In my opinoin, all that macro is for, is collecting information from the user/developer

I'm not sure I understand the (b) scenario. How does the parameter help with caching? Is it using umbraco macro caching, or is the parameter just sent to the macro, which then does caching internally? And how is that parameter set, if not by an end user or a developer? Is that injected as well? And couldn't the macro then inject it itself?

''Comment by [sgay|http://www.codeplex.com/site/users/view/sgay] on 7/27/2012 6:40:16 PM:'' @morten (a) is a bit of a hack indeed but a nice one -- but we could live without it (b) create a custom macro parameter and override Value so it returns the current culture. because it is a parameter it will be used in the cache key. so you have different cached content per culture. another one does the same with the current request scheme, because some users want to have different content cached depending on whether they are doing http or https

''Comment by [drobar|http://www.codeplex.com/site/users/view/drobar] on 7/28/2012 2:48:46 PM:'' @sgay - Regarding the "B" scenario...

We talked about your need for a 'cache macro by culture' tickbox to go with the 'by page' and 'personalized' checkboxes at the retreat. Can we separate this issue (removing the 'show' option) from the need for a 'cache by culture' issue? Then each will gets its proper implementation rather than mixing the issues. (Please make a workitem for the cache by culture issue if you've not done so already). That way we wouldn't have to recommend people take your current approach as the preferred method.

As for the "A" scenario, even Stephane says he can do without it.

In which case, would we not be back to being able to remove the 'show' checkbox?

''Comment by [mortenbock|http://www.codeplex.com/site/users/view/mortenbock] on 7/28/2012 3:17:17 PM:'' I would also vote that "caching by custom logic" should not be the reason for keeping the "show" checkbox. Instead, maybe it would be possible to add a fixed parameter where you could do some calculations of a cachekey, that the macro caching would then take into account? Something like

<umb:macro param1="foo" param2="bar" cachekey="<%= MyLibrary.GetCultureOrWhatever() %>" runat="server" />

Could that be the solution? And maybe it would already work, since it seems the params do not need to be defined up front for them to get passed to the macro?

Of course this does not help in the case where the macro is put in the RTE. If that needs to be supported, then the macro definitions would need a field where a custom cachehash calculator could be defined or something?

''Comment by [sgay|http://www.codeplex.com/site/users/view/sgay] on 7/28/2012 3:52:51 PM:'' @all You convinced me. You can dispose of .Show. Kill!

Comments

Chriztian Steinmeier 20 Aug 2012, 21:33:20

Just to clarify a single point in Matt's comment - in the case of an XSLT macro, the XSLT file won't be passed any parameters set on the macro tag, if they are not created as actual parameters on the macro. Regarding the "Show" checkbox - I've never had a use case for it, always wondered what it did, so let's just get rid of it.


Sebastiaan Janssen 07 Oct 2012, 13:47:01

Pull request applied: http://umbraco.codeplex.com/SourceControl/network/forks/michielvoo/UmbracoU4628/contribution/3464 Fixed in changeset b583f85949f1


esunxray 07 Oct 2012, 15:01:47

I think this is useful to me. I always use this function for most macro. And I think this is useful to most user who don't need to know some parameters. By this small function. Most of my macro no need to show any parameters to the end user. This can let user feel very good. My suggestion: keep it there, you will give user one more choice. if someone don't like this, you only need to set checkbox's default value as true.


Michiel van Oosterhout 07 Oct 2012, 15:20:08

@esunxray From what I can tell by looking at the code, when the Show checkbox is checked for a macro parameter, it is saved to the parameter's Public property. However, it seems that this property is not used when the parameters are listed in either insert or edit macro dialogs. In other words, it seems that this feature is not actually functional, because all macro parameters are always listed when inserting a macro into the rich text editor. Perhaps I overlooked something?


esunxray 07 Oct 2012, 15:29:36

So, Is there any way to let it useful?


Sebastiaan Janssen 07 Oct 2012, 15:31:04

I've just tested this in 4.9.0 and in 4.7.0, turning off the "Show" checkbox does not prevent the parameter from showing up in either the rich text editor or the template editor. It did not work at all so there is no need for it.


Mike Chambers 21 Nov 2012, 18:04:14

I have a use... document parameter of macrocontainer , or widgetpicker [http://our.umbraco.org/projects/backoffice-extensions/widget-picker-datatype] where by you may want to allow a content editor limited control of the macro, but allow a developer complete control when the macro inserted into a template ;-)

but a little late to the party :-(

ps MacroContatiner.. relevant code... [umbraco.editorControls.macrocontainer.MacroEditor.cs]

foreach (MacroProperty macroProperty in formMacro.Properties) { //Only add properties that people may see. if (macroProperty.Public) {


Morten Bock 21 Nov 2012, 21:48:55

@Mike As it is described in this discussion, you can achieve the same goal by having multiple macros pointing to the same xslt/cshtml/ascx file. And then just use the limited one for what the content editors will be using.


Mike Chambers 22 Nov 2012, 09:06:08

@Morten Bock Yes I did see that, but that is a significant overhead both in terms of initially creating the macros (we can have up to 30 macro params, we even have a pseudo usercontrol class library so that we can use the import parameters to try to mitigate this before setting the macro to the cshtml file), and maintaining macros going forwards.. the "show" checkbox was in my view a much better control for this. I'd have probably advocated a config setting to show/hide the checkboxes?? If the macrocontainer datatype still has references to macroProperty.Public I presume that the underlying structure still supports the "show" but the checkboxes simply made invisible in the admin?? Maybe someone can point me in the direction of adding them back in..


Priority: Normal

Type: Bug

State: Fixed

Assignee:

Difficulty: Very Easy

Category:

Backwards Compatible: True

Fix Submitted: Pull request

Affected versions:

Due in version: 4.10.0

Sprint:

Story Points:

Cycle: