- From: Gérard Talbot <css21testsuite@gtalbot.org>
- Date: Sat, 25 Oct 2014 14:34:31 -0400
- To: Manuel Rego Casasnovas <rego@igalia.com>
- Cc: public-css-testsuite@w3.org
Le 2014-10-22 12:19, Manuel Rego Casasnovas a écrit :
[snipped]
> one of the people with more experience in
> the W3C tests suite helping with the reviews.
> We understand that if this is not possible, as we have 3 people
> implementing this feature, we could review our own patches. But,
> initially it doesn't seem like the best approach.
> Rego
>
> [1] http://test.csswg.org/shepherd/search/spec/css-grid-1/
Rego,
I can not view the existing Grid Layout 1 tests right now with Shepherd.
I can make some comments on 1 test (randomly chosen). I understand you
are not the author of such test... and it appears that those tests were
based on an outdated version of the spec.
http://test.csswg.org/source/css-grid-1/grid-layout-repeat-notation.html
Pass-fail conditions sentence could be more meaningful. Something like:
Test passes if there are 5 short colored horizontal stripes touching
each other: from left to right: blue, yellow, orange, cyan and pink. The
pink stripe should be shorter than the other 4 colored stripes.
----
<meta name="assert" content="the subgrid layout should behave the same
as reference.">
That assert text is not helpful, nor a clear explanation of what the
test is trying to do.
----
line 10: body {
margin: 0;
padding: 0;
}
#caseTitle {
margin: 10px;
height: 40px;
line 17: }
Those 2 rules are not part of the test and those 2 rules are not needed,
not necessary and, in fact, are to be avoided.
body element's margins and padding are the same in all recent mainstream
browsers and, even if there were differences, then all you need to do is
to not declare those in the reference file; the default margin and
padding for body element of user agent style sheet would apply both in
the test and in the reference file.
Same thing with the pass-fail-conditions sentence: the default margin
and padding for p element of user agent style sheets of all recent
mainstream browsers is the same. So, no need to reset margin or height
of that <p>.
----
#grid {
width: 450px;
line 20: background: #eee;
Is there a need to use a background color for the grid? I believe there
is not. If such color was part of the pass conditions of the test or the
fail conditions of the test, then setting a background color would be
useful, needed; in such case, we would probably use green and red
colors.
----
line 49: <div class="a"> </div>
<div class="b"> </div>
<div class="c"> </div>
<div class="d"> </div>
<div class="e"> </div>
Class is for logical grouping of elements; id is for document-unique
elements; so here, just an id attribute is okay.
Also, it's always better to use meaningful, self-explanatory id
attribute value to help reviewing, help maintenance of tests. So,
<div id="blue-stripe"> </div>
<div id="yellow-stripe"> </div>
<div id="orange-stripe"> </div>
<div id="cyan-stripe"> </div>
<div id="pink-stripe"> </div>
or even
<div id="first-column"> </div>
<div id="second-column"> </div>
<div id="third-column"> </div>
<div id="fourth-column"> </div>
<div id="fifth-column"> </div>
Test authors should try to name elements with descriptive names that
helps understanding tests themselves.
Good identifiers are those which are
- descriptive with regards to the working logic of the testcase,
- descriptive of the design logic of the testcase or
- descriptive of the respective position in the containment hierarchy or
- descriptive of the respective position in the positioning hierarchy.
Identifiers should describe the role of an element inside the building
logic of a testcase.
----
http://test.csswg.org/source/css-grid-1/reference/grid-layout-repeat-notation-ref.html
In the reference file,
body {
margin: 0;
padding: 0;
}
#caseTitle {
margin: 10px;
height: 40px;
}
#grid {
width: 450px;
position: relative;
}
those 3 rules are not needed, not necessary and can be safely removed.
Gérard
--
CSS3 Writing modes section
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/
Received on Saturday, 25 October 2014 18:35:12 UTC