Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a template for CSS functional notation #28953

Merged
merged 9 commits into from
Sep 20, 2023

Conversation

wbamberg
Copy link
Collaborator

@wbamberg wbamberg commented Sep 5, 2023

We don't have a template for CSS functions (or CSS types, see #26881), and the way we document them is quite inconsistent:

https://developer.mozilla.org/en-US/docs/Web/CSS/basic-shape/polygon
https://developer.mozilla.org/en-US/docs/Web/CSS/transform-function/matrix
https://developer.mozilla.org/en-US/docs/Web/CSS/sign
https://developer.mozilla.org/en-US/docs/Web/CSS/min
https://developer.mozilla.org/en-US/docs/Web/CSS/pow
https://developer.mozilla.org/en-US/docs/Web/CSS/var

Some choices I've made here:

  1. the syntax example only includes the function itself, not a complete declaration (i.e. https://developer.mozilla.org/en-US/docs/Web/CSS/var#syntax , not https://developer.mozilla.org/en-US/docs/Web/CSS/pow#syntax
  2. uses "Parameters" and "Return value", not "Values" (i.e. https://developer.mozilla.org/en-US/docs/Web/CSS/pow#parameters, not https://developer.mozilla.org/en-US/docs/Web/CSS/var#values)

The reasoning for 1 is that including declarations leads us into conversations like #28873 (review), and I wonder if this will be confusing when we inevitably omit some of the properties that the function can provide a value for.

Happy to discuss though. @yarusome , what do you think?

@wbamberg wbamberg requested a review from a team as a code owner September 5, 2023 19:28
@wbamberg wbamberg requested review from Elchi3 and removed request for a team September 5, 2023 19:28
@github-actions github-actions bot added the Content:Meta Content in the meta docs label Sep 5, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2023

@yarusome
Copy link
Contributor

yarusome commented Sep 6, 2023

The overall structure presented in this template looks good, but I have one explicit usage doubt: For oklch(), where should the invocation pattern oklch(L C H[/ A]) go?

In the template, the "Syntax" section first lists example uses of the functions, then lists paramaters in the "Parameters" subsection, but parameters are usually order-dependent, and there is no easy way to indicate if any literal tokens like commas and slashes are needed in between.

Comparing to JSON.stringify(), the "Syntax" section there is used to show the invocation patterns. So the issue lies in the different roles of the "Syntax" sections on CSS and JavaScript pages.

A smaller issue is that, I think, example uses in the "Syntax" section should not have a comma at the line end as they are not complete value declarations. JavaScript pages don't have such commas, either.


Maybe the "Syntax" section can show the pattern in comments:

/* var( <custom-property-name> , <declaration-value>? ) */

/* Without a fallback */
var(--custom-prop)

/* With fallback */
var(--custom-prop,) /* empty value as fallback */
var(--custom-prop, initial) /* initial value of the property as fallback */
var(--custom-prop, #FF0000)
var(--my-background, linear-gradient(transparent, aqua), pink)
var(--custom-prop, var(--default-value))
var(--custom-prop, var(--default-value, red))

Then custom-property-name and declaration-value are listed as paramters in the "Parameter" subsection.

A few notes:

  1. The pattern in the comment can be in a form that better explains the production in the spec, e.g. for oklch():

    /* oklch( <L> <C> <H> [/ <A> ]? ) */

    (There could be a linter rule for space normalization.)

  2. The comment showing each invocation pattern should be followed by exactly one empty line. (Some color functions have legacy invocation patterns, but fortunately they're demoted now.

  3. Any comment at the end of line that contains an example use should be preceded by exactly one space. (This could be done by linter or Prettier?)

An alternative format is to list several patterns to reduce the use of quantifiers:

/* Without a fallback */

/* var( <custom-property-name> ) */
var(--custom-prop)

/* With fallback */

/* var( <custom-property-name> , ) */
var(--custom-prop,) /* empty value as fallback */

/* var( <custom-property-name> , <declaration-value> ) */
var(--custom-prop, initial) /* initial value of the property as fallback */
var(--custom-prop, #FF0000)
var(--my-background, linear-gradient(transparent, aqua), pink)
var(--custom-prop, var(--default-value))
var(--custom-prop, var(--default-value, red))

But this format has the risk that some productions may contain a lot of quantifiers to expand.


PS: CSS type pages seem to a deeper hole to fall into 😬

@wbamberg
Copy link
Collaborator Author

wbamberg commented Sep 6, 2023

Thanks for your thoughtful response, @yarusome !

The overall structure presented in this template looks good, but I have one explicit usage doubt: For oklch(), where should the invocation pattern oklch(L C H[/ A]) go?

In the template, the "Syntax" section first lists example uses of the functions, then lists paramaters in the "Parameters" subsection, but parameters are usually order-dependent, and there is no easy way to indicate if any literal tokens like commas and slashes are needed in between.

Yeah, this is a good point, and before I'd read the second part of your post I'd also thought we could use comments in the "syntax example" block to express this.

Comparing to JSON.stringify(), the "Syntax" section there is used to show the invocation patterns. So the issue lies in the different roles of the "Syntax" sections on CSS and JavaScript pages.

The general point is also right though, and it is a bit worrying that it might be misleading to call these things "Parameters" when they are rather different to JS parameters. We could call if "Values" instead, but still have "Return value"? Not sure. I think I still like "Parameters" better.

A smaller issue is that, I think, example uses in the "Syntax" section should not have a comma at the line end as they are not complete value declarations. JavaScript pages don't have such commas, either.

Do you mean semicolon?

Maybe the "Syntax" section can show the pattern in comments:

/* var( <custom-property-name> , <declaration-value>? ) */

/* Without a fallback */
var(--custom-prop)

/* With fallback */
var(--custom-prop,) /* empty value as fallback */
var(--custom-prop, initial) /* initial value of the property as fallback */
var(--custom-prop, #FF0000)
var(--my-background, linear-gradient(transparent, aqua), pink)
var(--custom-prop, var(--default-value))
var(--custom-prop, var(--default-value, red))

Then custom-property-name and declaration-value are listed as paramters in the "Parameter" subsection.

A few notes:

  1. The pattern in the comment can be in a form that better explains the production in the spec, e.g. for oklch():
    css /* oklch( <L> <C> <H> [/ <A> ]? ) */

I'm not sure the extra syntax is a good idea. They seem to be signalling that this is formal syntax, but this is pseudo-formal syntax. And I'm worried it might confuse people that say / is meant to appear literally but < isn't, leading to people thinking it's like: oklch(<1> <2> <3> / <4>).

In general, formal syntax is hard enough for people without adding something else that is kind of like formal syntax, but isn't.

If we can I would prefer a more literal/duplicated approach:

/* oklch( L C H) */
oklch(40.1% 0.123 21.57)

/* oklch( L C H / A) */
oklch(59.69% 0.156 49.77 / .5)

...and the things in the brackets are either literals or things we explain in "Parameters".

An alternative format is to list several patterns to reduce the use of quantifiers:

/* Without a fallback */

/* var( <custom-property-name> ) */
var(--custom-prop)

/* With fallback */

/* var( <custom-property-name> , ) */
var(--custom-prop,) /* empty value as fallback */

/* var( <custom-property-name> , <declaration-value> ) */
var(--custom-prop, initial) /* initial value of the property as fallback */
var(--custom-prop, #FF0000)
var(--my-background, linear-gradient(transparent, aqua), pink)
var(--custom-prop, var(--default-value))
var(--custom-prop, var(--default-value, red))

Oh, this is what I said up there I think :).

But this format has the risk that some productions may contain a lot of quantifiers to expand.

Yes, this is a worry, but I think it is better in most cases.

PS: CSS type pages seem to a deeper hole to fall into 😬

Ha, well feel free to start something there if you want :).

@yarusome
Copy link
Contributor

yarusome commented Sep 7, 2023

Do you mean semicolon?

Yes, my head must be contorted at that time.

If we can I would prefer a more literal/duplicated approach:

/* oklch( L C H) */
oklch(40.1% 0.123 21.57)

/* oklch( L C H / A) */
oklch(59.69% 0.156 49.77 / .5)

...and the things in the brackets are either literals or things we explain in "Parameters".

This looks much better than the angled notations! (But why /* oklch( L C H) */ instead of /* oklch(L C H) */ or /* oklch( L C H ) */?)

@wbamberg
Copy link
Collaborator Author

wbamberg commented Sep 7, 2023

(But why /* oklch( L C H) */ instead of /* oklch(L C H) */ or /* oklch( L C H ) */?)

No reason, just a typo.

@OnkarRuikar
Copy link
Contributor

I'd also thought we could use comments in the "syntax example" block to express this.

:) Calling it "Syntax" is confusing. It makes readers start looking for syntax patterns in these usage examples. This is somewhat frustrating cos real syntax comes much later on the page. We could have order like syntax, formal syntax, parameters, and return values.
Also, on CSS pages this is not the same as JS or API syntax sections which breaks the readers understanding of the overall site's structure. We could call the section "Usage", "Usage Examples".

If we can I would prefer a more literal/duplicated approach:

We are already doing this so we could put this in the template.

@wbamberg
Copy link
Collaborator Author

wbamberg commented Sep 8, 2023

:) Calling it "Syntax" is confusing. It makes readers start looking for syntax patterns in these usage examples. This is somewhat frustrating cos real syntax comes much later on the page. We could have order like syntax, formal syntax, parameters, and return values.

All our research says that (unfortunately) most people don't understand formal syntax or even look at it. And I do think the intention of this section is to show the syntax, it just does it "by example" rather than using a formal notation.

We could call the section "Usage", "Usage Examples".

This seems like a very vague name. How is "usage examples" different from "examples"? How is someone expected to understand that distinction? An example is different to this block, because an example would show complete code and explain what the specific values chosen do.

There's a long discussion about all this here: https://github.com/orgs/mdn/discussions/218 FWIW I do think (as I said in that discussion) that we ought to reunite "formal syntax" and "values", but there didn't seem to be consensus for it.

Anyway: please, let's keep this to functions in this discussion. I don't think we should only rename "Syntax" for CSS functions, and I would like to get towards some consistency for those pages.

@wbamberg
Copy link
Collaborator Author

I pushed a new commit including the changes requested by @yarusome , more or less. I removed from the syntax block the inline comments like /* initial value of the property as fallback */ because I think it is better to explain things in prose.

@OnkarRuikar
Copy link
Contributor

  1. Should "Parameters" section follow the formal syntax order? or we put optional pramas last in the list?
  2. Is it mandatory to use only the params mentioned in formal syntax? Same name and same number of times?
  3. About comments in "Syntax" section, should it be only explanations?

Copy link
Member

@estelle estelle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a couple suggestions and a couple edits.


## Description

This is an optional section to include a description of the function and explain how it works. Use this section to explain related terms and add use cases for the function.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This is an optional section to include a description of the function and explain how it works. Use this section to explain related terms and add use cases for the function.
Include a description of the function and explain how it works. Use this section to explain related terms and add use cases for the function.

Prefer to have this semi-required.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy to recommend it but this edit makes it required, not semi-required. At the moment many of our pages don't have description sections, and if we make it mandatory we are likely to get low-value edits just to tick boxes in the template.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could say "While this section is optional, we highly recommend you to add a description..."

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or just "We highly recommend you include a description..."

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if it is optional, we should say it is optional. It's just clearer that way. I've pushed an update along the lines of Dipika's suggestion.

@wbamberg
Copy link
Collaborator Author

wbamberg commented Sep 13, 2023

  1. Should "Parameters" section follow the formal syntax order? or we put optional pramas last in the list?

I think following the formal syntax order would be better.

  1. Is it mandatory to use only the params mentioned in formal syntax? Same name and same number of times?

Ideally we would have synchronization between the "syntax example code block", the "values" list, and the formal syntax. That way people can cross-reference from one to the other.

As I said in the other PR, that would mean things like:

/* drop-shadow( <length> <length> <length> ) */

...which is not that far from @dipikabh 's suggestion of:

/* Three length values */

...only it maps more explicitly to the <length> listed in the other 2 sections.

  1. About comments in "Syntax" section, should it be only explanations?

I'm not sure what "only explanations" means here. From @yarusome 's comment at #28953 (comment), I'm not too keen on:

var(--custom-prop, initial) /* initial value of the property as fallback */

...because (1) I think the value and meaning of initial should be documented in "Values", and (2) if we allow comments like this, what's the rationale for not having comments like:

var(--my-background, linear-gradient(transparent, aqua), pink) /* Set a gradient as a fallback */

I think the function of the "syntax example code block" is to describe the syntax of the thing, and the semantics ought to be explained in prose.

Anyway, this is just my opinion, and maybe there are cases where it wouldn't work. But even if we can't agree on all these details, I would love to have something we can use to get at least a minimal amount of consistency between pages here.

Copy link
Contributor

@dipikabh dipikabh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking this up, Will! I've added a few suggestions and questions.


```css
/* Without a fallback */

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As pointed in previous comment, I think we should not have an empty line here

/* var( <custom-property-name> , ) */
var(--custom-prop,)

/* var( <custom-property-name> , <declaration-value> ) */
Copy link
Contributor

@dipikabh dipikabh Sep 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency, we would need to add a descriptive comment here as well?

Which makes me wonder - is it optional or mandatory to add two comments for each syntax use case. We would need to be more specific in the instructions. At the least, a syntax use case should be preceded by a descriptive comment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really know how prescriptive we need to be in general.

I have 2 main concerns:

  • people should be able to cross-reference between this code block, the "Values" list, and the formal syntax. So we should if at all possible use the same names for things in all three places.
  • we should use the code block as a place to show the syntax, not to describe the syntax.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, it can be a guideline instead of a requirement and authors and reviewers can take a call based on the context.


## Description

This is an optional section to include a description of the function and explain how it works. Use this section to explain related terms and add use cases for the function.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could say "While this section is optional, we highly recommend you to add a description..."


### Add a descriptive heading

Each example must have an H3 heading (`###`) naming the example. The heading should be descriptive of what the example is doing. For example, "A simple example" does not say anything about the example and therefore, not a good heading. The heading should be concise. For a longer description, use the paragraph after the heading.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I always thought that in addition to making an H3 heading mandatory in "Examples", we also had some guidance to include a few words about the example. It might make more sense now if we get rid of the language H4 headers as well as "Result". We need to convey that the H3 should be followed by a description of the example scenario/setup or what we're trying to demo. And ideally, after the result frame, explain the output as a result of what we did in the example codes.

  2. I realize that this would need to be updated across all the template files that have the "Examples" section and I can take that up we agree on the general idea to include that instruction.

  3. CSS property page template, just as an example, also has a "Note" block (I remember you added that across templates :) ). Are we going to add that here?

Copy link
Collaborator Author

@wbamberg wbamberg Sep 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. CSS property page template, just as an example, also has a "Note" block (I remember you added that across templates :) ). Are we going to add that here?

I've added this.

  1. I always thought that in addition to making an H3 heading mandatory in "Examples", we also had some guidance to include a few words about the example. It might make more sense now if we get rid of the language H4 headers as well as "Result". We need to convey that the H3 should be followed by a description of the example scenario/setup or what we're trying to demo. And ideally, after the result frame, explain the output as a result of what we did in the example codes.

  2. I realize that this would need to be updated across all the template files that have the "Examples" section and I can take that up we agree on the general idea to include that instruction.

Can we split proposals for how to change the way we examples across all pages off from this request to add a template for a specific page type? This seems really out of scope for my PR here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we split proposals for how to change the way we examples across all pages off from this request to add a template for a specific page type? This seems really out of scope for my PR here.

Fair enough, better to discuss this separately

@wbamberg
Copy link
Collaborator Author

OK, I've made most of the updates and argued with a couple.

Copy link
Contributor

@dipikabh dipikabh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for the updates, @wbamberg! Leaving my +1.

@Elchi3 Elchi3 removed their request for review September 19, 2023 13:04
@estelle estelle merged commit fcf8683 into mdn:main Sep 20, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:Meta Content in the meta docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants