Re: [css-grid] grid-float-001, grid-inline-float-001, grid-clear-001 and grid-inline-clear-001: review request

Hi Gérard,

Thanks for the review.

On 03/01/15 21:10, Gérard Talbot wrote:
>> * grid-float-001:
>>   * Source:
>> https://github.com/mrego/csswg-test/blob/css-grid-float-clear-tests/css-grid-1/grid-model/grid-float-001.xht
>>
>>   * View:
>> https://rawgit.com/mrego/csswg-test/css-grid-float-clear-tests/css-grid-1/grid-model/grid-float-001.xht
>>
> 
> 1-
> The meta assert more or less repeats what is already given in the title
> element. So, you could skip the meta assert here.
> This issue has been happening in a few of your other tests too.

Yeah, you're right, I'll review it and remove from previous tests where
it's redundant too.

> 2-
> You may want to create a variant of such test, say, grid-float-002 where
> the code would be
> <div class="grid">
>             <div class="test-overlapping-green float-left"></div>
>             <div class="test-overlapping-green float-left"></div>
>         </div>
> 
> 3-
> You may also want to create another variant of such test where the
> left-floating box would have content, say,
> grid-float-003
> 
>   <meta content="ahem" name="flags" />
> 
> (...)
>             .grid {
>                 display: grid;
>                 font: 1.25em/2.5 Ahem; /* computes to 20px/50px */
>             }
> 
> .test-overlapping-green {
>                 background-color: green;
>             }
> 
>             .float-left {
>                 float: left;
>             }
> 
> (...)
> 
> <div class="grid">
>             <div class="test-overlapping-green float-left">FLOAT</div>
>             <div class="test-overlapping-green">NotFL</div>
>         </div>
> 
> 4-
> You may also want to create another variant of such test where the 2
> left-floating boxes have content, say,
> grid-float-004
> 
>   <meta content="ahem" name="flags" />
> 
> (...)
> 
> 
>             .grid {
>                 display: grid;
>                 font: 1.25em/2.5 Ahem; /* computes to 20px/50px */
>             }
> 
> .test-overlapping-green {
>                 background-color: green;
>             }
> 
>             .float-left {
>                 float: left;
>             }
> 
> (...)
> 
> <div class="grid">
>             <div class="test-overlapping-green float-left">FLOAT</div>
>             <div class="test-overlapping-green float-left">FLOAT</div>
>         </div>
> 
> 
> 5-
> You may also want to create another variant of such test where the 2
> left-floating boxes have not-filling content, say,
> grid-float-005
> 
>   <meta content="ahem" name="flags" />
> 
> (...)
> 
> 
>             .grid {
>                 display: grid;
>                 font: 1.25em/2.5 Ahem; /* computes to 20px/50px */
>             }
> 
> .test-overlapping-green {
>                 background-color: green;
>             }
> 
>             .float-left {
>                 float: left;
>             }
> 
> (...)
> 
> <div class="grid">
>             <div class="test-overlapping-green float-left">FL</div>
>             <div class="test-overlapping-green float-left">FLO</div>
>         </div>
> 
> 
> 6-
> You may also want to create another variant of such test where the 1 (or
> 2) left-floating box(es) have not-filling content, say,
> grid-float-006
> 
>             .grid {
>                 display: grid;
>                 font: 1.25em/2.5 Ahem; /* computes to 20px/50px */
>             }
> 
> .test-overlapping-green {
>                 background-color: green;
>             }
> 
>             .float-left {
>                 float: left;
>             }
> 
> (...)
> 
> <div class="grid">
>             <div class="test-overlapping-green float-left">FLO</div>
>             <div class="test-overlapping-green">NF</div>
>         </div>

Thanks for all the variant suggestions.

My plan is to try to have basic tests covering the whole spec as a first
step. Then in a second phase we could create variant tests to cover more
cases.

>> * grid-clear-001:
>>   * Source:
>> https://github.com/mrego/csswg-test/blob/css-grid-float-clear-tests/css-grid-1/grid-model/grid-clear-001.xht
>>
>>   * View:
>> https://rawgit.com/mrego/csswg-test/css-grid-float-clear-tests/css-grid-1/grid-model/grid-clear-001.xht
>>
> 
> If 'float' does not apply to grid and inline-grid elements, then how is
> it possible and realistic that 'clear' could (or possibly would) have a
> rendering effect on grid and inline grid elements? And so, why would
> there be a need or justification to test 'clear' property?

Again, you're right. It makes not sense to check "clear" when "float" is
not supported. So, I'll remove those tests for the moment.

Thanks,
  Rego

Received on Thursday, 8 January 2015 09:29:33 UTC