Skip to content
This repository was archived by the owner on Feb 24, 2021. It is now read-only.

Conversation

arronei
Copy link

@arronei arronei commented Mar 3, 2017

  • Added checks and counts for Unknown element
  • Added checks and counts for Custom element
  • Added test case into current test file

Copy link
Contributor

@gregwhitworth gregwhitworth left a comment

Choose a reason for hiding this comment

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

I'm not going to merge this in until I can satisfy that @mustjab has kicked off our current run and this one won't be fetched until I can ensure that it runs without any errors.


// If the browser doesn't recognize the element - throw it away
if(element instanceof HTMLUnknownElement) {
node = "**UNKNOWN";
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are you doing this custom naming of **UNKNOWN and **CUSTOM? This is going to mean nothing to everyone else IMO.

Copy link
Author

@arronei arronei Mar 6, 2017

Choose a reason for hiding this comment

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

HTMLCustomElement didn't make sense since there isn't such a thing. And HTMLUnknownElement wasn't correct since there is no element named that.

I figured creating 2 special names that bubble up to the top would be the best solution. Though I am not really tied to it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, then we'll keep this we can always change it later.

@gregwhitworth
Copy link
Contributor

@arronei Before doing this can you add in the namespacing of the elements as well. I'm thinking the best way to do this is by adding it in an additional column with the namespace.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants