Skip to content

Automatically detect capitalized HTML attributes#5125

Merged
kevinpschaaf merged 3 commits into
masterfrom
auto-detect-attribute-capitalization
Mar 13, 2018
Merged

Automatically detect capitalized HTML attributes#5125
kevinpschaaf merged 3 commits into
masterfrom
auto-detect-attribute-capitalization

Conversation

@TimvdLippe
Copy link
Copy Markdown
Contributor

Support capitalized HTML attributes by providing a mixin that uses an heuristic based on capitalization.

Reference Issue

Fixes #5114
Fixes #1831

Copy link
Copy Markdown
Member

@kevinpschaaf kevinpschaaf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. As a matter of convention, we put all (non-legacy) mixins in the lib/mixins folder.

  2. I just had a thought, it might be nice to make this an "SVG-compatibility" mixin where we can do all the crazy stuff needed to support SVG's rather than this one thing (which would affect naming, mostly). But then I realized there's actually very little we can do in v1 CE, since SVGElement is no longer extendable (at least for now). The rough value-added SVG compatibility things I had in my head were:

  • Make an SVG namespaced template
  • Make a CE that (begrudgingly) stamps the SVG template to the light dom

@sorvell Worth considering?

* @protected
* @suppress {missingProperties} Interfaces in closure do not inherit statics, but classes do
*/
static _parseTemplateNodeAttribute(node, templateInfo, nodeInfo, name, value) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's go ahead and just put this into PropertyEffects. It seems like it's only going to help and does no harm.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done 👍

@TimvdLippe TimvdLippe changed the title Add mixin to automatically detect capitalized HTML attributes Fix automatically detection of capitalized HTML attributes Mar 6, 2018
@kevinpschaaf kevinpschaaf changed the title Fix automatically detection of capitalized HTML attributes Automatically detect capitalized HTML attributes Mar 6, 2018
let origName = name;
let kind = 'property';
if (name[name.length-1] == '$') {
if (capitalAttributeRegex.test(name)) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add a comment in the code here explaining why we're doing this. The only way we see a capital letter here is if the attr has a capital letter in it per spec. In this case, to make sure this binding works, we go ahead and make the binding to the attribute.

We should also add something simplified about this to the doc.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sorvell Done 👍

@TimvdLippe
Copy link
Copy Markdown
Contributor Author

Docs issue opened at Polymer/old-docs-site#2490

@kevinpschaaf kevinpschaaf merged commit 5422bef into master Mar 13, 2018
@kevinpschaaf kevinpschaaf deleted the auto-detect-attribute-capitalization branch March 13, 2018 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants