Skip to content
This repository has been archived by the owner on Mar 7, 2019. It is now read-only.

Custom label template broken #429

Closed
kipusoep opened this issue Sep 13, 2017 · 12 comments
Closed

Custom label template broken #429

kipusoep opened this issue Sep 13, 2017 · 12 comments

Comments

@kipusoep
Copy link

kipusoep commented Sep 13, 2017

With version 1.16.0 my custom UrlPicker label template broke.
The labels are empty but appear when I expand an item.
It works with v1.15.1.

The code:

var UrlPickerTemplate = {};

UrlPickerTemplate.getTitle = function (value, scope) {
    //this is the property model
    if (value.length) {
        var firstValue = value[0];
        return ArchetypeSampleLabelTemplates.UrlPicker(firstValue, scope, { propertyName: "name" });
    }

    //if you wanted to get the name of the content instead, you'd have to get it from the server here since it's not in the model

    return "";
};
@kgiszewski
Copy link
Owner

@kipusoep I'll look into this today. Thx for the report.

@kgiszewski
Copy link
Owner

Curious if this is related to #428 @Nicholas-Westby

@kipusoep
Copy link
Author

Looking forward to your findings! :-)

@Nicholas-Westby
Copy link
Contributor

Somewhat related. I think it's because ArchetypeSampleLabelTemplates.UrlPicker calls archetypeCacheService.getEntityById which initially returns null. Subsequent calls will eventually return a value after the cache has been populated.

I could modify archetypeCacheService.getEntityById to return a promise, but that won't help people that are expecting the entity to be directly returned. Maybe I could initiate an additional digest cycle to force it to re-run after the asynchronous call returns, but that could lead to performance issues and unintended side-effects.

In short, it would be relatively easy to fix this particular issue, but fixing all issues others may see will be more challenging, so I'll have to think on that a bit (also open to other ideas).

@Nicholas-Westby
Copy link
Contributor

My current plan is to take both routes. That is, modify archetypeCacheService.getEntityById so it will return a promise if you pass an optional boolean parameter to force it to do so.

Also, I can initiate an additional digest cycle. To reduce the number of digest cycles, I can debounce it so it runs no more frequently than once every half second.

The first method (i.e., the promise) would be preferred, but the second method (i.e., digest cycle) should at least get things working (to some degree) for others that are already expecting the non-promise result.

@Nicholas-Westby
Copy link
Contributor

FYI, I don't expect to be able to get to this until next week. If you like, I can probably squeeze in a quick fix for the promise variation tonight, which would require changes to UrlPickerTemplate.getTitle. It'd basically have to change to something like this:

UrlPickerTemplate.getTitle = function (value, scope) {
    //this is the property model
    if (value.length) {
        var firstValue = value[0];
        return ArchetypeSampleLabelTemplates.UrlPicker(firstValue, scope, {
            propertyName: "name",
            returnPromise: true
        });
    }

    //if you wanted to get the name of the content instead, you'd have to get it from the server here since it's not in the model

    return "";
};

What do you think, @kgiszewski and @kipusoep?

@kgiszewski
Copy link
Owner

i think i might have a fix that is backwards compatible. I'll post soon.

kgiszewski added a commit that referenced this issue Sep 13, 2017
@kgiszewski
Copy link
Owner

I'm gonna grab some lunch: https://github.com/kgiszewski/Archetype/pull/430/files?w=1

Not sure if this is a good or bad idea :) But essentially I'm raising an event that says to clear out any 'loaded' label templates if a new entity has been added to the cache. I have to test this at scale (many fieldsets) and with promises. But so far so good.

@Nicholas-Westby
Copy link
Contributor

Don't have time for a thorough review at the moment, so I'm not entirely sure what's going on. My main concern is that I don't see that you are tying the specific entity being updated to the fieldset title that requires that entity. So I wonder if you would be prematurely considering fieldset titles to be loaded.

Then again, I didn't read it too thoroughly, so I could well be missing something.

@kgiszewski
Copy link
Owner

I stumbled upon an issue in the caching with datatypes, so i resolved that and i'm still going down the rabbit hole. Don't sweat a fix right away.

@Nicholas-Westby
Copy link
Contributor

Cool. FYI, I was tinkering with the way things are cached too for that other issue: Nicholas-Westby@65fd9e0

@kgiszewski
Copy link
Owner

Ok @kipusoep the fix i pushed and released today should take care of some of the bugs we saw. However for your exact issue; you'll wanna try to use the built-in label for URL picker and your template would look something like this:

{{url}} where url is the name of the URL picker.

I'm closing this for now; please ping back if you have any troubles (#431).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants