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

custom-elements: Add a test for customized built-in elements coverage. #9175

Merged
merged 1 commit into from
Jan 24, 2018

Conversation

tkent-google
Copy link
Contributor

@tkent-google tkent-google commented Jan 24, 2018

No description provided.

@ghost
Copy link

ghost commented Jan 24, 2018

Build PASSED

Started: 2018-01-24 07:12:16
Finished: 2018-01-24 07:20:46

This report has been truncated because the number of unstable tests exceeds GitHub.com's character limit for comments (65536 characters).

Failing Jobs

  • safari:11.0

Unstable Browsers

Browser: "Safari 11.0" (failures allowed)

View in: WPT PR Status | TravisCI

Copy link
Member

@TakayoshiKochi TakayoshiKochi left a comment

Choose a reason for hiding this comment

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

LGTM

@TakayoshiKochi TakayoshiKochi merged commit d032fb2 into master Jan 24, 2018
@tkent-google tkent-google deleted the tkent-ce-builtin-coverage branch January 24, 2018 10:04
@snuggs
Copy link
Contributor

snuggs commented Mar 15, 2018

@tkent-google @TakayoshiKochi first would like to say AMAZING work on this. Been a long time coming. 🎉

Looking at ways can contribute more here. Am good at refactoring. Was wondering could we foreach the elements instead of explicitly defining each one? Also did notice a few edge cases towards the end of the setup.

At minimum I believe constructor () { super () } is supperfluous and can just use class extends HTML...Element {}.

At least for HTMLElement Which theoretically should mean the rest. I don't see why native elements would require a constructor. Would at least save 3 * number_of_common_elements lines of code.

Just a thought FWIW.

Great work nonetheless. And FAST too!

@tkent-google
Copy link
Contributor Author

Yeah, the test has a lot of rooms for improvement. Feel free to change it!

@snuggs
Copy link
Contributor

snuggs commented Mar 16, 2018

Yeah, the test has a lot of rooms for improvement

Indeed. However reminds me of one of my favorite quotes. "Better to have a finished stick figure than an unfinished Picasso". Favorite YES. Practicing in my coding NO :-( . I need to print that by my IDE lolz.

Feel free to change it!

You don't have to ask me twice!

Happy Friday @tkent-google !

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.

4 participants