- From: Marcos Cáceres <notifications@github.com>
- Date: Fri, 22 Sep 2017 05:49:57 +0000 (UTC)
- To: w3c/manifest <manifest@noreply.github.com>
- Cc: Subscribed <subscribed@noreply.github.com>
- Message-ID: <w3c/manifest/pull/613/review/64484990@github.com>
marcoscaceres requested changes on this pull request. > @@ -1261,7 +1259,7 @@ <h3 id="navigation-scope-security-considerations"> </p> <p> A <a>manifest</a> is obtained and applied regardless of the - <a><code>media</code></a> attribute of the <a><code>link</code> + <a>media</a> attribute of the <a><code>link</code> Nit: seems you missed a closing `</a>`. Also, please remove the `<code>`. > @@ -1574,7 +1572,7 @@ <h3 id="applying"> <div> <p> Please note that the <a>start URL</a> is not necessarily the value - of the <a><code>start_url</code> member</a>: the user or user agent + of the <a data-link-for="WebAppManifest"><code>start_url</code></a> member: the user or user agent Bit cleaner, you can do: ```HTML <a data-tl="WebAppManifest.start_url">start_url</a> ``` > @@ -1592,17 +1590,39 @@ <h3 id="applying"> <div class="issue" data-number="446"></div> </section> </section> - <section> + <section data-dfn-for="WebAppManifest" data-link-for="WebAppManifest"> <h2> Manifest and its members This should change to: ```HTML <h2> <dfn>WebAppManifest</dfn> dictionary </h2> ``` > <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> See comment above... so here ` <a>WebAppManifest</a>` > <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> Actually, " A manifest consists of a top-level..." is redundant... remove this sentence. The WebIDL itself already says this. > </p> + <pre class="idl"> Nit: I prefer seeing the IDL before the description... "an IDL block speaks a thousand words" 😂 > @@ -1663,7 +1686,7 @@ <h3 id="applying"> end-user, the user agent MUST use the <a>base direction</a> to compute directional runs and layout or position text correctly in text containing mixed-direction sequences [[!BIDI]]. When the <a>base - direction</a> is "<a data-lt="direction auto">auto</a>" the user + direction</a> is "<a data-link-for="TextDirectionType">auto</a>" the user move `data-link-for="TextDirectionType"` inside ` <dl data-dfn-for="TextDirectionType">` > @@ -2090,15 +2121,15 @@ <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>orientation</dfn> Nit: the `<dfn>orientation</dfn>` and so on, should all be defined inside the `h3`s... like this: ```HTML <h3> <dfn>orientation</dfn> member </h3> <p>The <a>orientation</a> member bla bla..</p> ``` Applies to all of the ones below... > @@ -2995,13 +3035,13 @@ <h3 id="applying"> </pre> </div> </section> - <section> + <section data-dfn-for="imageresource"> ``` <section data-dfn-for="ImageResource" data-link-for="ImageResource"> ``` > @@ -3094,18 +3134,18 @@ <h3 id="applying"> </li> </ol> </section> - <section> + <section data-dfn-for="imageresource"> You shouldn't need: ``` data-dfn-for="imageresource" ``` ReSpec will find it from the parent section. > @@ -3234,41 +3274,40 @@ <h3 id="applying"> Each member specifies which object a multi-purpose member can be used with. </p> - <section> + <section data-dfn-for="ExternalApplicationResource" data-link-for="ExternalApplicationResource"> `platform` should be defined in the ExternalApplicationResource section. If this is a shared definition, then we should make a note that two things share the same definition. > @@ -3472,11 +3517,20 @@ <h3 id="applying"> </section> <section> Add: `data-dfn-for= "ExternalApplicationResource"` and `data-link-for="ExternalApplicationResource"` > @@ -3472,11 +3517,20 @@ <h3 id="applying"> </section> <section> Please make sure all sections that define IDL things have both. -- 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-64484990
Received on Friday, 22 September 2017 05:50:22 UTC