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

adding compat data for Character Data #1732

Merged
merged 5 commits into from
Jul 18, 2018
Merged

Conversation

lkellar
Copy link
Contributor

@lkellar lkellar commented Apr 4, 2018

No description provided.

@Elchi3 Elchi3 added the data:api Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API label Apr 5, 2018
},
"chrome": {
"version_added": "1"
},
Copy link
Contributor

Choose a reason for hiding this comment

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

chrome_android is missing. Make sure it's present throughout.

"description": "Implements <a href='https://developer.mozilla.org//docs/Web/API/ChildNode'><code>ChildNode</code></a> Interface",
"support": {
"chrome": {
"version_added": null
Copy link
Contributor

Choose a reason for hiding this comment

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

true for all 3 flavors of Chrome and both of Opera.

Copy link
Contributor

@caugner caugner left a comment

Choose a reason for hiding this comment

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

LGTM, apart from the missing sub-features.

blind-review

@@ -0,0 +1,92 @@
{
"api": {
"CharacterData": {
Copy link
Contributor

Choose a reason for hiding this comment

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

The following sub-features are missing (see DOM Standard):

  • appendData
  • data
  • deleteData
  • insertData
  • length
  • replaceData
  • substringData

Since MDN doesn't have any compat data for these sub-features yet, we can set all browsers mentioned in this feature to null. I would also suggest adding mdn_urls, even if the sub-articles don't exist yet.

@lkellar
Copy link
Contributor Author

lkellar commented Jul 15, 2018

Made the changes requested

Copy link
Contributor

@caugner caugner left a comment

Choose a reason for hiding this comment

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

Thanks for adding the sub-features. 🎉

And sorry, I noticed only now that status information are missing.

"version_added": true
}
}
},
Copy link
Contributor

@caugner caugner Jul 16, 2018

Choose a reason for hiding this comment

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

Sorry, I just noticed that status information are missing, both for the feature and each sub-feature. The information can be derived from the Specifications section on MDN.

Since the DOM Standard doesn't mention the ChildNode interface, this sub-feature should be probably marked "standard_track": false" and everything else "standard_track": true".

@lkellar
Copy link
Contributor Author

lkellar commented Jul 16, 2018 via email

@lkellar
Copy link
Contributor Author

lkellar commented Jul 16, 2018

Added the statuses

Copy link
Contributor

@caugner caugner left a comment

Choose a reason for hiding this comment

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

LGTM, ping @Elchi3

Copy link
Collaborator

@ddbeck ddbeck left a comment

Choose a reason for hiding this comment

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

Thanks for this PR, @katzrkool! And thank you for the prior reviews, @caugner! The only thing blocking this from being merged is to sort the browsers in the "support" blocks alphabetically. Thank you!

@lkellar
Copy link
Contributor Author

lkellar commented Jul 18, 2018

Made requested changes

@ddbeck
Copy link
Collaborator

ddbeck commented Jul 18, 2018

Thank you, @katzrkool! 🎉

@ddbeck ddbeck merged commit 3546fdd into mdn:master Jul 18, 2018
@lkellar lkellar deleted the character-data branch July 18, 2018 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data:api Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants