[css-grid] Fix/row column span inheritance bug

[bogdan0083]

Fixes #1146

/cc @Dan503

🙃

[Dan503]

Woo hoo! 😃

I'll test it out tomorrow night 😁

[Dan503]

Actually I'm wondering if it makes more sense to only apply the span if the default one spans.

If the default one spans then all the others inherit it.

If the other ones span, theoretically no other grid cells will inherit it.

[bogdan0083]

Actually I'm wondering if it makes more sense to only apply the span if the default one spans.

I'm not sure if I understood you correctly, but I guess how you described it is the way it works now.

If the default one doesn't have row/column span values, the duplicate rules will have span properties only if needed.

I would be happy if you show me an example of what you mean :)

[Dan503]

.content {
  -ms-grid-row: 1;
  -ms-grid-row-span: 2; /* <-- rowspan output here */
  -ms-grid-column: 2;
  grid-area: content;
}
.double-sidebar > .content {
  -ms-grid-row: 1;
  -ms-grid-row-span: 1; /* <-- row span is output to override the other one */
  -ms-grid-column: 2;
}

But if it is the other way around.

.content {
  -ms-grid-row: 1;
  /* <-- no rowspan needed here */
  -ms-grid-column: 2;
  grid-area: content;
}
.single-sidebar > .content {
  -ms-grid-row: 1;
  -ms-grid-row-span: 2; /* <-- row span is output here */
  -ms-grid-column: 2;
}

Yeah so it only needs to be output if the default one is the one that has a row span.

[bogdan0083]

But if it is the other way around.

.content {
  -ms-grid-row: 1;
  /* <-- no rowspan needed here */
  -ms-grid-column: 2;
  grid-area: content;
}
.single-sidebar > .content {
  -ms-grid-row: 1;
  -ms-grid-row-span: 2; /* <-- row span is output here */
  -ms-grid-column: 2;
}

Yeah so it only needs to be output if the default one is the one that has a row span.

You have .single-sidebar > .content. That's a typo, right? It should be .double-sidebar > .content like in your first example above

[Dan503]

No it's the difference between using this input CSS

.content {
  grid-area: content;
}

/* layout.scss */
.single-sidebar {
  display: grid;
  grid-template:
    "nav       content" 1fr
    "subscribe content" 1fr /
     1fr       2fr;
}

.double-sidebar {
  display: grid;
  grid-template:
    "nav    content   sidebar" 1fr
    "nav    subscribe sidebar" 1fr /
     1fr     2fr       1fr;
}

And this input CSS

.content {
  grid-area: content;
}

/* layout.scss */
.double-sidebar {
  display: grid;
  grid-template:
    "nav    content   sidebar" 1fr
    "nav    subscribe sidebar" 1fr /
     1fr     2fr       1fr;
}

.single-sidebar {
  display: grid;
  grid-template:
    "nav       content" 1fr
    "subscribe content" 1fr /
     1fr       2fr;
}

[bogdan0083]

Alright, it is all clear to me now, thanks for clarification!

I checked the output for both inputs and the results are the exact you described:

.content {
  -ms-grid-row: 1;
  -ms-grid-row-span: 2; /* <-- rowspan output here */
  -ms-grid-column: 2;
  grid-area: content;
}
.double-sidebar > .content {
  -ms-grid-row: 1;
  -ms-grid-row-span: 1; /* <-- row span is output to override the other one */
  -ms-grid-column: 2;
}

But if it is the other way around.

.content {
  -ms-grid-row: 1;
  /* <-- no rowspan needed here */
  -ms-grid-column: 2;
  grid-area: content;
}
.single-sidebar > .content {
  -ms-grid-row: 1;
  -ms-grid-row-span: 2; /* <-- row span is output here */
  -ms-grid-column: 2;
}

Yeah so it only needs to be output if the default one is the one that has a row span.

Now I'll wait for your testing, and we can proceed to code review and merge! 😄

[Dan503]

Sorry that I haven't tested yet. I've had a ridiculously busy weekend.

I should be able to test tomorrow night.

[Dan503]

The code seems pretty solid. I wasn't able to break it. Nice job @bogdan0083 😊

@ai This one is ready for your review and merge 😁

I think after this issue is merged we can release a patch release. 😊

The patch release would feature fixes for issue #1143 and and this one.

[Dan503]

I've tested this issue now. I wasn't able to break it. Nice job @bogdan0083 😊

@ai this is ready for review and merge now.

Once merged I think Autoprefixer can release a new patch release with the fixes for this issue and issue #1143.

[Dan503]

Sorry about the spam mail. GitHub bugged out on me and wouldn't submit the comment :(

[bogdan0083]

Yeah. Github is lagging quite heavily today. 😕

[ai]

Released in 9.3