Merge pull request #1904 from googlefonts/fontoverview-group-tweaks

[font overview] Group By improvements
googlefonts · Dec 31, 2024 · 10cef1c · 10cef1c
2 parents 1ec2cac + 9762b5f
commit 10cef1c
Show file tree

Hide file tree

Showing 5 changed files with 80 additions and 62 deletions.
diff --git a/src/fontra/client/core/glyph-organizer.js b/src/fontra/client/core/glyph-organizer.js
@@ -1,4 +1,5 @@
 import { getGlyphInfoFromCodePoint, getGlyphInfoFromGlyphName } from "./glyph-data.js";
+import { capitalizeFirstLetter } from "./utils.js";
 
 function getGlyphInfo(glyph) {
   const codePoint = glyph.codePoints[0];
@@ -9,7 +10,7 @@ function getGlyphInfo(glyph) {
   );
 }
 
-function getGroupingInfo(glyph, options) {
+function getGroupByInfo(glyph, options) {
   const glyphInfo = getGlyphInfo(glyph);
   return {
     ...Object.fromEntries(
@@ -21,19 +22,21 @@ function getGroupingInfo(glyph, options) {
   };
 }
 
-const groupProperties = [
-  "script",
-  "category",
-  "case",
-  "subCategory",
-  "glyphNameExtension",
+export const groupByProperties = [
+  { key: "script", label: "Script" },
+  { key: "case", label: "Case", compare: compareCase },
+  { key: "category", label: "Category" },
+  { key: "subCategory", label: "Sub-category" },
+  { key: "glyphNameExtension", label: "Glyph name extension" },
 ];
 
+export const groupByKeys = groupByProperties.map(({ key }) => key);
+
 export class GlyphOrganizer {
   constructor() {
     this._glyphNamesListFilterFunc = (item) => true; // pass all through
 
-    this.setGroupings([]);
+    this.setGroupByKeys([]);
   }
 
   setSearchString(searchString) {
@@ -45,15 +48,15 @@ export class GlyphOrganizer {
     this._glyphNamesListFilterFunc = (item) => glyphFilterFunc(item, searchItems);
   }
 
-  setGroupings(groupings) {
+  setGroupByKeys(groupByKeys) {
     const options = {};
-    groupings.forEach((grouping) => (options[grouping] = true));
+    groupByKeys.forEach((groupByKey) => (options[groupByKey] = true));
 
-    this.setGroupingFunc((glyph) => getGroupingKey(glyph, options));
+    this.setGroupByFunc((glyph) => getGroupByKey(glyph, options));
   }
 
-  setGroupingFunc(groupingFunc) {
-    this._groupingFunc = groupingFunc;
+  setGroupByFunc(groupByFunc) {
+    this._groupByFunc = groupByFunc;
   }
 
   sortGlyphs(glyphs) {
@@ -70,34 +73,34 @@ export class GlyphOrganizer {
     const groups = new Map();
 
     for (const item of glyphs) {
-      const groupingInfo = this._groupingFunc(item);
-      let group = groups.get(groupingInfo.groupingKey);
+      const groupByInfo = this._groupByFunc(item);
+      let group = groups.get(groupByInfo.groupByKey);
       if (!group) {
-        group = { groupingInfo, glyphs: [] };
-        groups.set(groupingInfo.groupingKey, group);
+        group = { groupByInfo, glyphs: [] };
+        groups.set(groupByInfo.groupByKey, group);
       }
       group.glyphs.push(item);
     }
 
     const groupEntries = [...groups.values()];
     groupEntries.sort(compareGroupInfo);
 
-    const sections = groupEntries.map(({ groupingInfo, glyphs }) => ({
-      label: groupingInfo.groupingKey,
+    const sections = groupEntries.map(({ groupByInfo, glyphs }) => ({
+      label: groupByInfo.groupByKey,
       glyphs: glyphs,
     }));
 
     return sections;
   }
 }
 
-function compareGroupInfo(groupingEntryA, groupingEntryB) {
-  const groupingInfoA = groupingEntryA.groupingInfo;
-  const groupingInfoB = groupingEntryB.groupingInfo;
+function compareGroupInfo(groupByEntryA, groupByEntryB) {
+  const groupByInfoA = groupByEntryA.groupByInfo;
+  const groupByInfoB = groupByEntryB.groupByInfo;
 
-  for (const prop of groupProperties) {
-    const valueA = groupingInfoA[prop];
-    const valueB = groupingInfoB[prop];
+  for (const { key, compare } of groupByProperties) {
+    const valueA = groupByInfoA[key];
+    const valueB = groupByInfoB[key];
 
     if (valueA === valueB) {
       continue;
@@ -109,7 +112,7 @@ function compareGroupInfo(groupingEntryA, groupingEntryB) {
       return -1;
     }
 
-    return valueA < valueB ? -1 : 1;
+    return compare ? compare(valueA, valueB) : valueA < valueB ? -1 : 1;
   }
 
   return 0;
@@ -164,34 +167,41 @@ function getBaseGlyphName(glyphName) {
   return i >= 1 ? glyphName.slice(0, i) : "";
 }
 
-function getGroupingKey(glyph, options) {
-  const groupingInfo = getGroupingInfo(glyph, options);
+function getGroupByKey(glyph, options) {
+  const groupByInfo = getGroupByInfo(glyph, options);
 
-  let groupingKey = "";
+  const groupByKeyItems = [];
 
-  if (groupingInfo.category) {
-    groupingKey += groupingInfo.category;
+  if (groupByInfo.script) {
+    groupByKeyItems.push(capitalizeFirstLetter(groupByInfo.script));
   }
 
-  if (groupingInfo.subCategory) {
-    groupingKey += (groupingKey ? " / " : "") + groupingInfo.subCategory;
+  if (groupByInfo.case) {
+    groupByKeyItems.push(capitalizeFirstLetter(groupByInfo.case));
   }
 
-  if (groupingInfo.case) {
-    groupingKey += (groupingKey ? " / " : "") + groupingInfo.case;
+  if (groupByInfo.category) {
+    groupByKeyItems.push(groupByInfo.category);
   }
 
-  if (groupingInfo.script) {
-    groupingKey += (groupingKey ? " " : "") + `(${groupingInfo.script})`;
+  if (groupByInfo.subCategory) {
+    groupByKeyItems.push(groupByInfo.subCategory);
   }
 
-  if (groupingInfo.glyphNameExtension) {
-    groupingKey += (groupingKey ? " " : "") + `(*${groupingInfo.glyphNameExtension})`;
+  if (groupByInfo.glyphNameExtension) {
+    groupByKeyItems.push(`*${groupByInfo.glyphNameExtension}`);
   }
 
-  if (!groupingKey) {
-    groupingKey = "Other";
+  if (!groupByKeyItems.length) {
+    groupByKeyItems.push("Other");
   }
 
-  return { groupingKey, ...groupingInfo };
+  return { groupByKey: groupByKeyItems.join(" / "), ...groupByInfo };
+}
+
+function compareCase(caseA, caseB) {
+  const cases = ["upper", "lower", "minor"];
+  const indexA = cases.indexOf(caseA);
+  const indexB = cases.indexOf(caseB);
+  return indexA - indexB;
 }
diff --git a/src/fontra/client/web-components/glyph-cell-view.js b/src/fontra/client/web-components/glyph-cell-view.js
@@ -14,6 +14,8 @@ export class GlyphCellView extends HTMLElement {
     this.locationKey = options?.locationKey || "fontLocationSourceMapped";
     this.glyphSelectionKey = options?.glyphSelectionKey || "glyphSelection";
 
+    this._closedSections = new Set();
+
     this._magnification = 1;
 
     this._resetSelectionHelpers();
@@ -102,6 +104,16 @@ export class GlyphCellView extends HTMLElement {
     this._resetSelectionHelpers();
     this.glyphSections = glyphSections;
 
+    if (this.accordion.items) {
+      this.accordion.items.forEach((item) => {
+        if (item.open) {
+          this._closedSections.delete(item.sectionLabel);
+        } else {
+          this._closedSections.add(item.sectionLabel);
+        }
+      });
+    }
+
     let sectionIndex = 0;
     const accordionItems = glyphSections.map((section) => ({
       label: html.span({}, [
@@ -111,7 +123,8 @@ export class GlyphCellView extends HTMLElement {
           makeGlyphCountString(section.glyphs, this.fontController.glyphMap),
         ]),
       ]),
-      open: true,
+      sectionLabel: section.label, // not part of Accordion data, this is for us
+      open: !this._closedSections.has(section.label),
       content: html.div({ class: "font-overview-accordion-item" }, []),
       glyphs: section.glyphs,
       sectionIndex: sectionIndex++,

diff --git a/src/fontra/client/web-components/ui-accordion.js b/src/fontra/client/web-components/ui-accordion.js
@@ -72,7 +72,7 @@ export class Accordion extends UnlitElement {
         {
           class: "ui-accordion-item-header",
           onclick: (event) =>
-            this._handleItemHeaderClick(event, itemElement, itemElements),
+            this._handleItemHeaderClick(event, item, itemElement, itemElements),
         },
         [
           html.createDomElement("inline-svg", {
@@ -116,16 +116,18 @@ export class Accordion extends UnlitElement {
     return this.shadowRoot.querySelectorAll(selector);
   }
 
-  _handleItemHeaderClick(event, itemElement, itemElements) {
+  _handleItemHeaderClick(event, item, itemElement, itemElements) {
     if (event.altKey) {
       // Toggle all items depending on the open/closed state of the clicked item
-      const onOff = !itemElement.classList.contains("ui-accordion-item-closed");
+      const doClose = !itemElement.classList.contains("ui-accordion-item-closed");
       itemElements.forEach((itemElement) =>
-        itemElement.classList.toggle("ui-accordion-item-closed", onOff)
+        itemElement.classList.toggle("ui-accordion-item-closed", doClose)
       );
+      this.items.forEach((item) => (item.open = !doClose));
     } else {
       // Toggle single item
       itemElement.classList.toggle("ui-accordion-item-closed");
+      item.open = !item.open;
     }
   }
 

diff --git a/src/fontra/views/fontoverview/fontoverview.js b/src/fontra/views/fontoverview/fontoverview.js
@@ -66,7 +66,7 @@ export class FontOverviewController extends ViewController {
     });
 
     this.fontOverviewSettingsController.addKeyListener("groupByKeys", (event) => {
-      this.glyphOrganizer.setGroupings(event.newValue);
+      this.glyphOrganizer.setGroupByKeys(event.newValue);
       this.updateGlyphSelection();
     });
 

diff --git a/src/fontra/views/fontoverview/panel-navigation.js b/src/fontra/views/fontoverview/panel-navigation.js
@@ -1,4 +1,8 @@
-import { GlyphOrganizer } from "/core/glyph-organizer.js";
+import {
+  GlyphOrganizer,
+  groupByKeys,
+  groupByProperties,
+} from "/core/glyph-organizer.js";
 import * as html from "/core/html-utils.js";
 import { translate } from "/core/localization.js";
 import { ObservableController } from "/core/observable-object.js";
@@ -59,20 +63,9 @@ export class FontOverviewNavigation extends HTMLElement {
       ]
     );
 
-    const groupByProperties = [
-      ["script", "Script"],
-      ["category", "Category"],
-      ["subCategory", "Sub-category"],
-      ["case", "Case"],
-      ["glyphNameExtension", "Glyph name extension"],
-    ];
-
-    const groupByKeys = groupByProperties.map((item) => item[0]);
-
     const groupByController = new ObservableController({});
 
-    groupByController.addKeyListener(
-      groupByKeys,
+    groupByController.addListener(
       (event) =>
         (this.fontOverviewSettings.groupByKeys = groupByKeys.filter(
           (key) => groupByController.model[key]
@@ -81,7 +74,7 @@ export class FontOverviewNavigation extends HTMLElement {
 
     const groupByContainer = html.div({}, [
       html.span({}, ["Group by"]),
-      ...groupByProperties.map(([key, label]) =>
+      ...groupByProperties.map(({ key, label }) =>
         labeledCheckbox(label, groupByController, key)
       ),
     ]);