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

Added 'is' attribute for custom elements #3930

Merged
merged 1 commit into from
Jun 2, 2015
Merged

Added 'is' attribute for custom elements #3930

merged 1 commit into from
Jun 2, 2015

Conversation

Wildhoney
Copy link
Contributor

I've been extending out my current project for webcomponents in React, and I noticed that the is attribute isn't supported for React, and is therefore pruned from the element.

I can't be entirely sure this isn't intentional, but I thought I'd open a PR nonetheless. I think it definitely should be in the framework. If not, it would be nice to know what the rationale behind its exclusion is/was.

@syranide
Copy link
Contributor

It should be MUST_USE_ATTRIBUTE as it's not supported by every browser.

@Wildhoney
Copy link
Contributor Author

@syranide thanks 👍 PR updated.

@zpao
Copy link
Member

zpao commented May 22, 2015

cc @jimfb - not sure what the state of web components support is, but let's make sure this fits into the general plan.

@jimfb
Copy link
Contributor

jimfb commented May 22, 2015

@zpao I think this is fixed automatically by #3067, right? Are we ready/willing to accept that one, I can rebase, just let me know.

@jimfb jimfb closed this May 22, 2015
@jimfb jimfb reopened this May 22, 2015
@Wildhoney
Copy link
Contributor Author

Seems like it could be!

@syranide
Copy link
Contributor

@jimfb This attribute is for regular elements, not (only?) web components (#3067).

<button is="taco-button">Eat Me!</button>

@Wildhoney
Copy link
Contributor Author

As far as I understand the is attribute it's only for custom elements, unless I'm mistaken – I've never used the is attribute outside of a custom element context.

In the above case:

document.registerElement('taco-button', {
    prototype: HTMLButtonElement.prototype,
    extends: 'button'
});

@syranide
Copy link
Contributor

@Wildhoney The code I pasted is from the W3 TR above.

@Wildhoney
Copy link
Contributor Author

@syranide sorry, I misunderstood your point. Yep, you're right – #3067 won't solve this for the reason you mentioned – the is attribute is to be used on native elements. Interestingly, it's forbidden to extend an already extended element, so you'd never see:

<custom-element is="another-custom-element" />

@jimfb
Copy link
Contributor

jimfb commented May 22, 2015

Ok, I think you're right, looks like this can be applied to standard DOM elements. I'm fine with moving forward with this diff, pending approval from @zpao et al.

@Wildhoney
Copy link
Contributor Author

@zpao Approved for merge?

@jimfb
Copy link
Contributor

jimfb commented Jun 2, 2015

@Wildhoney I think there is a merge conflict. Can you rebase?

@zpao @spicyj This looks good to me. Any objections to merging? Let me know!

@sophiebits
Copy link
Contributor

Good with me.

@Wildhoney
Copy link
Contributor Author

Sorry, I accidentally removed keyParams and keyType when resolving conflict – re-added them.

@sophiebits
Copy link
Contributor

Can you squash your commits together?

@Wildhoney
Copy link
Contributor Author

@spicyj Is this okay?

@sophiebits
Copy link
Contributor

Try doing a git rebase -i origin/master and then delete all lines but the one commit where you actually added it, then you can force-push to the same branch to update this PR.

@sophiebits
Copy link
Contributor

When you're done, the Commits tab here will have only one commit.

@Wildhoney
Copy link
Contributor Author

Done 👍 I was missing the git push --force – as once I'd squashed all commits, I did a git pull which brought everything back again.

@Wildhoney
Copy link
Contributor Author

Thanks @spicyj

sophiebits added a commit that referenced this pull request Jun 2, 2015
Added 'is' attribute for custom elements
@sophiebits sophiebits merged commit 1af9b54 into facebook:master Jun 2, 2015
@sophiebits
Copy link
Contributor

Thank you!

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

Successfully merging this pull request may close these issues.

6 participants