Re: [w3c/manifest] feat: add categories member (resolves #569) (#598)

marcoscaceres requested changes on this pull request.

Super close to done! Just a couple of little nits.  

> @@ -2612,6 +2617,112 @@ <h3 id="applying">
       </section>
       <section>
         <h3>
+          <code title="">categories</code> member
+        </h3>
+        <p>
+          The <dfn id="member-categories"><code>categories</code>
+          member</dfn> describes the expected application categories (or 

Please remove ("or single category") ... as `categories: []` is valid. 

> @@ -2612,6 +2617,112 @@ <h3 id="applying">
       </section>
       <section>
         <h3>
+          <code title="">categories</code> member
+        </h3>
+        <p>
+          The <dfn id="member-categories"><code>categories</code>
+          member</dfn> describes the expected application categories (or 
+          single category) to which the web application belongs. This is a hint 

I would remove "This is a hint ..." as that is basically what you say in the next paragraph. 

> +          member</dfn> describes the expected application categories (or 
+          single category) to which the web application belongs. This is a hint 
+          to catalogs or stores of web applications under which categories 
+          (or category) to list this web application.
+        </p>
+        <p>
+          The <code>categories</code> member is only meant as a hint to catalogs 
+          or stores listing web applications and it is expected that these 
+          will make a best effort to find appropriate categories (or category)
+          under which to list the web application.
+        </p>
+        <p>
+          The <dfn>steps for processing the <code>categories</code>
+          member</dfn> are given by the following algorithm. The algorithm
+          takes a <var>manifest</var> as an argument. This algorithm returns an
+          array of strings or <code>undefined</code>. The array of strings MAY 

Please remove "The array of strings MAY include only one value." It can include no values. 

> +          will make a best effort to find appropriate categories (or category)
+          under which to list the web application.
+        </p>
+        <p>
+          The <dfn>steps for processing the <code>categories</code>
+          member</dfn> are given by the following algorithm. The algorithm
+          takes a <var>manifest</var> as an argument. This algorithm returns an
+          array of strings or <code>undefined</code>. The array of strings MAY 
+          include only one value.
+        </p>
+        <ol>
+          <li>Let <var>categories</var> be an empty list.</li>
+          <li>Let <var>unprocessed categories</var> be the result of calling
+          the <a>[[\GetOwnProperty]]</a> internal method of <var>manifest</var>
+          with argument "<code>categories</code>".</li>
+          <li>If <var>unprocessed categories</var>is an array, then:</li>

I think there is a markup error here:
```HTML
 <li>If <var>unprocessed categories</var>is an array, then:</li>
```

Should be:

```HTML
 <li>If <var>unprocessed categories</var>is an array, then:
    <ol>
       <!-- other steps-->
    </ol>
</li>
```

> +              <li>For each <var>potential category</var> in the array:</li>
+                <ol>
+                  <li>
+                    <a>Trim</a>(<var>value</var>) and convert 
+                    <a>to ASCII lowercase</a>
+                  </li>
+                  <li>
+                    Append <var>potential category</var> to 
+                    <var>categories</var>
+                  </li>
+                </ol>
+            </ol>
+          <li>
+            Otherwise, if <var>unprocessed categories</var> is not 
+            <code>undefined</code>
+          </li>

As above, markup error here. 

> +            <code>undefined</code>
+          </li>
+            <ol>
+              <li>
+                <a>Issue a developer warning</a> that the type is not 
+                supported.
+              </li>
+            </ol>
+          <li>Return <var>categories</var></li>
+        </ol>
+        <p>
+          The categories string array is case insensitive and converted 
+          to lower-case by following the processing algorithm. Thus, 
+          <code>sports</code>, <code>Sports</code>, <code>SPORTS</code>, 
+          and <code>SpOrTs</code> are all equivalent. Manifest authors 
+          SHOULD use lower-case.

Avoid using RFC2119 keywords on anything but user agents.  

So, I suggest removing "Manifest authors SHOULD..." as it doesn't matter. 

You can instead add a Note, that "Authors are encouraged to use lower case" (see other authoring notes in the spec). 

> +              <li>
+                <a>Issue a developer warning</a> that the type is not 
+                supported.
+              </li>
+            </ol>
+          <li>Return <var>categories</var></li>
+        </ol>
+        <p>
+          The categories string array is case insensitive and converted 
+          to lower-case by following the processing algorithm. Thus, 
+          <code>sports</code>, <code>Sports</code>, <code>SPORTS</code>, 
+          and <code>SpOrTs</code> are all equivalent. Manifest authors 
+          SHOULD use lower-case.
+        </p>
+        <p>
+          The categories string array is a hint to catalogs and stores 

You already stated this a few paragraphs above, so remove this paragraph. 

> +        </ol>
+        <p>
+          The categories string array is case insensitive and converted 
+          to lower-case by following the processing algorithm. Thus, 
+          <code>sports</code>, <code>Sports</code>, <code>SPORTS</code>, 
+          and <code>SpOrTs</code> are all equivalent. Manifest authors 
+          SHOULD use lower-case.
+        </p>
+        <p>
+          The categories string array is a hint to catalogs and stores 
+          of where the Manifest author suggests their web app should be 
+          listed. This is hint or suggestion from the Manifest author. 
+          Like search engine keywords, catalogs and stores are not 
+          required to honor this hint.</p>
+        <p>
+          If a categories string array has multiple values, a consuming 

"Consuming systems" are outside the scope of this spec, so I suggest dropping this also. 

> +          <code>sports</code>, <code>Sports</code>, <code>SPORTS</code>, 
+          and <code>SpOrTs</code> are all equivalent. Manifest authors 
+          SHOULD use lower-case.
+        </p>
+        <p>
+          The categories string array is a hint to catalogs and stores 
+          of where the Manifest author suggests their web app should be 
+          listed. This is hint or suggestion from the Manifest author. 
+          Like search engine keywords, catalogs and stores are not 
+          required to honor this hint.</p>
+        <p>
+          If a categories string array has multiple values, a consuming 
+          system MAY use one or more of these values.  
+        </p>
+        <p>
+          Manifest authors SHOULD use the below common categories:

We probably need to make this into a registry. The use of plural and singular form seems a bit arbitrary. 

@kenchris, can you check?    

> +          <code>sports</code>, <code>Sports</code>, <code>SPORTS</code>, 
+          and <code>SpOrTs</code> are all equivalent. Manifest authors 
+          SHOULD use lower-case.
+        </p>
+        <p>
+          The categories string array is a hint to catalogs and stores 
+          of where the Manifest author suggests their web app should be 
+          listed. This is hint or suggestion from the Manifest author. 
+          Like search engine keywords, catalogs and stores are not 
+          required to honor this hint.</p>
+        <p>
+          If a categories string array has multiple values, a consuming 
+          system MAY use one or more of these values.  
+        </p>
+        <p>
+          Manifest authors SHOULD use the below common categories:

About the registry: we need to do something similar to https://www.w3.org/Payments/card-network-ids

In a separate document. 



-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/w3c/manifest/pull/598#pullrequestreview-57691855

Received on Tuesday, 22 August 2017 08:21:53 UTC