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

Updating CSS Shapes data #1871

Merged
merged 3 commits into from Apr 26, 2018

Conversation

Projects
None yet
4 participants
@rachelandrew
Contributor

rachelandrew commented Apr 20, 2018

The data for CSS Shapes was incorrect in a variety of ways. I have attempted to fix it as part of the work I've been doing on MDN docs for Shapes.

@ddbeck

This is a very nice update. Welcome to BCD, @rachelandrew!

Overall this looks great. There's a couple of linter-related quirks to fix up and a couple of questions about mdn_urls, but otherwise very close to merging. Thank you!

},
"samsunginternet_android": {
"version_added": null
"version_added": "4"

This comment has been minimized.

@ddbeck

ddbeck Apr 25, 2018

Collaborator

Unlike the other browsers, samsunginternet_android version numbers do include the .0 suffix, so they'll need to be added throughout (If you're curious, see #1784 for details)

@@ -55,17 +68,17 @@
},
"inset": {
"__compat": {
"mdn_url": "https://developer.mozilla.org/docs/Web/CSS/basic-shape#inset()",
"mdn_url": "https://developer.mozilla.org/docs/Web/CSS/basic-shape",

This comment has been minimized.

@ddbeck

ddbeck Apr 25, 2018

Collaborator

I was a little confused by dropping the URL fragment here. Does the linked fragment not cover the right thing? If not, we could probably drop this subfeature mdn_url entirely, since the __compat links to the same place

This comment has been minimized.

@ddbeck

ddbeck Apr 26, 2018

Collaborator

OK, seeing the other changes my question is answered. It'd be nice to see the inset fragment restored here, for consistency's sake

"standard_track": true,
"deprecated": false
}
},
"image": {
"__compat": {
"mdn_url": "https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Shapes/Shapes_From_Images",

This comment has been minimized.

@ddbeck

ddbeck Apr 25, 2018

Collaborator

As much as I love this guide, I wonder if maybe this ought to link to the <image> reference? Maybe @Elchi3 has an opinion on the intended use for the mdn_url on subfeatures?

In any case, the en-US/ bit can be removed to get a pass from the linter

This comment has been minimized.

@Elchi3

Elchi3 Apr 26, 2018

Member

+1 on the <image> URL. Thank you!

@@ -12,7 +12,7 @@
"version_added": "37"
},
"chrome_android": {
"version_added": null
"version_added": "37"

This comment has been minimized.

@jpmedley

jpmedley Apr 25, 2018

Contributor

android_webview has the same version number as chrome_android.

@rachelandrew

This comment has been minimized.

Contributor

rachelandrew commented Apr 26, 2018

I think I've addressed all of these comments.

@ddbeck

Thank you for the updates! There's one small thing outstanding and then we'll be ready to go

@@ -55,17 +68,17 @@
},
"inset": {
"__compat": {
"mdn_url": "https://developer.mozilla.org/docs/Web/CSS/basic-shape#inset()",
"mdn_url": "https://developer.mozilla.org/docs/Web/CSS/basic-shape",

This comment has been minimized.

@ddbeck

ddbeck Apr 26, 2018

Collaborator

OK, seeing the other changes my question is answered. It'd be nice to see the inset fragment restored here, for consistency's sake

@Elchi3

Elchi3 approved these changes Apr 26, 2018

I've added the URL fragments real quick, so that this can land in the release I'm preparing right now.

Thanks everyone! 🎉

Comments addressed

@Elchi3 Elchi3 merged commit 507cb88 into mdn:master Apr 26, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Elchi3 Elchi3 referenced this pull request Jul 4, 2018

Merged

Enable shape outside #2413

@rachelandrew rachelandrew deleted the rachelandrew:css-shapes-data branch Aug 21, 2018

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