V8: Link Picker value not saved/passed in macro context within RTE

[mzajkowski]

It may sounds like a crazy nesting situation, but it became pretty common in some of our projects. We're using Nested Content and Doc Type Grid Editor packages to let editors create more complex data structures in their grid layouts for example. In some cases, when we're using RTE we create some macros to wrap up re-usable bits of code and keep them in control over the whole website.

In some macros we needed a Url Picker data type, so we've exported it to package.manifest and tagged it as isParameterEditor (btw. bump up to this issue: https://github.com/umbraco/Umbraco-CMS/issues/7319) and all is good for most of the properties.

The issue is with the MultiUrlPicker -> Umbraco.LinkPicker. Whenever it's used there it doesn't store the value.

Some ideas to solve it (pushing value in Angular) were discussed here: https://our.umbraco.com/forum/templates-partial-views-and-macros/98546-macro-parameter-editor-in-umbraco-8.

Maybe someone will have a better idea or understanding how it can be fixed??

Umbraco version

v8.x (it's ok with RJP's version of the package in v7.x).

Reproduction

Steps to reproduce

1. Create grid.
2. Create grid editor with RTE in it.
3. Add macro with Url Picker property (exported to package.manifest earlier).
4. Select url in picker within the macro and save.
5. Preview the value in RTE/macro view.

Expected result

Json/string/strongly typed value as macro parameter.

Actual result

No value in property. Empty macro parameter.

[nul800sebastiaan]

Oh man, that's a really tough one. Technically of course this is "not supported" but I am sure you have good reasons for doing what you're trying to do.

I can't really help you along either, but we'll be happy for some community members to look into it and see if they can figure out what is wrong and help fix it.

The only thing I'm afraid of a little bit is that we'll end up having to create some convoluted code for this that will be impossible to understand two months from now. So hopefully a fix for this will be fairly simple and not a weird workaround (if it has to be a weird workaround, it should be well documented in the code at least).

[umbrabot]

Hi @mzajkowski,

We're writing to let you know that we've added the Up For Grabs label to your issue. We feel that this issue is ideal to flag for a community member to work on it. Once flagged here, folk looking for issues to work on will know to look at yours. Of course, please feel free work on this yourself ;-). If there are any changes to this status, we'll be sure to let you know.

For more information about issues and states, have a look at this blog post

Thanks muchly, from your friendly PR team bot :-)

[mzajkowski]

Thanks @nul800sebastiaan!

And yes, obviously the goal is to not "tailor" it for this use-case, but maybe enable it by default as I presume it shouldn't be losing context especially when we have an amazing infinite-editing on place.

Would be great if someone with higher skills in Angular look at it as I totally don't know where to look for the potential solution :) Thanks for placing the label here - keeping an eye on it and seeking for the angular experts to help us all make it useful and better <3

[shearer3000]

hi Marcin - yes you are right, this is the same as 7025. I would like to mention this issue doesn't actually have anything to do with using grids. It is simply to do with the RTE and trying to use the URL picker for a macro parameter. So 1 and 2 can technically be removed from the replication steps. a core datatype like this should be supported in macros.

[mzajkowski]

Yeah, so it is just exposing the wider issue with it - I'm glad TBH :) As if it would be only in the edge case which I've mentioned it might be put far far away on the priorities list. IMO it's now very important as Link Picker is/was one of the most used pickers in our solutions.

Sad I don't have time now to get deeper into it, but maybe someone will!

[stevemegson]

The problem seems to be that MultiUrlPicker only updates $scope.model.value on the formSubmitting event. That fires when saving content, but not when saving macro parameters. Based on how the content picker handles this, adding this code to MultiUrlPicker seems to solve the problem:

$scope.$watchCollection("renderModel", function (newVal, oldVal) {
    $scope.model.value = newVal;
});

It looks like nested content handles this by sending its own formSubmitting event to ensure that property editors update their values, so the same may be possible for macro parameters.

[lars-erik]

I just hit the same problem while using the picker as a setting for a grid editor. Threw in a patched version we can use for the grid use case at least. (Guess it would work for RTE as well, but then you'd need a replacement data type registered)

@stevemegson's suggestion works, and I added a validity check as well since I guess that's the reason someone changed to having a rendermodel and only updating model.value on submit.

$scope.$watch("renderModel",
    function() {
        if ($scope.multiUrlPickerForm.$valid) {
            $scope.model.value = $scope.renderModel;
        }
    });

Dunno if it would also solve #7805.

@nul800sebastiaan Does HQ really need a PR for this, or could for instance @nielslyngsoe just throw it in there before the next release that have room for this? ;)

[nielslyngsoe]

@nielslyngsoe I would also have to make a PR, so same privileges as you. :-D But without looking into this case I did notice that the proposed fix looks for .$valid, which would be a problem if someone wants to save a none-valid property.

We want to allow users to save with validation-errors, but not published with them. So the data should always be set on our model. :-) I hope that helps making a solid PR for this.

[lars-erik]

Thanks, @nielslyngsoe. Then I guess @stevemegson's change is just as good. Or just even drop the use of renderModel all toghether. ;)

[emmaburstow]

Hey folks,

Thanks for reporting this @mzajkowski and for all the discussion here too. Whilst we're sure this is still an issue, we haven't had anyone pick this work up and as @stevemegson has been kind enough to offer a solution so we can workaround the problem, I'm going to close the issue.

Feel free to re-open if you still have a burning desire for the fix. And of course, you can submit a PR with the notes above as a guide too.

Emma