chinikes commented 4 years ago

Epic

Fix linting errors in the pages

Acceptance criteria

[ ] Zero linting errors on CSS pages

wbamberg commented 4 years ago

I ran the linter over the CSS tree today. A major caveat is that we don't have all the recipes defined yet, and we haven't set all the document tags yet. So we're not yet linting all the pages - in fact we see 478 "Recipe missing" errors out of 1007 pages. But even so, this is enough data to tell us some things about the CSS errors we have.

Below I've taken the error summaries and just grouped them by ingredient handler, and ordered them roughly by the number of errors, excluding "Recipe missing":

*data.examples*

EmbedLiveSample must be final node                                  550 errors     css-property/data.examples/nodes-after-live-sample                      
Expected h2#Examples for data.examples                              211 errors     css-property/data.examples/expected-heading                             
Expected h2#Examples for data.examples                              26 errors      css-shorthand-property/data.examples/expected-heading                   
Expected h2#Examples for data.examples                              4 errors       css-at-rule/data.examples/expected-heading                              
Expected h2#Examples for data.examples                              2 errors       css-selector/data.examples/expected-heading                             
No H3-demarcated examples found                                     7 errors       css-shorthand-property/data.examples/missing-example-h3                 
No H3-demarcated examples found                                     6 errors       css-at-rule/data.examples/missing-example-h3                            
No H3-demarcated examples found                                     91 errors      css-property/data.examples/missing-example-h3                           
No H3-demarcated examples found                                     1 error        css-selector/data.examples/missing-example-h3                           
EmbedLiveSample ID argument must match H3 heading ID                51 errors      css-property/data.examples/embedlivesample-id-mismatch                  
EmbedLiveSample ID argument must match H3 heading ID                4 errors       css-shorthand-property/data.examples/embedlivesample-id-mismatch        

*data.formal_definition*

Expected h2#Formal_definition for data.formal_definition            391 errors     css-property/data.formal_definition/expected-heading                    
Expected h2#Formal_definition for data.formal_definition            43 errors      css-shorthand-property/data.formal_definition/expected-heading          

*data.formal_syntax*

Expected h2#Formal_syntax for data.formal_syntax                    389 errors     css-property/data.formal_syntax/expected-heading                        
Expected h2#Formal_syntax for data.formal_syntax                    43 errors      css-shorthand-property/data.formal_syntax/expected-heading              
Expected h2#Formal_syntax for data.formal_syntax                    13 errors      css-at-rule/data.formal_syntax/expected-heading                         

*prose.see_also*

Expected h2#See_also for prose.see_also                             132 errors     css-property/prose.see_also/expected-heading                            
Expected h2#See_also for prose.see_also                             16 errors      css-shorthand-property/prose.see_also/expected-heading                  
Expected h2#See_also for prose.see_also                             3 errors       css-at-rule/prose.see_also/expected-heading                             

*data.specifications*

Expected h2#Specifications for data.specifications                  88 errors      css-property/data.specifications/expected-heading                       
Expected h2#Specifications for data.specifications                  15 errors      css-shorthand-property/data.specifications/expected-heading             
Expected h2#Specifications for data.specifications                  1 error        css-at-rule/data.specifications/expected-heading                        
Expected SpecName macro for data.specifications                     67 errors      css-property/data.specifications/expected-macro                         
Expected SpecName macro for data.specifications                     6 errors       css-shorthand-property/data.specifications/expected-macro               

*data.browser_compatibility*

Expected h2#Browser_compatibility for data.browser_compatibility    63 errors      css-property/data.browser_compatibility/expected-heading                
Expected h2#Browser_compatibility for data.browser_compatibility    7 errors       css-shorthand-property/data.browser_compatibility/expected-heading      
Expected h2#Browser_compatibility for data.browser_compatibility    1 error        css-at-rule/data.browser_compatibility/expected-heading                 
Expected Compat macro for data.browser_compatibility                18 errors      css-property/data.browser_compatibility/expected-macro                  
Expected Compat macro for data.browser_compatibility                3 errors       css-shorthand-property/data.browser_compatibility/expected-macro        

*data.constituent_properties*

Expected h2#Constituent_properties for data.constituent_properties  43 errors      css-shorthand-property/data.constituent_properties/expected-heading     

*ingredient-out-of-order*

data.specifications not expected in this order                      26 errors      css-property/ingredient-out-of-order/data.specifications                
data.specifications not expected in this order                      1 error        css-shorthand-property/ingredient-out-of-order/data.specifications      
prose.* not expected in this order                                  14 errors      css-property/ingredient-out-of-order/prose.*                            
prose.* not expected in this order                                  1 error        css-at-rule/ingredient-out-of-order/prose.*                             
prose.* not expected in this order                                  1 error        css-shorthand-property/ingredient-out-of-order/prose.*                  
data.examples not expected in this order                            11 errors      css-property/ingredient-out-of-order/data.examples                      
data.examples not expected in this order                            3 errors       css-shorthand-property/ingredient-out-of-order/data.examples            
data.examples not expected in this order                            1 error        css-at-rule/ingredient-out-of-order/data.examples                       
data.browser_compatibility not expected in this order               9 errors       css-property/ingredient-out-of-order/data.browser_compatibility         
data.formal_syntax not expected in this order                       1 error        css-at-rule/ingredient-out-of-order/data.formal_syntax                  
data.formal_syntax not expected in this order                       1 error        css-property/ingredient-out-of-order/data.formal_syntax     

*prose.syntax*

Expected h2#Syntax for prose.syntax                                 8 errors       css-property/prose.syntax/expected-heading                              
Expected h2#Syntax for prose.syntax                                 1 error        css-at-rule/prose.syntax/expected-heading                               

*data.interactive_example?*

Interactive example must be preceded by a P node                    7 errors       css-property/data.interactive_example?/interactive-example-preceded-by-p
Interactive example must be in a DIV                                5 errors       css-property/data.interactive_example?/interactive-example-inside-div   
Only one interactive example is allowed                             1 error        css-property/data.interactive_example?/at-most-one-interactive-example  

*prose.short_description*

Expected short description for prose.short_description              1 error        css-property/prose.short_description/missing-prose-section

I think we can classify them by the amount of effort to fix like:

Huge:
- data.examples
Large:
- prose.see_also
Medium:
- data.formal_definition
- data.formal_syntax
- data.specifications
- data.browser_compatibility
- ingredient-out-of-order
Small:
- data.constituent_properties
- prose.syntax
- data.interactive_example?
- prose.short_description

data.examples

The good news here is that although there are hundreds of errors, the linter seems to be over-sensitive here, sometimes generating dozens of errors for only a couple of underlying problems. For example, the box-shadow page generated 31 "data.examples/nodes-after-live-sample" errors for only a couple of underlying problems.

The bad news is that there are still a lot of errors and some of them will require careful thought and significant reworking to resolve.

prose.see_also

This is mostly about missing "See also" sections, and involves real work thinking of good "See also" links and adding them. As discussed before, if it starts to look like we are artificially adding links just to satisfy the linter, we could make this ingredient optional. But I think adding cross-linking like this adds real benefits to users.

data.formal_definition

This seems like a lot because there are lots of errors. We want to add a new H2 section to every property page (and some other pages too). But the new sections are very simple and consistent, so it should be quite quick, predictable, and parallelizable.

data.formal_syntax

Same as data.formal_definition. We should do this and data.formal_definition together.

data.specifications

This looks like a lot but I think a lot is coming from misnamed "Specifications" sections. For example it looks like all the padding-block-*, padding-inline-*, and scroll-margin-* pages use "Specification".

data.browser_compatibility

Again it looks like lots of these are subtly misnamed sections, often where the ID is "Browser_Compatibility".

ingredient-out-of-order

This is a hard one to predict, and we could dig into it more.

data.constituent_properties

This is another brand-new section, only applies to a few pages, and is pretty mechanical.

data.syntax

There are only 9 of these errors in all 500-odd pages, so it can't be that bad.

data.interactive_example?

There are only 13 of these errors in all 500-odd pages, so it can't be that bad.

prose.short_description

Only one of these. Probably quicker to fix than to write this sentence.

wbamberg commented 4 years ago

I ran the linter over the CSS tree again today. It now recognizes all except 4 pages (the 4 "keyword" pages).

I've split the output by ingredients, to mirror the user stories under this epic, and dumped them in a spreadsheet: https://docs.google.com/spreadsheets/d/1JQIgRB978bEKHFmQoAjY4stddDdbaFAULj_TOK2jYwc/edit?usp=sharing. This has one sheet per ingredient, and each sheet has one row per page that contains at least one error about that ingredient. Some pages have multiple errors per page.

ddbeck commented 4 years ago

@wbamberg two questions about fixing CSS page errors, hopefully uncomplicated:

Should I remove fixed pages from the spreadsheet? I fixed one (and removed it) but then I thought better of it—maybe you wanted a record of what needed to be fixed.
For examples specifically, do you know of any CSS pages which demonstrate the form particularly good well? I know how to satisfy the linter, but that's not necessarily a marker of quality.

wbamberg commented 4 years ago

@wbamberg two questions about fixing CSS page errors, hopefully uncomplicated:

Should I remove fixed pages from the spreadsheet? I fixed one (and removed it) but then I thought better of it—maybe you wanted a record of what needed to be fixed.

I'm not really sure about the general role of the spreadsheet here tbh, and I don't have a fully-baked process for how people can coordinate their efforts to fix errors. I guess you could say, this spreadsheet is a tool people might find useful, not a process they have to follow.

But, I think people should mark them as done and leave them in the spreadsheet. The main reason being that otherwise we can't easily distinguish between a page not appearing because it was missed in the analysis, and a page not appearing because it was fixed.

For examples specifically, do you know of any CSS pages which demonstrate the form particularly good well? I know how to satisfy the linter, but that's not necessarily a marker of quality.

Good question! The linter is actually fairly relaxed about examples, isn't it. A good consistent format might be:

<h3>A meaningful title</h3>

<p>Some descriptive prose.</p>

<p>Which can include all kinds of markup.</p>

<h4>HTML</h4> (omitted if no HTML is present or if HTML is hidden)

<pre>...the HTML source</pre> (optionally hidden)

<h4>CSS</h4> (omitted if no CSS is present is present or if CSS is hidden)

<pre>...the CSS source</pre> (optionally hidden)

<h4>JavaScript</h4> (omitted if no JavaScript is present is present or if JavaScript hidden)

<pre>...the JavaScript source</pre> (optionally hidden)

<h4>Result</h4>

{{EmbedLiveSample("H3 id")}}

But it's intentional in the linter specification that it allows people to interleave prose and code. We discussed this back in https://github.com/mdn/stumptown-content/issues/350.

wbamberg commented 4 years ago

So https://wiki.developer.mozilla.org/en-US/docs/Web/CSS/box-shadow#Examples could be a good example of a page with good examples :). (also, a magnificent quote in the first of them).

wbamberg commented 4 years ago

I've filed https://github.com/mdn/sprints/issues/3491 for deciding what to do about prefixed features.

wbamberg commented 4 years ago

On a similar note to https://github.com/mdn/sprints/issues/3491, what about properties associated with the old flexbox draft specification?

https://wiki.developer.mozilla.org/en-US/docs/Web/CSS/box-direction https://wiki.developer.mozilla.org/en-US/docs/Web/CSS/box-flex https://wiki.developer.mozilla.org/en-US/docs/Web/CSS/box-flex-group https://wiki.developer.mozilla.org/en-US/docs/Web/CSS/box-lines https://wiki.developer.mozilla.org/en-US/docs/Web/CSS/box-ordinal-group https://wiki.developer.mozilla.org/en-US/docs/Web/CSS/box-orient https://wiki.developer.mozilla.org/en-US/docs/Web/CSS/box-pack

? Do we provide any value by documenting these properties? Does anyone use them? @rachelandrew , would you mind venturing an opinion?

ddbeck commented 4 years ago

@wbamberg I finished up the fixable data.formal_definition and data.formal_syntax pages today. Some conclusions after rerunning the linter against all the CSS pages:

There are six pages which still produce errors (for data.formal_*). They're a subset of the old flexbox property pages you mentioned above, plus the related -moz-box-ordinal-group which is related and the font-smooth page which is not. I've created filter views for both sheets to quickly find these incomplete pages.
There are 13 pages which have actionable notes on the spreadsheet. I'd suggest user stories to deal with these items in the next sprint. They fall into two main buckets: proposals to archive (the old flexbox stuff and -moz-binding) or missing mdn/data ("Value not found in DB!"). I've created filter views for both sheets to find the entries with notes as well.

wbamberg commented 4 years ago

@wbamberg I finished up the fixable data.formal_definition and data.formal_syntax pages today.

Thanks so much for doing this work Daniel! I'll comment on the other stuff over in https://github.com/mdn/sprints/issues/3324.

a2sheppy commented 4 years ago

I've finished the expected-heading Syntax items.

mdn / sprints

E - Fix CSS linter errors #3139

Epic

Acceptance criteria

data.examples

prose.see_also

data.formal_definition

data.formal_syntax

data.specifications

data.browser_compatibility

ingredient-out-of-order

data.constituent_properties

data.syntax

data.interactive_example?

prose.short_description