Complete flexbox support for Chrome #1945

frenic · 2018-04-25T12:07:17Z

https://www.chromestatus.com/feature/4837301406400512
https://www.chromestatus.com/feature/6600926009753600
https://caniuse.com/#feat=flexbox

ExE-Boss

The following applies to all files:

review?(@jpmedley)

ExE-Boss · 2018-04-25T12:50:57Z

css/properties/align-items.json

+            "chrome": [
+              {
+                "version_added": "52",
+                "notes": "Older versions of the specification treat absolute positioned children as though they are a 0 by 0 flex item. Later specification versions take the children out of the flow and set their positions based on align and justify properties. Chrome implements the new behavior beginning with Chrome 52."


This should be moved to the end of the "version_added": "29" block.

ExE-Boss · 2018-04-25T12:54:42Z

css/properties/align-items.json

+                "notes": "Older versions of the specification treat absolute positioned children as though they are a 0 by 0 flex item. Later specification versions take the children out of the flow and set their positions based on align and justify properties. Chrome implements the new behavior beginning with Chrome 52."
+              },
+              {
+                "version_added": "29"


This should be followed by "partial_implementation": true, eg:

{ "version_added": "29", "partial_implementation": true, "notes": "…" // Contains the text from above }

ExE-Boss · 2018-04-25T12:56:27Z

css/properties/align-items.json

@@ -8,10 +8,19 @@
            "webview_android": {
              "version_added": "4.4"
            },


webview_android should be the same as chrome and be the last in the support block (see issue #398 and PR #1882)

I agree that it should be in alphabetical order. But I'm not convinced that webview_android should be the same as chrome? According to caniuse the full spec was implemented in 4.4.

Or did you mean the same as chrome but with "version_added": "4.4" instead of "version_added": "29"?

Chrome Developer Relations here. He means that Android webview follows Chrome version numbering, not Android version numbering.

Oh, I see. Didn't know. Thanks

ExE-Boss · 2018-04-25T12:56:54Z

css/properties/align-items.json

+                "prefix": "-webkit-",
+                "version_added": "21"
+              }
+            ],
            "chrome_android": {
              "version_added": null
            },


chrome_android should be the same as chrome.

ExE-Boss

There are a few more things that require attention.

Also, please don’t amend the commit, and instead make additional commits implementing the requested changes, as this makes reviewing the PR easier.

ExE-Boss · 2018-04-25T13:54:11Z

css/properties/align-items.json

-            "chrome_android": {
-              "version_added": null
-            },
+            "chrome": [


The first element of the chrome and chrome_android support array should be:

{ "version_added": "52" }

followed by the current contents of this array.
This is because Chrome 52 now implements this fully.

Fixed! Also added this for justify-content and order until @jpmedley has confirmed.

Here's the full list of what's supported in Chrome: https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/css/CSSValueKeywords.json5?l=520

Sorry, I don't quite understand that to do with this.

justify-content is supported on Chromium browsers as is anything else not commented out of that list. Were you not asking me to confirm the status of justify-content in Chrome?

Oh, my bad. The comment was poorly formulated. And it concerned align-self, not justify-content. What I meant to say was that added the note about absolute positioned flex children for align-self too after reading this:

However, the latest version of the spec takes them fully out of flow and sets the static position based on align and justify properties.

And I was wondering if that would be correct? Does that changed behavior affect the order property as well?

It doesn't appear so.

Sorry about that link. It was supposed to link to align-self. Shortly after I posed it, a new version of the source file got merged to trunk which made the line number wrong.

Ok, thanks 👍

ExE-Boss · 2018-04-25T13:54:22Z

css/properties/align-items.json

+                "version_added": "21"
+              }
+            ],
+            "chrome_android": [


ExE-Boss · 2018-04-25T13:55:02Z

css/properties/align-self.json

@@ -10,16 +10,26 @@
            },
            "chrome": [
              {
-                "version_added": "36"


ditto, but with "version_added": "36"

ExE-Boss · 2018-04-25T13:56:18Z

css/properties/justify-content.json

@@ -12,16 +12,25 @@
            "chrome": [
              {
                "version_added": "29",
-                "notes": "Older versions of the specification treat absolutely positioned children as though they are 0 by 0 flex items. Later versions of the specification take them out of the flow and set their positions based on align and justify properties. Versions 52 and later implement the new behavior."
+                "partial_implementation": true,
+                "notes": "Older versions of the specification treat absolute positioned children as though they are a 0 by 0 flex item. Later specification versions take the children out of the flow and set their positions based on align and justify properties. Chrome implements the new behavior beginning with Chrome 52."


@jpmedley Can you confirm this?

It is correct. https://www.chromestatus.com/feature/6600926009753600

ExE-Boss · 2018-04-25T13:56:55Z

css/properties/order.json

-                "version_added": "29"
+                "version_added": "29",
+                "partial_implementation": true,
+                "notes": "Older versions of the specification treat absolute positioned children as though they are a 0 by 0 flex item. Later specification versions take the children out of the flow and set their positions based on align and justify properties. Chrome implements the new behavior beginning with Chrome 52."


jpmedley · 2018-04-25T14:33:43Z

css/properties/align-items.json

@@ -8,10 +8,19 @@
            "webview_android": {
              "version_added": "4.4"
            },


Chrome Developer Relations here. He means that Android webview follows Chrome version numbering, not Android version numbering.

jpmedley · 2018-04-25T14:34:33Z

css/properties/align-items.json

-            "chrome_android": {
-              "version_added": null
-            },
+            "chrome": [


Here's the full list of what's supported in Chrome: https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/css/CSSValueKeywords.json5?l=520

jpmedley · 2018-04-25T14:37:12Z

css/properties/align-items.json

+                "notes": "Older versions of the specification treat absolute positioned children as though they are a 0 by 0 flex item. Later specification versions take the children out of the flow and set their positions based on align and justify properties. Chrome implements the new behavior beginning with Chrome 52."
+              },
+              {
+                "prefix": "-webkit-",


@Elchi3 Do you have a preference on whether 'webkit' includes dashes. I just grepped my local repo and found we're pretty inconsistent.

Is that in the css tree or the api tree?

The css tree should contain dashes, whereas the api tree should not (see the Vendor Prefix Glossary page for more information).

I did not know that. (I'm only every removing prefixes.) The css/ folder appears to be consistent. The api/ folder is not. I didn't check the others.

What @ExE-Boss says is correct. Not sure if we could validate some more automatically there.

frenic · 2018-04-25T18:20:01Z

Modified webview_android
Noticed that I did a mistake and modified the wrong flex.json at some point. Reverted that.
Removed the note from order because it said align and justify properties.

However, the latest version of the spec takes them fully out of flow and sets the static position based on align and justify properties.
~~Not sure what to do with this?~~

~~Here's the full list of what's supported in Chrome: https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/css/CSSValueKeywords.json5?l=520~~

frenic · 2018-04-26T16:32:54Z

@ExE-Boss have I missed anything?

ExE-Boss

LGTM, r+

* Complete flexbox support for Chrome * Add version for full Chrome implementation of flexbox * Follow Chrome version for flexbox in Webview * Revert Flex type * Remove partial implementation from `order`

This reverts commit 55a5dbb.

ExE-Boss suggested changes Apr 25, 2018

View reviewed changes

frenic force-pushed the flexbox-support branch from c3e74ef to 129517d Compare April 25, 2018 13:24

Complete flexbox support for Chrome

5f5619d

frenic force-pushed the flexbox-support branch from 129517d to 5f5619d Compare April 25, 2018 13:30

ExE-Boss suggested changes Apr 25, 2018

View reviewed changes

Add version for full Chrome implementation of flexbox

719f41e

jpmedley suggested changes Apr 25, 2018

View reviewed changes

frenic added 3 commits April 25, 2018 20:12

Follow Chrome version for flexbox in Webview

1ea592a

Revert Flex type

8aaa64c

Remove partial implementation from order

586c04c

frenic force-pushed the flexbox-support branch from c913558 to 586c04c Compare April 25, 2018 18:12

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

ExE-Boss approved these changes Apr 26, 2018

View reviewed changes

jpmedley approved these changes Apr 27, 2018

View reviewed changes

ddbeck merged commit 01c2b6d into mdn:master Apr 27, 2018

Elchi3 added a commit that referenced this pull request Aug 14, 2018

Revert "Complete flexbox support for Chrome (#1945)"

bd4d8b6

This reverts commit 55a5dbb.

Complete flexbox support for Chrome #1945

Complete flexbox support for Chrome #1945

Conversation

frenic commented Apr 25, 2018

ExE-Boss left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ExE-Boss left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

frenic Apr 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ExE-Boss Apr 25, 2018 • edited Loading

Choose a reason for hiding this comment

ExE-Boss Apr 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ExE-Boss Apr 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ExE-Boss Apr 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

frenic commented Apr 25, 2018 • edited Loading

frenic commented Apr 26, 2018

ExE-Boss left a comment • edited Loading

Choose a reason for hiding this comment

frenic Apr 26, 2018 •

edited

Loading

ExE-Boss Apr 25, 2018 •

edited

Loading

ExE-Boss Apr 25, 2018 •

edited

Loading

ExE-Boss Apr 25, 2018 •

edited

Loading

ExE-Boss Apr 25, 2018 •

edited

Loading

frenic commented Apr 25, 2018 •

edited

Loading

ExE-Boss left a comment •

edited

Loading