Re: [whatwg/encoding] Non-normatively visualize the indexes (#89)

annevk requested changes on this pull request.

Couple nits. Looks good to me overall. I wonder if @inexorabletash would also be willing to review.

> @@ -68,6 +68,8 @@ if [ $BRANCH != "master" ] ; then
          > $BRANCH_DIR/index.html;
     cp *.txt $BRANCH_DIR/;
     cp *.json $BRANCH_DIR/;
+    cp *.css $BRANCH_DIR/;
+    python visualize.py $BRANCH_DIR/

Was it intentional to skip commit snapshots?

> @@ -18,6 +18,7 @@ Markup Shorthands: css off
 Translate IDs: dictdef-textdecoderoptions textdecoderoptions,dictdef-textdecodeoptions textdecodeoptions
 </pre>
 
+<style>@import 'visualization-colors.css';</style>

According to Tab we can use `<link>` and Bikeshed will move it correctly in the output.

> @@ -653,33 +654,66 @@ changed, so has the <a>index</a>.
 <var>code point</var> in <var>index</var>, or null if
 <var>code point</var> is not in <var>index</var>.
 
+<div class=note id=visualization>
+<p>There is a non-normative visualization for each index other than index gb18030 ranges.

Since this is inside a `<div>` it needs to be indented by one.

> @@ -653,33 +654,66 @@ changed, so has the <a>index</a>.
 <var>code point</var> in <var>index</var>, or null if
 <var>code point</var> is not in <var>index</var>.
 
+<div class=note id=visualization>
+<p>There is a non-normative visualization for each index other than index gb18030 ranges.

We should also xref the indexes here I think.

> @@ -653,33 +654,66 @@ changed, so has the <a>index</a>.
 <var>code point</var> in <var>index</var>, or null if
 <var>code point</var> is not in <var>index</var>.
 
+<div class=note id=visualization>
+<p>There is a non-normative visualization for each index other than index gb18030 ranges.
+index jis0208 also has an alternative Shift_JIS visualization.
+Additionally, there is visualization of the Basic Multilingual Plane coverage of each index other than index gb18030 ranges.
+
+<p>The legend for the visualizations is:
+<ul class=visualizationlegend>
+    <li class=unmapped>Unmapped

These have too much indentation. `<div>` + `<ul>` means two spaces.

>   <tr>
   <td><dfn export>index Big5</dfn>
   <td><a href=index-big5.txt>index-big5.txt</a>
+  <td><a href=big5.html>Visualization</a>

We should add a title here saying "index Big5 visualization". Even better would be to make each link text unique somehow since that affects screen readers.

>    <td>This matches the KS X 1001 standard and the Unified Hangul Code, more
-  commonly known together as Windows Codepage 949.
+  commonly known together as Windows Codepage 949. This index covers the Hangul Syllables

s/This index/It/

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/encoding/pull/89#pullrequestreview-16941952