Re: [w3c/manifest] Add IDL section and define IDL for all the members (#613)

marcoscaceres requested changes on this pull request.

Mostly nits, but a couple of things to change... looking good.

>        </p>
+      <pre class="idl">
+          [NoInterfaceObject]

Remove [NoInterfaceObject]  ... it's not an interface, it's a dictionary 😉. Also, you should never ever need to use NoInterfaceObject in a spec unless you are creating some kind of mix-in. 

> @@ -1592,17 +1590,40 @@ <h3 id="applying">
         <div class="issue" data-number="446"></div>
       </section>
     </section>
-    <section>
+    <section data-dfn-for="webappmanifest" data-link-for="webappmanifest">

The `data-dfn-for="webappmanifest" data-link-for="webappmanifest"` need to match actual IDL things, so, should be "WebAppManifest".

>        <h2>
         Manifest and its members
       </h2>
       <p>
         A <dfn>manifest</dfn> is a JSON document that contains startup
         parameters and application defaults for when a web application is
-        launched. A manifest consists of a top-level <a>object</a> that
-        contains zero or more members. Each of the members are defined below,
-        as well as how their values are processed.
+        launched. A manifest consists of a top-level <dfn>WebAppManifest</dfn>
+        <a>object</a> that contains zero or more members.

It's a dictionary, not an object. 

>        <h2>
         Manifest and its members
       </h2>
       <p>
         A <dfn>manifest</dfn> is a JSON document that contains startup
         parameters and application defaults for when a web application is
-        launched. A manifest consists of a top-level <a>object</a> that
-        contains zero or more members. Each of the members are defined below,
-        as well as how their values are processed.
+        launched. A manifest consists of a top-level <dfn>WebAppManifest</dfn>
+        <a>object</a> that contains zero or more members.
+        Each of the members are defined below, as well as how their values are processed.

Nit: make sure you run tidy over the doc. 

> @@ -1612,45 +1633,49 @@ <h3 id="applying">
           <code>dir</code> member

Can you replace this with: `<dfn>dir</dfn>member`, and the same for the rest? 

> @@ -1612,45 +1633,49 @@ <h3 id="applying">
           <code>dir</code> member

you won't need `<code>`, ReSpec will automatically code them. 

>          <p>
           The <dfn>directionality-capable members</dfn> are:
         </p>
         <ul>
           <li>
-            <a><code>description</code> member</a>.
+            <a><code>description</code></a> member.

s/`<a><code>description</code></a> member.`/`<a>description</a> member.`

Same for the rest. You should never need `<code>` anywhere. 

>            </li>
         </ul>
         <p>
-          The <dfn data-lt="text-direction value">text-direction values</dfn>
-          are the following, implying that the value of the
+          The <dfn data-lt="text-direction value">text-direction values</dfn> defined by
+          <dfn>TextDirectionType</dfn>, are the following, implying that the value of the

The `TextDirectionType` should have it's own top-level section. 

> @@ -2090,8 +2124,8 @@ <h3 id="applying">
           <code>orientation</code> member
         </h3>
         <p>
-          The <dfn id="member-orientation"><code>orientation</code>
-          member</dfn> is a <a>string</a> that serves as the <a>default
+          The <dfn><code>orientation</code></dfn>

s/`<dfn><code>orientation</code></dfn>`/`<dfn>orientation</dfn> member`

As above, the `dfn` for this should occur inside a `h3` element in its own section. 

> @@ -2090,8 +2124,8 @@ <h3 id="applying">
           <code>orientation</code> member
         </h3>
         <p>
-          The <dfn id="member-orientation"><code>orientation</code>
-          member</dfn> is a <a>string</a> that serves as the <a>default
+          The <dfn><code>orientation</code></dfn>

The above applies for all the things below. 

>        </h2>
       <div class="issue" data-number="361"></div>
+      <pre class="idl">
+        [NoInterfaceObject]
+        dictionary ImageResource {
+          USVString src;
+          DOMString sizes;

I don't think this is required. 

-- 
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/613#pullrequestreview-64175329