Add basic support for CSS grid prefixing #174

etripier · 2019-03-03T04:31:34Z

This PR implements very basic style prefixing for CSS grid. This is the first step to solving #173.

First, here's what I added:

Prefixing for display: grid | inline-grid
Prefixing / expansion for grid-column | grid-column-start | grid-column-end | grid-row | grid-row-start | grid-row-end
Prefixing for align-self and justify-self
Prefixing for grid-template-columns | grid-template-rows

Here's what I did not add (beyond the properties not listed above, obviously):

Support for span values
repeat conversion (e.g., repeat(2, 1fr) -> (1fr)[2])
Auto-placement support
Support for negative values in grid-column and grid-row (see caveats at https://css-tricks.com/css-grid-in-ie-css-grid-and-the-new-autoprefixer/)

This provides basic support for IE10/11 and early versions of Edge. The rest is difficult to add given the limited ability of this program to set styles on child elements (a general limitation of style composability imposed by React). I think it's possible to convert the repeat syntax and add basic span support, but that looks like it may be reaching beyond the scope of this program.

Ah! One last note: I noticed that the grid values in propertyMap.js may be generating invalid styles. Should I try to trim that down?

etripier · 2019-03-03T04:35:36Z

@garrettberg @ljharb @lencioni @calinoracation Hey there! Would you take a look whenever you have a chance? 🙏

lencioni · 2019-03-04T16:54:13Z

modules/__tests__/createPrefixer-test.js

+          const output = {
+            gridTemplateColumns: '1fr auto',
+            msGridColumns: '1fr auto',
+            msGridTemplateColumns: '1fr auto', // Not a valid property


What do you mean by the comment here?

The list of properties defined here causes this prefix to be automatically generated, even if it isn't valid (to the best of my knowledge). I don't have full context on why this is generated, but I think it could be removed - just waiting to hear back for confirmation.

lencioni · 2019-03-04T16:56:05Z

modules/__tests__/createPrefixer-test.js

+
+      describe('template', () => {
+        it('should prefix column templating', () => {
+          const input = { gridTemplateColumns: '1fr auto' }


Can you add some test cases for named grid areas?(e.g. https://vgpena.github.io/using-css-grid-the-right-way/#use-names-not-numbers)

lencioni · 2019-03-04T16:58:49Z

modules/plugins/grid.js

+  return typeof value === 'number' && !isNaN(value)
+}
+
+const alignmentValues = ['center', 'end', 'start', 'stretch']


I wonder if it is worth using something that has constant time lookup for this? I don't see Set used anywhere though, but an object should do the trick. n=4 though, so maybe it doesn't really matter.

It definitely shouldn't matter, but an object would be fine too i suppose.

lencioni · 2019-03-04T17:00:05Z

modules/plugins/grid.js

+/* @flow */
+
+function isSimplePositionValue(value: any) {
+  return typeof value === 'number' && !isNaN(value)


Can we use Number.isNaN instead of this function? https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/isNaN

why? isNaN is available in more places, and the typeof ensures it'll behave identically.

Yeah we came full circle on this one

…d grid lines

etripier · 2019-03-04T19:03:02Z

@lencioni Hey Joe, thanks for the comments! I went ahead and addressed them.

lencioni · 2019-03-04T20:32:06Z

modules/plugins/grid.js

@@ -1,10 +1,10 @@
 /* @flow */

 function isSimplePositionValue(value: any) {
-  return typeof value === 'number' && !isNaN(value)
+  return typeof value === 'number' && !Number.isNaN(value)


Actually, I wonder if Number.isFinite is what you want here.

Also, I noticed that IE does not support either, so maybe what you originally had would be better in the end?

@lencioni I went back to isNaN and removed the use of Set because I don't think IE11 supports constructing one from an iterable.

etripier · 2019-03-07T20:09:34Z

@rofrischmann Hey Robin! I'm trying to add basic support for CSS grid. Would you please take a look when you have a chance?

robinweser · 2019-03-07T23:22:10Z

Sure! Ill check it once I wake up tomorrow! Is it ready yet? Wanna get it merged and releasd asap :p

calinoracation

This looks fantastic!

etripier · 2019-03-08T17:43:48Z

It's ready! It doesn't fully cover the grid spec, but it adds enough basic compatibility that you can make things work on IE10/11. I might try to add more advanced features in other PRs.

etripier · 2019-03-14T05:07:25Z

@rofrischmann Hey there, would you please take a look when you have time? I think this is ready to go.

robinweser · 2019-03-14T07:26:45Z

About the propertyMap.js, did you remove that or do you think we should do it? After all, it should just add prefixes to some values, so it won't break anything. But, it might be unnecessary as well.

etripier · 2019-03-15T03:56:24Z

@rofrischmann I think we can probably take it out. I took a look at the older IE spec and some MDN data and came up with this:

gridTemplateColumns - `-ms-grid-columns` is used instead
gridTemplateRows    - `-ms-grid-rows` is used instead
gridTemplateAreas   - not supported in IE
gridTemplate        - not supported in IE
gridAutoColumns     - `-ms-grid-columns` is used instead
gridAutoRows        - `-ms-grid-rows` is used instead
gridAutoFlow        - not supported in IE
grid                - not supported in IE
gridRowStart        - `-ms-grid-row` is used instead (not a 1-1 match)
gridColumnStart     - `-ms-grid-column` is used instead (not a 1-1 match)
gridRowEnd          - `-ms-grid-row-span` is used instead (not a 1-1 match)
gridRow             - not a 1-1 match, implemented in the PR
gridColumn          - not a 1-1 match, implemented in the PR
gridColumnEnd       - `-ms-grid-column-span` is used instead (not a 1-1 match)
gridColumnGap       - not supported in IE
gridRowGap          - not supported in IE
gridArea            - not supported in IE
gridGap             - not supported in IE

lencioni · 2019-03-17T20:02:10Z

robinweser · 2019-03-28T21:35:03Z

Sorry for the delay. I had some unpredictable incidents in my family that required my full attention.
I'll try to merge and release this one asap. Probably on monday or tuesday.

robinweser

Alright, LGTM! Great work all of you, thank you =)

robinweser · 2019-04-04T09:20:27Z

Weeeh, its out at 5.1.0 :P

etripier · 2019-04-05T01:20:23Z

Thank you so much! 🙏

kevinweber · 2020-01-02T10:26:34Z

modules/__tests__/createPrefixer-test.js

+        })
+
+        it('should not expand non-numerical values', () => {
+          const input = { gridRow: '2 / span 3' }


@lencioni What's the reason why we wouldn't want 2 / span 3 to expand?

Add basic support for CSS grid prefixing

4a08e39

lencioni reviewed Mar 4, 2019

View reviewed changes

Use Set for constant-time lookup, use Number.isNaN, add test for name…

0c2fc79

…d grid lines

lencioni reviewed Mar 4, 2019

View reviewed changes

etripier added 2 commits March 4, 2019 12:39

Fix IE11 compatibility

cb00c1f

IE doesn't support Array#includes

30dd7f2

calinoracation reviewed Mar 8, 2019

View reviewed changes

Remove propertyMap grid values

ee494d6

robinweser self-requested a review April 4, 2019 08:42

robinweser approved these changes Apr 4, 2019

View reviewed changes

robinweser merged commit 0bc1284 into robinweser:master Apr 4, 2019

kevinweber reviewed Jan 2, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add basic support for CSS grid prefixing #174

Add basic support for CSS grid prefixing #174

etripier commented Mar 3, 2019

etripier commented Mar 3, 2019

lencioni Mar 4, 2019

etripier Mar 4, 2019

lencioni Mar 4, 2019

lencioni Mar 4, 2019

ljharb Mar 5, 2019

lencioni Mar 4, 2019

ljharb Mar 5, 2019

lencioni Mar 5, 2019

etripier commented Mar 4, 2019

lencioni Mar 4, 2019

etripier Mar 4, 2019

etripier commented Mar 7, 2019

robinweser commented Mar 7, 2019

calinoracation left a comment

etripier commented Mar 8, 2019

etripier commented Mar 14, 2019

robinweser commented Mar 14, 2019 •

edited

Loading

etripier commented Mar 15, 2019 •

edited

Loading

lencioni commented Mar 17, 2019

robinweser commented Mar 28, 2019

robinweser left a comment •

edited

Loading

robinweser commented Apr 4, 2019

etripier commented Apr 5, 2019

kevinweber Jan 2, 2020

Add basic support for CSS grid prefixing #174

Add basic support for CSS grid prefixing #174

Conversation

etripier commented Mar 3, 2019

etripier commented Mar 3, 2019

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

etripier commented Mar 4, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etripier commented Mar 7, 2019

robinweser commented Mar 7, 2019

calinoracation left a comment

Choose a reason for hiding this comment

etripier commented Mar 8, 2019

etripier commented Mar 14, 2019

robinweser commented Mar 14, 2019 • edited Loading

etripier commented Mar 15, 2019 • edited Loading

lencioni commented Mar 17, 2019

robinweser commented Mar 28, 2019

robinweser left a comment • edited Loading

Choose a reason for hiding this comment

robinweser commented Apr 4, 2019

etripier commented Apr 5, 2019

Choose a reason for hiding this comment

robinweser commented Mar 14, 2019 •

edited

Loading

etripier commented Mar 15, 2019 •

edited

Loading

robinweser left a comment •

edited

Loading