Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions apps/palette-generator/palette-generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { render } from "preact";
import { html } from "htm/preact";
import { Palette } from "../../src/client/Palette";
import { BlissSymbol } from "../../src/client/BlissSymbol";
import { processPaletteLabels, fetchBlissGlossJson } from "./paletteJsonGenerator";
import { processPaletteLabels, loadBciAvSymbolsDict } from "./paletteJsonGenerator";
import "../../src/client/index.scss";
import {
initAdaptivePaletteGlobals, adaptivePaletteGlobals, cellTypeRegistry
Expand All @@ -22,7 +22,7 @@ import {

// Initialize any globals used elsewhere in the code.
await initAdaptivePaletteGlobals("paletteDisplay");
await fetchBlissGlossJson();
await loadBciAvSymbolsDict();
let currentPaletteName = "";

const MAX_MATCHES_OUTPUT = 7;
Expand All @@ -32,7 +32,7 @@ const MAX_MATCHES_OUTPUT = 7;
* set up a handler to adjust the palette for changes in the cell type.
*/
function initCellTypesSelect () {
const cellTypesSelect = document.getElementById("cellTypes");
const cellTypesSelect = document.getElementById("cellTypes") as HTMLSelectElement;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the as HTMLSelectElement because VSCode's linter complains, where the npm run lint script does not? If so, that's interesting because I frequently find these type errors when writing and running tests. The test(s) fail with an error like cellTypesSelect has no add() method. After I add as HTMLSelectElement, the test runs without that error.

Note: because this is an app-demo script, we decided not the write tests for them, so this was not reported by the test framework either.

Object.keys(cellTypeRegistry).forEach ((cellType) => {
// The "cell" type `ContentBmwEncoding` is for an array of symbols within
// a content area, not for cells within a palette. Avoid for now.
Expand All @@ -58,7 +58,7 @@ function renderExamples() {
isPresentation=false
labelledBy="slashExampleLabel"
/>
`, document.getElementById("slashExample"));
`, document.getElementById("slashExample") as HTMLElement);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The as HTMLElement is odd. The type of the second argument of render() is Element since the container can be an HTMLElement, an SVGSVGElement, a MathMLElement and other kinds of Elements. The return type for document.getElementById() is Element. Is this another place where VSCode's linter issued an error? In this case, I don't see why.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the Type Assertions docs it would seem that HTMLElement would be inferred anyways. I think this information would be redundant unless there is a desire to be explicit here.


// Semi-colon example
render(html`
Expand All @@ -68,7 +68,7 @@ function renderExamples() {
isPresentation=false
labelledBy="semicolonExampleLabel"
/>
`, document.getElementById("semicolonExample"));
`, document.getElementById("semicolonExample") as HTMLElement);

// Kerning example (relative kerning)
render(html`
Expand All @@ -78,7 +78,7 @@ function renderExamples() {
isPresentation=false
labelledBy="kerningExampleLabel"
/>
`, document.getElementById("kerningExample"));
`, document.getElementById("kerningExample") as HTMLElement);

// X example
render(html`
Expand All @@ -88,7 +88,7 @@ function renderExamples() {
isPresentation=false
labelledBy="XExampleLabel"
/>
`, document.getElementById("XExample"));
`, document.getElementById("XExample") as HTMLElement);
}

/**
Expand Down Expand Up @@ -123,7 +123,7 @@ function handleGenerateDisplayButton () {
if (glossesArray.length === 0) {
paletteDisplay.innerText = "<p>Missing glosses ?</p>";
}
const lookupResults = processPaletteLabels(
const lookupResults = await processPaletteLabels(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The await is not necessary. The following error is output from npm run serveAppsDemos:

  ✘ [ERROR] "await" can only be used inside an "async" function

    apps/palette-generator/palette-generator.ts:126:24:
      126 │   const lookupResults = await processPaletteLabels(

While processPaletteLabels() is actually declared as async, it does not appear to be asynchronous.

glossesArray,
paletteName.value,
parseInt(rowStart.value),
Expand Down
157 changes: 104 additions & 53 deletions apps/palette-generator/paletteJsonGenerator.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2024-2025 Inclusive Design Research Centre, OCAD University
* Copyright 2024-2026 Inclusive Design Research Centre, OCAD University
* All rights reserved.
*
* Licensed under the New BSD license. You may not use this file except in
Expand All @@ -9,25 +9,55 @@
* https://github.com/inclusive-design/adaptive-palette/blob/main/LICENSE
*/
import { v4 as uuidv4 } from "uuid";
import { bciAvSymbolsDictUrl, loadDataFromUrl } from "../../src/client/GlobalData";
import {
makeBciAvIdType, BCIAV_PATTERN_KEY, BLISSARY_PATTERN_KEY, decomposeBciAvId
} from "../../src/client/SvgUtils";
import { BciAvSymbolsDict } from "../../src/client";

const BLANK_CELL = "BLANK";
const SVG_PREFIX = "SVG:";
const SVG_SUFFIX = ":SVG";
const LABEL_MARKER = "LABEL:";

let bliss_gloss;
export async function fetchBlissGlossJson () {
// Read and parse the Bliss gloss JSON file
try {
const fetchResponse = await fetch("/data/bliss_symbol_explanations.json");
bliss_gloss = await fetchResponse.json();
} catch (error) {
console.error(`Error fetching 'bliss_symbol_explanations.json': ${error.message}`);
}
return bliss_gloss;
let bciAvSymbolsDict: BciAvSymbolsDict = [];

// Load the BCI AV symbols dictionary from the URL
export async function loadBciAvSymbolsDict() {
bciAvSymbolsDict = await loadDataFromUrl<BciAvSymbolsDict>(bciAvSymbolsDictUrl);
}
Comment on lines +23 to +28
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious about this approach for a specific load function. It seem there are other functions in this file that also require the symbols.

I thought that import caches, so it only happens once across all the files. Does fetch also cache?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know about caching. I do know that a dynamic import using the import(...) function is implemented as fetch(), ultimately. Maybe static import is also implemented as fetch? In this case, the result of loadDataFromUrl() is stored in the variable bciAvSymbolsDict, which is how other functions within the file get access to the symbols. That variable is within the memory space of the web page -- is that memory cached anywhere?

The main reason for replacing static import with fetch is to avoid the build warning message that some chunks are too big. The build process puts data that is imported statically into the dist folder hierarchy. That does not happen for data that is accessed via fetch().

PS. In this case (line 27), loadDataFromUrl() is not replacing a static import. It's replacing a direct call to fetch(), if that matters.


// Define types used in the functions below
interface BlissGloss {
id: string | number;
description: string;
composition?: (string | number)[]; // Optional
}

interface BciAvMatch {
bciAvId: number;
label: string;
composition?: (string | number)[]; // Optional
fullComposition?: (string | number)[]; // Optional
}

type MatchByInfo = Record<string, BciAvMatch[]>;

interface Cell {
type: string;
options: {
label: string;
bciAvId?: number | (string | number)[];
rowStart: number;
rowSpan: number;
columnStart: number;
columnSpan: number;
};
}

interface Palette {
name: string;
cells: Record<string, Cell>;
}

/**
Expand All @@ -36,7 +66,7 @@ export async function fetchBlissGlossJson () {
* @param {string} - the string to test.
* @returns {boolean}
*/
function isSvgBuilderString (theString) {
function isSvgBuilderString (theString: string) {
return theString.startsWith(SVG_PREFIX) && theString.endsWith(SVG_SUFFIX);
}

Expand All @@ -51,7 +81,7 @@ function isSvgBuilderString (theString) {
* @return {Array} - An array of the specifiers required by the SvgUtils.
* @throws {Error} - If the encoding is not well formed.
*/
function convertSvgBuilderString (theString) {
function convertSvgBuilderString (theString: string) {
let result;
// Three forms, one with commas and one without:
// - commas:
Expand Down Expand Up @@ -96,43 +126,62 @@ function convertSvgBuilderString (theString) {
* { id: {number}, description: {string}, ... }
* @throws {Error} If no BCI AV ID is found for the label.
*/
function findBciAvId(label, blissGlosses) {
const matches = [];
// Search for the label in the Bliss gloss

/**
* Helper function: Normalizes polymorphic return from decomposeBciAvId
* (value | array | undefined) into (array | undefined)
*/
const normalizeToOptionalArray = (val: unknown): (string | number)[] | undefined => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is cool. We might want to move this or some form of it to the GlobalUtils.ts. There are many places where I've had to add checks for a given BciAvIdType variable to see if it was the array or the number form and branch accordingly. I've often thought that there has to be a better way.

if (val === undefined || val === null) return undefined;
if (Array.isArray(val)) {
return val as (string | number)[];
}
return [val as string | number];
};

/**
* Helper function: Compares two arrays for equality safely
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe "Helper function: Compare two arrays for equality safely, where each item in the array is either a string or a number". What do you think?

*/
const areArraysEqual = (a?: (string | number)[], b?: (string | number)[]): boolean => {
if (!a || !b) return a === b; // If both undefined, they are equal
if (a.length !== b.length) return false;
return a.every((val, index) => String(val) === String(b[index]));
};

function findBciAvId(label: string, blissGlosses: BlissGloss[]): BciAvMatch[] {
const matches: BciAvMatch[] = [];
console.log(`For label ${label}:`);

for (const gloss of blissGlosses) {
// Try an exact match or a word match
const wordMatch = new RegExp("\\b" + `${label}` + "\\b");
if ((label === gloss.description) || wordMatch.test(gloss.description)) {
// Get the composition of all the parts of the symbol's compostion or
// its ID. But if the `fullComposition` is the same as the original
// ignore it.
const glossId = parseInt(gloss.id);
let fullComposition = undefined;
let equalCompositions = false;
if (gloss.composition) {
fullComposition = decomposeBciAvId(gloss.composition);
equalCompositions = fullComposition.join("") === gloss.composition.join("");
}
else {
fullComposition = decomposeBciAvId(glossId);
if (fullComposition && fullComposition.length === 1) {
equalCompositions = fullComposition[0] === glossId;
}
}
const wordMatch = new RegExp("\\b" + label + "\\b");

if (label === gloss.description || wordMatch.test(gloss.description)) {
const glossId = typeof gloss.id === "number" ? gloss.id : parseInt(gloss.id);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that the check for a "number" type is needed. According to MDN, parseInt() first converts its argument to a string and then parses that. So passing an actual number is fine.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may also want to include the radix parameter to be explicit about what base the number has.


// Determine the source for decomposition and what "original" state to compare against
const source = gloss.composition ?? glossId;
const originalAsArray = gloss.composition ?? [glossId];

const fullComp = normalizeToOptionalArray(decomposeBciAvId(source));

// Check if the decomposition resulted in exactly what we started with
const equalCompositions = areArraysEqual(fullComp, originalAsArray);

matches.push({
bciAvId: glossId,
label: gloss.description,
composition: gloss.composition,
fullComposition: ( equalCompositions ? undefined : fullComposition )
composition: gloss.composition, // Might be undefined
fullComposition: equalCompositions ? undefined : fullComp
});
console.log(`\tFound match: ${gloss.description}, bci-av-id: ${gloss.id}`);

console.log(`\tFound match: ${gloss.description}, bci-av-id: ${glossId}`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding glossId in this log statement: what is the reason to use a number, where before it used the original string value gloss.id?

}
}
// If no BCI AV ID is found, throw an error

if (matches.length === 0) {
throw new Error(`BciAvId not found for label: ${label}`);
}

return matches;
}

Expand All @@ -145,7 +194,7 @@ function findBciAvId(label, blissGlosses) {
* @returns {Object} The object that matches the given BCI AV ID
* @throws {Error} If the given BCI AV ID is invalid (not in the gloss)
*/
function findByBciAvId (bciAvId: string, blissGlosses: array) {
function findByBciAvId (bciAvId: string, blissGlosses: BlissGloss[]) {
const theEntry = blissGlosses.find((entry) => (entry.id === bciAvId));
if (theEntry === undefined) {
throw new Error(`BciAvId not found for BCI AV ID: ${bciAvId}`);
Expand Down Expand Up @@ -197,17 +246,18 @@ function findByBciAvId (bciAvId: string, blissGlosses: array) {
* was no match in the gloss
* }
*/
export function processPaletteLabels (paletteLabels, paletteName, startRow, startColumn, cellType) {

export async function processPaletteLabels (paletteLabels: string[][], paletteName: string, startRow: number, startColumn: number, cellType: string) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unneccessary async. At least, I can't tell why this is asynchronous.

// Initialize palette to return, the matches, and the error list
const finalJson = {
const finalJson: Palette = {
"name": paletteName,
"cells": {}
};
const matchByInfoArray = [];
const errors = [];
const matchByInfoArray: MatchByInfo[] = [];
const errors: string[] = [];

paletteLabels.forEach((row, rowIndex) => {
row.forEach((infoString, colIndex) => {
paletteLabels.forEach((row: string[], rowIndex: number) => {
row.forEach((infoString: string, colIndex: number) => {
const current_row = startRow + rowIndex;
const current_column = startColumn + colIndex;

Expand All @@ -217,7 +267,7 @@ export function processPaletteLabels (paletteLabels, paletteName, startRow, star
}
// Create a cell object for the current `infoString`, leaving the
// `bciAvId` field undefined for now.
const cell = {
const cell: Cell = {
type: cellType,
options: {
label: infoString,
Expand All @@ -243,27 +293,28 @@ export function processPaletteLabels (paletteLabels, paletteName, startRow, star
}
else {
// If the `infoString` is a BCI AV ID (a number), just use it for the
// `bciAvId`. Even so, find its description from the `bliss_gloss`
// `bciAvId`. Even so, find its description from the `bciAvSymbolsDict`
cell.options.bciAvId = parseInt(infoString);
if (!isNaN(cell.options.bciAvId)) {
const glossEntry = findByBciAvId(infoString, bliss_gloss);
const glossEntry = findByBciAvId(infoString, bciAvSymbolsDict);
cell.options.label = actualLabel || glossEntry["description"];
}
else {
// Find the BCI AV IDs for the current infoString. Use the first
// match for the palette
const matches = findBciAvId(infoString, bliss_gloss);
const matches = findBciAvId(infoString, bciAvSymbolsDict);
cell.options.bciAvId = matches[0].bciAvId;
cell.options.label = actualLabel || infoString;
const inputMatches = {};
const inputMatches: MatchByInfo = {};
inputMatches[infoString] = matches;
matchByInfoArray.push(inputMatches);
}
}
}
catch (error) {
catch (error: unknown) {
// If an error occurs, add it to the errors array
errors.push(error.message);
const errorMessage = error instanceof Error ? error.message : String(error);
errors.push(errorMessage);

// Change the cell label to indicate that this cell is not right yet.
// The `bciAvId` encoding means "not found".
Expand Down
Loading