sealcode / sealious

An extensible, declarative node framework
25 stars 2 forks source link

New field type html #268

Closed arkadiusz-wieczorek closed 7 years ago

kuba-orlik commented 8 years ago

I've refactored the docs, @arkadiusz-wieczorek please let me know what you think.

kuba-orlik commented 8 years ago

@arkadiusz-wieczorek I have some questions:

arkadiusz-wieczorek commented 8 years ago
kuba-orlik commented 8 years ago

I suggest that in order to make this field-type feature-complete, we should add regex support.

Also, I think it'd be great to modify the params synopsis and behaviour a little:

type Decision: "remove" | "keep"

params: {
    tags?: {
        default_decision?: Decision,
        keep?: Array<String>,
        remove?: Array<String>,
    },
    attributes?: {
        default_decision?: Decision,
        keep?: Array<String>,
        remove?: Array<String>,
    },
    tag_content?: {
        default_decision?: Decision,
        keep?: Array<String>,
        remove?: Array<String>
    }
}

Note the new tag_content attribute. It's logic is the same as for tags and attrbiutes, but applies to the elements' contents. I've done some thinking and I'm convinced this can be added to current code in an elegant manner.

We'll be able to support a wider range of use-cases this way, including automatic removal of <script> tags' content :)

What do you think?

arkadiusz-wieczorek commented 8 years ago

I think that the tag_content it's good move.

However I don't know how to add regex support. Exactly how to check element in array (is pattern or is only tag)? For not to create each element of the new pattern. Unless we consider all elements as patterns through a literal notation if it is inexpensive.

Maybe developer should add patterns and ordinary e.g. tags in array, but patterns should be in a literal notation. I think about e.g. /^h[1-9]$/i; If you right with me it I don't have any questions for regex support :)

kuba-orlik commented 8 years ago

I think that the tag_content it's good move.

Cool! :)

However I don't know how to add regex support. Exactly how to check element in array (is pattern or is only tag)? For not to create each element of the new pattern. Unless we consider all elements as patterns through a literal notation if it is inexpensive.

Here's my suggestion: we don't give full regex support, but a simple optional wildcard (*):

function is_in(array, tag_name){
    return array.map(function(element){
        return new RegExp(element.replace("*", ".+"));
    }).map(function(regexp){
        return regexp.test(tag_name);
    }).reduce((a, b) => a || b);
}

var to_remove = ["script", "data-*"];

console.log(is_in(to_remove, "script")); //true
console.log(is_in(to_remove, "h1")); //false
console.log(is_in(to_remove, "data-description")); //true

Do you think that's ok?

arkadiusz-wieczorek commented 8 years ago

Might be ok

adwydman commented 8 years ago

What is up with this pull request?

kuba-orlik commented 8 years ago

It's not finished yet. @arkadiusz-wieczorek perhaps you could pick it up sometime soon? :)