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

Added ability to extend the default html5shiv list of elements #142

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@wheresrhys

wheresrhys commented Jan 28, 2014

In order to e.g. run html5shiv over svg elements as well as the default html5 ones without having to know anything about the default list's contents.

I've left the code changes deliberately verbose in order to make it clear what's going on, but it could easily be cut down to something more concise (a couple of ternary operators should do it)

Added ability to extend the default html5shiv list of elements
In order to e.g. run html5shiv over svg elements as well as the default html5 ones without having to know anything about the default list's contents.

I've left the code changes deliberately verbose in order to make it clear what's going on, but it could easily be cut down to something more concise (a couple of ternary operators should do it)
@jonathantneal

This comment has been minimized.

Show comment
Hide comment
@jonathantneal

jonathantneal Jan 28, 2014

Collaborator

I like that you've created a helper property for appending elements to the shiv. I don't like that the element list in this commit is missing elements, at least, template. I'm not sure about removing the (albeit edge case) ability to choose which elements you wish to be shived.

Collaborator

jonathantneal commented Jan 28, 2014

I like that you've created a helper property for appending elements to the shiv. I don't like that the element list in this commit is missing elements, at least, template. I'm not sure about removing the (albeit edge case) ability to choose which elements you wish to be shived.

@wheresrhys

This comment has been minimized.

Show comment
Hide comment
@wheresrhys

wheresrhys Jan 28, 2014

I copied the list from the html5shiv Modernizr are using (this was originally a pull request on Modernizr), so that's worrying, but I can update to make the list match the canonical one.

This doesn't remove the ability to choose which elements to shiv; the list is only extended if options.extendElementsList is truthy; otherwise it overwrites in full, the same as the current behaviour

wheresrhys commented Jan 28, 2014

I copied the list from the html5shiv Modernizr are using (this was originally a pull request on Modernizr), so that's worrying, but I can update to make the list match the canonical one.

This doesn't remove the ability to choose which elements to shiv; the list is only extended if options.extendElementsList is truthy; otherwise it overwrites in full, the same as the current behaviour

@aFarkas

This comment has been minimized.

Show comment
Hide comment
@aFarkas

aFarkas Jan 28, 2014

Owner

Well, the current API to add new elements is indeed really bad. The reason for this, is that we don't have a method like addElements. Adding a new extra property might make it a little bit better, but doesn't solve the API problem. What happens now if two different authors want to extend the extendElementsList property?

I really do understand your problem, but this is not the solution.

Owner

aFarkas commented Jan 28, 2014

Well, the current API to add new elements is indeed really bad. The reason for this, is that we don't have a method like addElements. Adding a new extra property might make it a little bit better, but doesn't solve the API problem. What happens now if two different authors want to extend the extendElementsList property?

I really do understand your problem, but this is not the solution.

@wheresrhys

This comment has been minimized.

Show comment
Hide comment
@wheresrhys

wheresrhys Jan 28, 2014

Fair enough. The solution I had was just trying to work with the existing API, but if it's gonna be overhauled anyway then that's all to the better. Is an update planned for the near future?

wheresrhys commented Jan 28, 2014

Fair enough. The solution I had was just trying to work with the existing API, but if it's gonna be overhauled anyway then that's all to the better. Is an update planned for the near future?

@aFarkas

This comment has been minimized.

Show comment
Hide comment
@aFarkas

aFarkas Feb 1, 2014

Owner

There is currently no roadmap for this. @jonathantneal works on a new version, but this might take some time. I would suggest the following API.

html5shiv.addElements('newelement meganewelement');
//or
html5shiv.addElements(['newelement', 'meganewelement']);

For the unlikley need remove elements from the list the developer can still use the exposed elements property.

What do you think?

Owner

aFarkas commented Feb 1, 2014

There is currently no roadmap for this. @jonathantneal works on a new version, but this might take some time. I would suggest the following API.

html5shiv.addElements('newelement meganewelement');
//or
html5shiv.addElements(['newelement', 'meganewelement']);

For the unlikley need remove elements from the list the developer can still use the exposed elements property.

What do you think?

@aFarkas aFarkas closed this in 40df868 May 3, 2014

@wheresrhys wheresrhys deleted the wheresrhys:patch-1 branch May 3, 2014

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