sachatrauwaen / OpenContent

Structured Content editing for DNN (Dotnetnuke)
46 stars 25 forks source link

Add contains helper #90

Closed WillStrohl closed 4 years ago

WillStrohl commented 5 years ago

For more dymamic processing in templates, it would be very useful to have the ability to check a string value to see if it contains an expected value within the string.

Proposed Addition:

Handlebars.registerHelper('contains', function (needle, haystack, options) {
    needle = Handlebars.escapeExpression(needle);
    haystack = Handlebars.escapeExpression(haystack);
    return (haystack.indexOf(needle) > -1) ? options.fn(this) : options.inverse(this);
});

It would be called liked so:

{{#contains "valuetolookfor" "setting-valuetolookfor-value"}}
    <div>My custom conditional content</div>
{{/contains}}

This was referenced from the following SO question/answer:
https://stackoverflow.com/questions/15497794/handlebars-if-value-contains-x

I'd like to do this as a PR myself, but I'm going to need some assistance in knowing the correct places to make my updates. So far, I've been basing my updates on the following commits I've found.

https://github.com/sachatrauwaen/OpenContent/commit/5782c5b42c8a04e6378b0424b62735b5c58306e6

https://github.com/sachatrauwaen/OpenContent/commit/942acfb0df9c3a6e71addb84addcb93449880b41

WillStrohl commented 5 years ago

Here is what I've done so far. :)

https://github.com/sachatrauwaen/OpenContent/compare/develop...UpendoVentures:Issues/Issue-90

sachatrauwaen commented 5 years ago

First, thanks to contribute.

The way to contrbute is perfect (develop branch).

I have just some remark about the code, i don't think the foreach loop is needed here . Here a rewrite :

bool res = false;
if (arguments != null && arguments.Length == 2)
{
   var arg1 = arguments[0].ToString();
   var arg2 = arguments[1].ToString();
   res = arg2.Contains(arg1);
}
                 if (res)
                {
                    options.Template(writer, (object)context);
                }
                else
                {
                    options.Inverse(writer, (object)context);
                }
WillStrohl commented 5 years ago

@sachatrauwaen Doesn't the foreach ensure that the arguments are not null though? I based this logic on another existing method in the same class.

WillStrohl commented 5 years ago

Also, what do I need to do in order to get contains added to this? (I'm testing this right now.) image

WillStrohl commented 5 years ago

By the way, I haven't made your suggested changes yet, and testing looks good so far. :) image image

WillStrohl commented 5 years ago

Okay, I just took some time to run through your suggested code now. It's definitely a better performing option, and it also tests well (see below). I'll do a PR once I know how to add this to the autocomplete.

image image

sachatrauwaen commented 5 years ago

Here the place where to add autocomplete for the new helper :

https://github.com/sachatrauwaen/OpenContent/blob/master/OpenContent/js/oc.codemirror.js

and search for handlebarsHelpers. Then just add a item in the array.

sachatrauwaen commented 5 years ago

For info : HandlebarsUtils.IsTruthyOrNonEmpty(arg) is to check if a value is concidered in handlebars as true.

It's a littlebit like in javascript 0, "", false,... = false the rest is true.

So it is not the purpose of this method to ensure that the arguments are not null.