Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A variant of cleanTags which retains child elements #1141

Closed
paradite opened this issue Jul 12, 2016 · 2 comments
Closed

A variant of cleanTags which retains child elements #1141

paradite opened this issue Jul 12, 2016 · 2 comments

Comments

@paradite
Copy link
Contributor

paradite commented Jul 12, 2016

Description

Currently medium-editor supports cleanTags, in which the elements with the tags will be removed, along with their children.

I am proposing a variant of it, unwrapTags (or any other better name), which would remove the element with the tags but retain the child elements.

So for example, I have cleanTags: ['meta', 'sup'], for the following html:

<div>
  my outer text
  <sup>
    <div>my inner text</div>
  </sup>
</div>

cleanTags now gives:

<div>
  my outer text
</div>

unwrapTags would give:

<div>
  my outer text
  <div>my inner text</div>
</div>

medium-editor already has a util function unwrap which can do this internally.

Here is my implementation of unwrapTags along with the original cleanupTags:

// options
cleanTags: ['meta'],
unwrapTags: ['sub', 'sup']

        cleanupTags: function (el, tags) {
            tags.forEach(function (tag) {
                if (el.nodeName.toLowerCase() === tag) {
                    el.parentNode.removeChild(el);
                }
            });
        },

        unwrapTags: function(el, tags) {
            tags.forEach(function (tag) {
                if (el.nodeName.toLowerCase() === tag) {
                    MediumEditor.util.unwrap(el, document);
                }
            });
        },

Let me know if you find this feature useful enough to be included in medium-editor.

@paradite paradite changed the title A variance of cleanTags which retains child elements A variant of cleanTags which retains child elements Jul 12, 2016
@nmielnik
Copy link
Member

@paradite apologies no one responded to you sooner, I think this feature makes a lot of sense and you should definitely open a PR!

Some other ways to think about implementing it:

option 1

unwrapTags and cleanTags as separate arrays (like what you suggested)

// options
cleanTags: ['meta'],
unwrapTags: ['sub', 'sup']

        cleanupTags: function (el, tags) {
            tags.forEach(function (tag) {
                if (el.nodeName.toLowerCase() === tag) {
                    el.parentNode.removeChild(el);
                }
            });
        },

        unwrapTags: function(el, tags) {
            tags.forEach(function (tag) {
                if (el.nodeName.toLowerCase() === tag) {
                    MediumEditor.util.unwrap(el, document);
                }
            });
        },

option 2

cleanTags takes in all elements you want to remove/unwrap, and unwrapTags just specifies which you want to unwrap instead:

cleanTags: ['meta'],
unwrapTags: ['sub', 'sup'],

cleanupTags: function (el, tags, unwrap, doc) {
    if (tags.indexOf(el.nodeName.toLowerCase()) !== -1) {
        if (unwrap && unwrap.indexOf(el.nodeName.toLowerCase() !== -1) {
            MediumEditor.util.unwrap(el, doc);
        } else {
            el.parentNode.removeChild(el);
        }
    }
}

option 3

have cleanTags support taking in objects as well as strings. This way you can keep everything within the same cleanTags array and keep everything within the cleanupTags function, but specify an additional unwrap flag for the elements you don't want to be complete removed:

cleanTags: ['meta', { tag: 'sub', unwrap: true }, { tag: 'sup', unwrap: true }]

cleanupTags: function (el, tags, doc) {
    tags.forEach(function (tag) {
        var tagName = tag.tag || tag;
        var unwrap = tag.unwrap;
        if (el.nodeName.toLowerCase() === tagName) {
            if (tag.unwrap) {
                MediumEditor.util.unwrap(el, doc);
            } else {
                el.parentNode.removeChild(el);
            }
        }
    });
}

Regardless of what you want to do, keep in mind that functions on MediumEditor.util are public API, so we just have to be careful to make sure they're backwards compatible.

Also, if you go with option 1 or option 2, take the opportunity to make the function more efficient and do tags.indexOf(element.nodeName.toLowerCase()) !== -1 instead of looping through all of the tags.

Thanks and happy coding!

@paradite
Copy link
Contributor Author

Thanks for the reply. I will be working a PR in the next few days.

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

No branches or pull requests

2 participants