Improve sortClassesInSelectors method and add comments (BL-12857)

Also fix bugs caused by shortening paths in filter output.

Author: StephenMcConnel

Diff for: custom-css-analysis-and-migration/create-migrations.ts

@@ -37,7 +37,7 @@ for (const record of records) {
 
   ++count;
   if (Bun.argv.length > 2 && count > Number.parseInt(Bun.argv[2])) break;
-  console.log(`css = ${record.css}`);
+  //console.log(`css = ${record.css}`);
   const checksum = crypto.createHash("md5").update(record.css).digest("hex");
   const folder = `./output/migration/${count} ${checksum}`;
   console.log(`output folder=${folder}`);
@@ -59,7 +59,7 @@ for (const record of records) {
 
   const brandingSet = new Set();
   for (let i = 0; i < record.paths.length; ++i) {
-    const metaPath = record.paths[i] + "/meta.json";
+    const metaPath = "./output/downloads/" + record.paths[i] + "/meta.json";
     try {
       const metaString: string = fs.readFileSync(metaPath, "utf8");
       const metadata = JSON.parse(metaString);
@@ -69,14 +69,14 @@ for (const record of records) {
           brandingProject.toLowerCase() !== "local-community")
         brandingSet.add(brandingProject);
     } catch (e) {
-      console.log("Could not extract brandingProject from " + metaPath);
+      console.log("Could not extract brandingProjectName from " + metaPath + ": " + e);
     }
   }
 
   const uniqueUploaders = [
     ...new Set(
       record.paths.map((path: string) => {
-        const email = path.split("/")[2];
+        const email = path.split("/")[0];
         const emailParts = email.split("@");
         const obfuscatedEmail =
           emailParts[0].slice(0, -2) + "..." + "@" + emailParts[1];
@@ -100,8 +100,6 @@ for (const record of records) {
 // A replacement customBookStyles.css has been generated.
 {
   "cssThemeName": "default",
-  "coverShowTitleL2": ${migration.hideL2TitleOnCover ? "false" : "true"},
-  "coverShowTitleL3": false,
 }
 `;
   let appearancePath = `${folder}/appearance.json`;

Diff for: custom-css-analysis-and-migration/filter-stylesheets.ts

@@ -34,7 +34,7 @@ const recordsWithUniqueifiedPaths = filteredRecords.map((record) => {
   const uniquePaths = uniqueFilenames.map((filename) => {
     return paths.find((path) => path.endsWith(filename));
   });
-  const instanceId = paths[0].split("/")[3];
+  const instanceId = paths[0].split("/")[1];
   return {
     book_count: paths.length,
     unique_named_books: uniquePaths.length,

Diff for: custom-css-analysis-and-migration/migrateCssToAppearance.ts

@@ -2,6 +2,7 @@
  *  function that is called by create-migrations.ts
  * **/
 
+// These are documented at https://www.npmjs.com/package/css/v/3.0.0.
 import { parse, stringify, Rule, Stylesheet, Declaration } from "css";
 export interface Migration {
   modifiedCss: string;
@@ -144,12 +145,13 @@ export function migrateCssToAppearance(cssFileContent: string): Migration {
         // We're looking for rules that affect the marginBox's size and position (top/left/height/width)
         // because the new theme system controls these with variables that affect the .bloom-page margin
         // instead of absolutely positioning the .marginBox, so setting its top or left will have no effect,
-        // and setting its height or width will probably do something unwanted.  If such a rule also changes
+        // and setting its height or width will probably do something unwanted.  If such a rule also has
         // safe properties, we split it into two rules, one that has only the problem declarations (which
-        // we try to fix to the new values needed) and one that has only the safe declarations.
-        // We also remove the .marginBox class from the selectors for the rules with the updated new
-        // declarations, since it's no longer needed.
-
+        // we try to fix to have the new values needed) and one that has only the safe declarations.
+        // The .marginBox class is removed from the selectors for the rules with the updated new
+        // declarations since it's no longer needed.
+        // Producing split rules also helps human reviewers to see what's going on, and to check that the
+        // new rules are correct.
         const unknownDeclarations = rule.declarations?.filter(
           (d: Declaration) =>
             d.property !== "height" &&
@@ -164,7 +166,7 @@ export function migrateCssToAppearance(cssFileContent: string): Migration {
             selectors: rule.selectors,
             declarations: unknownDeclarations,
           };
-          sortClassesInSelector(newRule);
+          sortClassesInSelectors(newRule);
           AddWarningCommentsIfNeeded(newRule);
           otherRules.push(newRule);
           ruleIndices.push(index + 1);
@@ -230,7 +232,7 @@ export function migrateCssToAppearance(cssFileContent: string): Migration {
           }
         });
 
-        sortClassesInSelector(rule);
+        sortClassesInSelectors(rule);
         sortDeclarations(rule);
       }
     });
@@ -255,6 +257,8 @@ function sortDeclarations(rule: Rule) {
 
   // Define the property conversions object
   const orderedProperties: PropertyConversions = {
+    "--cover-margin-top": 0,
+    "--cover-margin-bottom": 1,
     "--page-margin-top": 0,
     "--page-margin-bottom": 1,
     "--page-margin-left": 2,
@@ -277,15 +281,24 @@ function sortDeclarations(rule: Rule) {
   });
 }
 
-function sortClassesInSelector(rule: any): void {
-  // sort the classes in the first selector if appropriate
+function sortClassesInSelectors(rule: any): void {
+  // Sort the classes in the selectors if feasible.
+  // This makes the rules easier to read and to compare.
   if (!rule.selectors || !rule.selectors.length) return;
-  var selector = rule.selectors[0].trim();
-  if (/^\.[A-Za-z][-.A-Za-z0-9]*$/.test(selector))
-  {
-    const classes = rule.selectors[0].trim().split(".").filter(Boolean).sort();
-    const sortedSelector = "." + classes.join(".");
-    rule.selectors[0] = sortedSelector;
+  for (let i = 0; i < rule.selectors.length; ++i) {
+    // Note that rule.selectors is an Array of Strings split on commas.
+    const selector = rule.selectors[i].trim();
+    // If the selector is just a list of classes in the same element, sort them.
+    // We don't try to sort selectors that have multiple element levels or that
+    // have something other than classes.
+    if (/^\.[A-Za-z][-.A-Za-z0-9]*$/.test(selector))
+    {
+      const classes = selector.split(".").filter(Boolean).sort();
+      const sortedSelector = "." + classes.join(".");
+      // We don't need to worry about leading or trailing spaces in the selector
+      // because the output stringify function has its own ideas about how to format.
+      rule.selectors[i] = sortedSelector;
+    }
   }
 }