Separating Grid and Flex support for align-items #2156

rachelandrew · 2018-05-25T09:09:02Z

Further to emails with @chrisdavidmills and @wbamberg an attempt at adding a separate section for grid and flex layout support info.

I am sure this will need all kinds of changes but if we can get this one figured out then I can go through all the rest.

ExE-Boss · 2018-05-26T01:01:58Z

css/properties/align-items.json

-        },
-        "first_last_baseline": {
+        "flex_context": {
+          "description": "Supported in Flex Layout",


This should be a sub-item of __compat.

I'm not sure I understand this comment - I was going on a suggestion by @wbamberg which had this outside __compat, so perhaps he could comment to clarify.

It's my mistake: yes, description should be inside __compat :).

ExE-Boss · 2018-05-26T01:02:42Z

css/properties/align-items.json

+              "chrome": [
+                {
+                  "version_added": "57"
+                }


Arrays are only permitted in support blocks when they have more than 1 entry.

ExE-Boss · 2018-05-26T01:02:47Z

css/properties/align-items.json

+              "chrome_android": [
+                {
+                  "version_added": "52"
+                }


ExE-Boss · 2018-05-26T01:02:59Z

css/properties/align-items.json

+              "edge": [
+                {
+                  "version_added": "16"
+                }


ExE-Boss · 2018-05-26T01:03:00Z

css/properties/align-items.json

+              "edge_mobile": [
+                {
+                  "version_added": true
+                }


ExE-Boss · 2018-05-26T01:03:03Z

css/properties/align-items.json

+              "firefox": [
+                {
+                  "version_added": "52"
+                }


ExE-Boss · 2018-05-26T01:03:04Z

css/properties/align-items.json

+              "firefox_android": [
+                {
+                  "version_added": "52"
+                }


ExE-Boss · 2018-05-26T18:50:42Z

css/properties/align-items.json

-                "version_added": null
-              },
+                "version_added": "52"
+                },


Wrong indentation

wbamberg · 2018-05-31T03:14:47Z

Thanks for this, @rachelandrew . It helps a lot to have something concrete to look at. With this data I think it would be best to include 2 tables in the page, with calls like:

<h2 id="Browser_compatibility">Browser compatibility</h2>

<h3 id="Support_in_Flex_layout">Support in Flex layout</h3>

<p>{{Compat("css.properties.align-items.flex_context")}}</p>

<h3 id="Support_in_Grid_layout">Support in Grid layout</h3>

<p>{{Compat("css.properties.align-items.grid_context")}}</p>

That would give us a table looking like this:

I think this looks pretty clear. Note that this means the description fields are redundant, but I don't think they do any harm.

If we tried to include both in the same table I think it would get confusing when sub-sub-features like "first baseline and last baseline" are included:

I can't think of any compelling reason not to use this as a model for the Grid/Flex/Multicol properties. I'm not sure if it will work very well when and if we want to support "aggregate" tables, like a table of all the Grid properties. But I don't think that should stop us moving forward with this.

I'd like to hear if @Elchi3 also thinks this is a good approach though, as it is a bit unusual.

wbamberg

It looks like npm-debug.log was included in this PR by accident.

Other than this I think this approach looks great. I'd also like to hear Florian's take though.

wbamberg · 2018-05-31T03:16:13Z

css/properties/align-items.json

+              "description": "<code>first baseline</code> and <code>last baseline</code>",
+              "support": {
+                "webview_android": {
+                  "version_added": null


It would be really nice to change some of these null values to Yes/No even if the version number is not known.

rachelandrew · 2018-05-31T05:26:27Z

I like the idea to have the separate tables. That looks nice and clear.

ExE-Boss · 2018-05-31T06:02:00Z

@rachelandrew @wbamberg We should probably also do the same table separation for gap, row-gap and column-gap (which is still duplicated in grid-column-gap), as a follow-up to #1990.

rachelandrew · 2018-05-31T06:03:21Z

@ExE-Boss yes that's the intention (I'm currently working on restructuring the Box Alignment documentation).

Elchi3 · 2018-05-31T09:40:57Z

I also like the idea of separate tables. Great idea, @wbamberg 👍

I'm not sure if it will work very well when and if we want to support "aggregate" tables, like a table of all the Grid properties.

Not sure about this yet either, but "aggregate" tables are a whole separate thing we'll have to look at on a future day. I agree to not worry about it for now.

Elchi3 · 2018-05-31T13:58:55Z

So, this PR looks good to me, but unsure about whether there is more to come per the first comment

"if we can get this one figured out then I can go through all the rest."

If not, I'm happy to merge this and get it in today's release.

rachelandrew · 2018-05-31T14:15:40Z

This one stands alone, so lets publish this one and I'll work on the others doing an individual PR for each property if that makes sense? It will end up being a lot of files in one PR otherwise.

Elchi3

That makes sense to me and is easier to review :) Thanks a lot @rachelandrew 👍
I'm merging this now and it will get published later this evening to MDN prod.

Comments are addressed

* Separating Grid and Flex support for align-items * fixes to align-items as requested * moving description as requested * fixing indentation * removing debug log

Separating Grid and Flex support for align-items

6bf4151

Elchi3 added the data:css 🎨 Compat data for CSS features. https://developer.mozilla.org/docs/Web/CSS label May 25, 2018

ExE-Boss suggested changes May 26, 2018

View reviewed changes

wbamberg self-requested a review May 26, 2018 05:12

rachelandrew added 2 commits May 26, 2018 14:55

fixes to align-items as requested

96711d4

moving description as requested

f3ac4e4

ExE-Boss reviewed May 26, 2018

View reviewed changes

css/properties/align-items.json Outdated

"version_added": null

},

"version_added": "52"

},

Copy link

Contributor

ExE-Boss May 26, 2018

Choose a reason for hiding this comment

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

Wrong indentation

ExE-Boss mentioned this pull request May 26, 2018

Use yargs to parse the arguments passed to the linter #2155

Merged

fixing indentation

7f9c57c

wbamberg previously requested changes May 31, 2018

View reviewed changes

removing debug log

149008c

ExE-Boss mentioned this pull request May 31, 2018

Merge grid-column-gap into column-gap #2184

Merged

Elchi3 approved these changes May 31, 2018

View reviewed changes

Elchi3 merged commit 8531552 into mdn:master May 31, 2018

frenic mentioned this pull request Jun 7, 2018

Support multiple compatibility tables frenic/csstype#52

Closed

ddbeck mentioned this pull request Jul 26, 2018

Data justify self #2516

Merged

rachelandrew mentioned this pull request Aug 7, 2018

justify-items do not follow the schema #2568

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Separating Grid and Flex support for align-items #2156

Separating Grid and Flex support for align-items #2156

rachelandrew commented May 25, 2018

ExE-Boss May 26, 2018

rachelandrew May 26, 2018

wbamberg May 26, 2018

ExE-Boss May 26, 2018

ExE-Boss May 26, 2018 •

edited

Loading

ExE-Boss May 26, 2018 •

edited

Loading

ExE-Boss May 26, 2018 •

edited

Loading

ExE-Boss May 26, 2018 •

edited

Loading

ExE-Boss May 26, 2018 •

edited

Loading

ExE-Boss May 26, 2018

wbamberg commented May 31, 2018

wbamberg left a comment

wbamberg May 31, 2018

rachelandrew commented May 31, 2018

ExE-Boss commented May 31, 2018

rachelandrew commented May 31, 2018

Elchi3 commented May 31, 2018

Elchi3 commented May 31, 2018

rachelandrew commented May 31, 2018

Elchi3 left a comment

Separating Grid and Flex support for align-items #2156

Separating Grid and Flex support for align-items #2156

Conversation

rachelandrew commented May 25, 2018

ExE-Boss May 26, 2018

Choose a reason for hiding this comment

rachelandrew May 26, 2018

Choose a reason for hiding this comment

wbamberg May 26, 2018

Choose a reason for hiding this comment

ExE-Boss May 26, 2018

Choose a reason for hiding this comment

ExE-Boss May 26, 2018 • edited Loading

Choose a reason for hiding this comment

ExE-Boss May 26, 2018 • edited Loading

Choose a reason for hiding this comment

ExE-Boss May 26, 2018 • edited Loading

Choose a reason for hiding this comment

ExE-Boss May 26, 2018 • edited Loading

Choose a reason for hiding this comment

ExE-Boss May 26, 2018 • edited Loading

Choose a reason for hiding this comment

ExE-Boss May 26, 2018

Choose a reason for hiding this comment

wbamberg commented May 31, 2018

wbamberg left a comment

Choose a reason for hiding this comment

wbamberg May 31, 2018

Choose a reason for hiding this comment

rachelandrew commented May 31, 2018

ExE-Boss commented May 31, 2018

rachelandrew commented May 31, 2018

Elchi3 commented May 31, 2018

Elchi3 commented May 31, 2018

rachelandrew commented May 31, 2018

Elchi3 left a comment

Choose a reason for hiding this comment

ExE-Boss May 26, 2018 •

edited

Loading

ExE-Boss May 26, 2018 •

edited

Loading

ExE-Boss May 26, 2018 •

edited

Loading

ExE-Boss May 26, 2018 •

edited

Loading

ExE-Boss May 26, 2018 •

edited

Loading