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

Feature/422 label template enhancements #425

Conversation

Nicholas-Westby
Copy link
Contributor

Allow label template functions to return promises and functions that can be invoked with injected dependencies.

…aded using promises (still need to actually allow for promises).
…promises work easier). In short, each substring is stored into an array, those items are modified, then when all the modifications are done the substrings are combined again.
Allow the value returned from a label template function to be recursively resolved until it finally becomes a string.
@kgiszewski
Copy link
Owner

This is looking good @Nicholas-Westby 👍

I need to put it thru the wringer and we'll get this guy merged in.

I would like a promise (or 2) example from you to verify your intent and functionality.

@kgiszewski
Copy link
Owner

@kjac Another set of eyes would be helpful :)

@Nicholas-Westby
Copy link
Contributor Author

Just made one more change (to allow for the functions to return things like numbers).

Also, did you see this comment?: #422 (comment)

It has some examples, including one promise example, which I'll paste again here for quick reference:

// Returns a function to be invoked with dependency injection,
// which in turn returns a promise to be resolved to a string value.
function testPromise(value) {
    return function ($timeout) {
        return $timeout(function () {
            return "As Promised: " + value;
        }, 1000);
    }
}

The $timeout function returns a promise. Perhaps you are wanting an example that returns a promise before it returns a function? If so, here is one:

// Returns a promise to be resolved to a string value.
window.testPromise2 = function(value) {
    return $timeout(function () {
        return "As Promised: " + value;
    }, 1000);
};

The trick is, that won't work unless this function exists inside of another function that happens to have a $timeout variable. For example, if you wanted to add this directly to Archetype, you could do so in archetypeLabelService.js:

angular.module('umbraco.services').factory('archetypeLabelService', function (
    archetypeCacheService, $q, $injector,
    // Note I added the $timeout dependency.
    $timeout) {

    // Returns a promise to be resolved to a string value.
    window.testPromise2 = function(value) {
        return $timeout(function () {
            return "As Promised: " + value;
        }, 1000);
    };

I'll see if I can work on an $http example to actually do something useful.

@Nicholas-Westby
Copy link
Contributor Author

OK, I just put together a... useful... example:

angular.module('umbraco.services').factory('archetypeLabelService', function (
    archetypeCacheService, $q, $injector,
    // Note I added the userService dependency.
    userService) {

    // Returns a promise to be resolved to a string value.
    window.userFirstName = function() {
        return userService.getCurrentUser().then(function (user) {
            return user.name.split(" ")[0];
        });
    };

You can then use a label template like this:

{{userFirstName()}} Likes {{petName}}

When might render to a label like this:

Kevin Likes Toytles

So useful.

Again, this could be rewritten like so (thanks to the new dependency injection capability):

// Returns a function that returns a promise to be resolved to a string value.
function userFirstName() {
    return function (userService) {
        return userService.getCurrentUser().then(function (user) {
            return user.name.split(" ")[0];
        });
    };
};

@kgiszewski
Copy link
Owner

Awesome, I hope to get to test it out tomorrow!

@kjac
Copy link
Contributor

kjac commented Sep 7, 2017

The code looks good to me. I don't have time to test atm, but I'm sure you guys are more than capable 😄

@kgiszewski
Copy link
Owner

@kjac no worries, I'm planning on checking out the code and testing locally today.

@kgiszewski
Copy link
Owner

Hey bud, testing this out I stumbled on a bug:

image

@kgiszewski
Copy link
Owner

The Archetype fieldset is throw exceptions. This particular one is a simple headline + image list:

image

The template is blank and should by default return the name of the fieldset.

@kgiszewski
Copy link
Owner

I'm gonna keep testing, so for now I'll put in a template into the offending fieldset configs.

@kgiszewski
Copy link
Owner

kgiszewski commented Sep 7, 2017

image

Everything else is testing out good too except this form doesn't make sense to me (below):

angular.module('umbraco.services').factory('archetypeLabelService', function (
    archetypeCacheService, $q, $injector,
    // Note I added the userService dependency.
    userService) {

    // Returns a promise to be resolved to a string value.
    window.userFirstName = function() {
        return userService.getCurrentUser().then(function (user) {
            return user.name.split(" ")[0];
        });
    };

@kgiszewski
Copy link
Owner

@Nicholas-Westby This is quite awesome. If you wouldn't mind tracking down the 'blank' label template issue, we should be g2g 👍

@Nicholas-Westby
Copy link
Contributor Author

Cool, I will look into that issue with the empty label template.

Regarding the form that doesn't make sense to you. That's a weird hack just to show you a particular example. Note the line at the top of this file: https://github.com/Nicholas-Westby/Archetype/blob/d46d64c52ba216d7d7d334267f718a7083c49f6e/app/services/archetypeLabelService.js

It is:

angular.module('umbraco.services').factory('archetypeLabelService', function (archetypeCacheService, $q, $injector) {

The sample code is just a function added to that file. You should never do that of course, but it seemed like you wanted an example with just promises and no dependency injection function, so you have to resort to doing weird stuff if you want to test that (part of the reason natively supporting dependency injection is useful). In order to use promises, you need to inject a service that returns a promise, and this hack was a way to get a reference to one (in this case, the userService).

Hopefully that clears it up? If not, I would simply ignore that example as it shouldn't be used in practice.

@Nicholas-Westby
Copy link
Contributor Author

FYI, the exception/blank label issue was pretty straightforward (needed to convert strings to promises that return strings). Fixed now.

@kgiszewski
Copy link
Owner

Super, will have a peek either this afternoon or first thing in the morning tomorrow.

@kgiszewski
Copy link
Owner

I'm thinking of a release in the next week or two.

I would like you to add some info here for label templates to be available with the release: https://github.com/kgiszewski/ArchetypeManual/blob/master/02%20-%20Configuration.md#label-template

Great work sir.

@Nicholas-Westby
Copy link
Contributor Author

Will do. I'll probably get to that over the weekend.

@kgiszewski kgiszewski merged commit b1f8c6a into kgiszewski:master Sep 7, 2017
@kgiszewski
Copy link
Owner

👍 Merged sir, thanks so much.

@Nicholas-Westby
Copy link
Contributor Author

FYI, I plan to improve this, but here's a quick peek into the documentation I'm writing: Nicholas-Westby/ArchetypeManual@c8f07b0

@Nicholas-Westby
Copy link
Contributor Author

Documentation pull request here: kgiszewski/ArchetypeManual#6

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

Successfully merging this pull request may close these issues.

3 participants