gridColumnStart CSS-in-JS prop does not work

derekhawker commented 3 months ago

I was trying to use gridColumnStart={2} on a CSS grid's item and found that it doesn't work. It's a valid CSS property so I would expect it to be supported. It's used to alter an item's default grid column positioning. There may be a similar problem with related props like gridColumnEnd, gridRowStart, and gridRowEnd.

There are two workaround for it. We can set style or use gridColumn shorthand.

I made a codesandbox to show the bug. In the first example, the Grid has an item in each column of each row. But the desired behaviour was for the 3rd grid item to display in the second column position, causing the 4th item to move to the first column on the next row. The second and third Grid examples show the workarounds I mentioned.

https://codesandbox.io/p/sandbox/agitated-morning-r92dgv?file=%2Fsrc%2Fapp.js

Minimal code example, in case that link isn't working

      <Grid templateColumns="1fr 1fr">
        <Box>One</Box>
        <Box>Two</Box>
        <Box gridColumnStart={2}>Three</Box>
        <Box>Four</Box>
      </Grid>

cheton commented 3 months ago

@derekhawker

According to the specifications defined on MDN Web Docs, the grid properties include the following:

Here is the current configuration in styled-system: https://github.com/trendmicro-frontend/tonic-ui/blob/master/packages/styled-system/src/config/grid.js

const config = {
  gridGap: {
    property: 'gridGap',
    scale: 'sizes',
  },
  gridColumnGap: {
    property: 'gridColumnGap',
    scale: 'sizes',
  },
  gridRowGap: {
    property: 'gridRowGap',
    scale: 'sizes',
  },
  gridColumn: true,
  gridRow: true,
  gridAutoFlow: true,
  gridAutoColumns: true,
  gridAutoRows: true,
  gridTemplateColumns: true,
  gridTemplateRows: true,
  gridTemplateAreas: true,
  gridArea: true,
};

We may need to add the following properties to the grid configuration. Please help check if any are missing:

{
  gridColumnEnd: true,
  gridColumnStart: true,
  gridRowEnd: true,
  gridRowStart: true,
  gridTemplate: true,
}

cheton commented 3 months ago

I just realized that grid-gap, grid-column-gap, and grid-row-gap have been renamed to gap, column-gap, and row-gap in CSS3.

According to MDN: https://developer.mozilla.org/en-US/docs/Web/CSS/gap

Early versions of the specification called this property grid-gap, and to maintain compatibility with legacy websites, browsers will still accept grid-gap as an alias for gap.

https://developer.mozilla.org/en-US/docs/Web/CSS/column-gap

Early versions of the specification called this property grid-column-gap, and to maintain compatibility with legacy websites, browsers will still accept grid-column-gap as an alias for column-gap.

https://developer.mozilla.org/en-US/docs/Web/CSS/row-gap

Early versions of the specification called this property grid-row-gap, and to maintain compatibility with legacy websites, browsers will still accept grid-row-gap as an alias for row-gap.

We can deprecate the use of grid-gap, grid-column-gap, and grid-row-gap in v2 or in the next major release. The new configuration might look like this:

const config = {
  gridArea: true,
  gridAutoColumns: true,
  gridAutoFlow: true,
  gridAutoRows: true,
  gridColumn: true,
  gridColumnEnd: true,
  gridColumnStart: true,
  gridRow: true,
  gridRowEnd: true,
  gridRowStart: true,
  gridTemplate: true,
  gridTemplateAreas: true,
  gridTemplateColumns: true,
  gridTemplateRows: true,

  // The following properties are renamed in CSS3 and will be deprecated or removed in the next major release
  gridGap: { // `gridGap` is an alias for `gap`
    property: 'gridGap',
    scale: 'sizes',
  },
  gridColumnGap: { // `gridColumnGap` is an alias for `columnGap`
    property: 'gridColumnGap',
    scale: 'sizes',
  },
  gridRowGap: { // `gridRowGap` is an alias for `rowGap`
    property: 'gridRowGap',
    scale: 'sizes',
  },
};

trendmicro-frontend / tonic-ui

gridColumnStart CSS-in-JS prop does not work #853