Merge pull request #180 from pihart/style_pre-import-pdfs

Style and code quality

src/components/Overlay.tsx

@@ -78,13 +78,10 @@ const Overlay = ({ qboard }: { qboard: QBoard }): JSX.Element => {
   return (
     <>
       <VirtualFileInput
-        acceptFiles={async (file) =>
-          qboard.history.execute((await qboard.files.acceptFile(file)).history)
-        }
+        acceptFiles={qboard.files.acceptFile}
         captureRef={(ref) => {
           qboard.globalState.fileInputRef = ref;
         }}
-        multiple
         accept="application/json, application/pdf, image/*"
       />
       <div className={`drop-area ${state.dragActive ? "active" : ""}`} />

src/components/VirtualFileInput.tsx

@@ -1,9 +1,9 @@
 import React, { useRef } from "react";
 
 /**
- * Passed to [[`OverlayButton`]]
+ * Passed to [[`VirtualFileInput`]]
  */
-interface OverlayButtonProps {
+interface VirtualFileInputProps {
   /**
    * An `onChange` callback that is given the extracted `FileList` as {@param files} if and only if
    * * it exists (is non-null), and
@@ -21,11 +21,11 @@ interface OverlayButtonProps {
   captureRef?: (ref: React.RefObject<HTMLInputElement>) => void;
 }
 
-const OverlayButton = ({
+const VirtualFileInput = ({
   acceptFiles,
   captureRef,
   ...attrs
-}: OverlayButtonProps &
+}: VirtualFileInputProps &
   React.DetailedHTMLProps<
     React.InputHTMLAttributes<HTMLInputElement>,
     HTMLInputElement
@@ -51,4 +51,4 @@ const OverlayButton = ({
   );
 };
 
-export default OverlayButton;
+export default VirtualFileInput;

src/lib/clipboard.ts

@@ -67,9 +67,6 @@ export default class ClipboardHandler {
   };
 
   pasteExternal = async (e: ClipboardEvent): Promise<void> => {
-    const historyCommand = await this.files.processFiles(
-      e.clipboardData!.files
-    );
-    this.history.execute(historyCommand);
+    await this.files.processFiles(e.clipboardData!.files);
   };
 }

src/lib/files.ts

@@ -1,35 +1,65 @@
 import { fabric } from "fabric";
 import { MalformedExpressionException, RequireSubType } from "@mehra/ts";
 
-import { HistoryCommand } from "./history";
+import HistoryHandler from "./history";
 import Pages, { PageJSON } from "./pages";
 import { Cursor } from "./page";
 
 const defaults = <T>(value: T | undefined, getDefaultValue: () => T) =>
   value === undefined ? getDefaultValue() : value;
 
 export class AsyncReader {
-  static readAsText = (file: File): Promise<string | ArrayBuffer> =>
+  private static setup = <
+    ReadType extends "Text" | "DataURL" | "ArrayBuffer" | "BinaryString"
+  >(
+    resolve: (
+      value: ReadType extends "Text" | "BinaryString"
+        ? "string"
+        : ReadType extends "DataURL"
+        ? `data:${string}`
+        : ReadType extends "ArrayBuffer"
+        ? ArrayBuffer
+        : never
+    ) => void,
+    reject: (reason: unknown) => void
+  ) => {
+    const reader = new FileReader();
+    reader.onload = () => {
+      resolve(reader.result as never);
+    };
+    reader.onerror = reject;
+    type readFn = (file: File) => void;
+    return (reader as unknown) as ReadType extends "Text"
+      ? { readAsText: readFn }
+      : ReadType extends "BinaryString"
+      ? { readAsBinaryString: readFn }
+      : ReadType extends "DataURL"
+      ? { readAsDataURL: readFn }
+      : ReadType extends "ArrayBuffer"
+      ? { readAsArrayBuffer: readFn }
+      : never;
+  };
+
+  static readAsText = (file: File): Promise<string> =>
     new Promise((resolve, reject) => {
-      const reader = new FileReader();
-      reader.onload = () => {
-        resolve(reader.result!);
-      };
-      reader.onerror = reject;
-      reader.readAsText(file);
+      AsyncReader.setup<"Text">(resolve, reject).readAsText(file);
     });
 
-  static readAsDataURL = (file: File): Promise<string | ArrayBuffer> =>
+  static readAsDataURL = (file: File): Promise<string> =>
     new Promise((resolve, reject) => {
-      const reader = new FileReader();
-      reader.onload = () => {
-        resolve(reader.result!);
-      };
-      reader.onerror = reject;
-      reader.readAsDataURL(file);
+      AsyncReader.setup<"DataURL">(resolve, reject).readAsDataURL(file);
     });
 }
 
+type JSONFile = File & { type: "application/json" };
+type ImageFile = File & { type: `image/${string}` };
+
+const isJSONFile = (file: File): file is JSONFile =>
+  file.type === "application/json";
+
+const isImageFile = (file: File): file is ImageFile =>
+  file.type.startsWith("image/");
+
 /**
  * Common to _all_ versions of exports
  */
@@ -97,7 +127,7 @@ export class JSONReader {
    * @throws {InvalidQboardFileException} if {@param json} doesn't represent a valid qboard file
    */
   static read(json: string | ArrayBuffer): PageJSON[] {
-    const object: unknown = JSON.parse(json.toString());
+    const object = JSON.parse(json.toString());
     return JSONReader.readParsed(object);
   }
 
@@ -170,71 +200,82 @@ export class JSONWriter {
   };
 }
 
-export type FileHandlerResponse = {
-  action: "none" | "image" | "json";
-  history?: HistoryCommand;
-};
-
 export default class FileHandler {
-  constructor(public pages: Pages) {}
+  constructor(public pages: Pages, private history: HistoryHandler) {}
 
-  processFiles = async (
-    files: FileList,
-    cursor?: Cursor
-  ): Promise<HistoryCommand> => {
-    const images: fabric.Object[] = [];
-    await Promise.all(
-      [...files].map(async (file) => {
-        if (file.type.startsWith("image/")) {
-          images.push(await this.handleImage(file, cursor));
-        }
-        if (file.type === "application/json") {
-          return this.handleJSON(file);
-        }
-      })
-    );
-    return {
-      add: images.flat(),
-    };
+  /**
+   * Accepts multiple files, usually via file drop, and performs the equivalent of adding them to qboard in order.
+   *  * Image files are added to the *active page*, at the location of {@param cursor} if it is provided.
+   *  * JSON files representing qboard files have their pages inserted into the page list *after the current page*,
+   *    and then the first page of the inserted file (=current page + 1) is activated.
+   *
+   * Implementation detail: currently *does* add each in order;
+   * this could likely be optimized.
+   * If so, be careful to validate the json files so that the behavior is equivalent to doing each individually.
+   * @param files The ordered list of files
+   */
+  processFiles = async (files: FileList, cursor?: Cursor): Promise<void> => {
+    const additions: fabric.Image[] = [];
+
+    for (const file of files) {
+      if (isImageFile(file)) {
+        // eslint-disable-next-line no-await-in-loop
+        additions.push(await this.handleImage(file, cursor));
+      }
+
+      if (isJSONFile(file)) {
+        // eslint-disable-next-line no-await-in-loop
+        await this.handleJSON(file);
+      }
+    }
+
+    this.history.add(additions);
   };
 
+  /**
+   * Accepts a single file, the first element of {@param files},
+   * usually from file upload through input element, and adds it to the qboard file.
+   *  * Image files are added to the *active page*, at the location of {@param cursor} if it is provided.
+   *  * JSON files representing qboard files completely overwrite the board and history,
+   *    and the first page of the added file (= 1) is loaded.
+   *
+   * @warn
+   * Gives you the history commands you must apply, but you must do it yourself.
+   * This function does not actually modify history.
+   */
   acceptFile = async (
     files: FileList,
     cursor?: Cursor
-  ): Promise<FileHandlerResponse> => {
-    if (!files.length) return { action: "none" };
+  ): Promise<"none" | "image" | "json"> => {
+    if (!files.length) return "none";
     const [file] = files;
 
-    if (file.type.startsWith("image/")) {
-      return {
-        action: "image",
-        history: { add: [await this.handleImage(file, cursor)] },
-      };
+    if (isImageFile(file)) {
+      this.history.add([await this.handleImage(file, cursor)]);
+      return "image";
     }
 
-    if (file.type === "application/json") {
+    if (isJSONFile(file)) {
       await this.openFile(file);
-      return {
-        action: "json",
-        history: { clear: [true] },
-      };
+      this.history.clear(true);
+      return "json";
     }
 
     // unsupported file
-    return { action: "none" };
+    return "none";
   };
 
-  openFile = async (file: File): Promise<boolean> => {
+  openFile = async (file: JSONFile): Promise<boolean> => {
     this.pages.savePage();
     return this.pages.overwritePages(
       JSONReader.read(await AsyncReader.readAsText(file))
     );
   };
 
   private handleImage = async (
-    file: File,
+    file: ImageFile,
     cursor?: Cursor
-  ): Promise<fabric.Object> =>
+  ): Promise<fabric.Image> =>
     AsyncReader.readAsDataURL(file)
       .then((result) => this.pages.canvas.addImage(result.toString(), cursor))
       .then((img) => {
@@ -256,7 +297,7 @@ export default class FileHandler {
         return img;
       });
 
-  private handleJSON = async (file: File): Promise<number> => {
+  private handleJSON = async (file: JSONFile): Promise<number> => {
     const pages = JSONReader.read(await AsyncReader.readAsText(file));
     return this.pages.insertPagesAfter(pages);
   };

src/lib/history.ts

@@ -5,17 +5,17 @@ import Pages from "./pages";
 import { ObjectId } from "../types/fabric";
 
 interface HistoryItem {
-  ids: number[];
+  ids: readonly number[];
   oldObjects: fabric.Object[] | null;
   newObjects: fabric.Object[] | null;
   page: number;
 }
 
-export type HistoryCommand = {
+export interface HistoryCommand {
   add?: fabric.Object[];
   remove?: fabric.Object[];
-  clear?: [boolean];
-};
+  clear?: readonly [boolean];
+}
 
 export default class HistoryHandler {
   history: HistoryItem[] = [];
@@ -29,6 +29,9 @@ export default class HistoryHandler {
     public updateState: () => void
   ) {}
 
+  /**
+   * Behavior is undefined if the same object is in both the `add` and `remove` properties of {@param command}
+   */
   execute = (command: HistoryCommand = {}): void => {
     if (command.clear) this.clear(command.clear[0]);
     this.add(command.add);
@@ -49,7 +52,7 @@ export default class HistoryHandler {
     this.updateState();
   };
 
-  store = (objects: fabric.Object[]): void => {
+  store = (objects: readonly fabric.Object[]): void => {
     if (this.locked) return;
     this.locked = true;
     this.selection = this.canvas.serialize(objects);

src/lib/page.ts

@@ -47,10 +47,10 @@ export default class Page extends fabric.Canvas {
   };
 
   // kind of inefficient
-  getObjectByIds = (ids: number[]): fabric.Object[] =>
+  getObjectByIds = (ids: readonly number[]): fabric.Object[] =>
     this.getObjects().filter((object) => ids.includes((object as ObjectId).id));
 
-  serialize = (objects: fabric.Object[]): fabric.Object[] => {
+  serialize = (objects: readonly fabric.Object[]): fabric.Object[] => {
     const selection = this.getActiveObjects();
     const reselect =
       selection.length > 1 && objects.some((obj) => selection.includes(obj));
@@ -74,7 +74,10 @@ export default class Page extends fabric.Canvas {
     );
   };
 
-  apply = (ids: number[], newObjects: fabric.Object[] | null): void => {
+  apply = (
+    ids: readonly number[],
+    newObjects: fabric.Object[] | null
+  ): void => {
     const oldObjects = this.getObjectByIds(ids);
     if (oldObjects.length) {
       this.remove(...oldObjects);

src/lib/pages.ts

@@ -166,7 +166,7 @@ export default class Pages {
    * Generally set when inserting blank pages, which don't contain any objects.
    */
   insertPagesBefore = async (
-    pages: PageJSON[],
+    pages: PageJSON[] = [defaultPageJSON],
     isNonModifying = false
   ): Promise<number> => {
     this.savePage();
@@ -188,7 +188,7 @@ export default class Pages {
    * Generally set when inserting blank pages, which don't contain any objects.
    */
   insertPagesAfter = async (
-    pages: PageJSON[],
+    pages: PageJSON[] = [defaultPageJSON],
     isNonModifying = false
   ): Promise<number> => {
     this.pagesJSON.splice(this.currentIndex + 1, 0, ...pages);

src/lib/qboard.ts

@@ -4,8 +4,8 @@ import { Network } from "@mehra/ts";
 import instantiateTools, { Tool, Tools } from "./tools";
 import Page from "./page";
 import Pages from "./pages";
-import FileHandler, { JSONReader } from "./files";
 import HistoryHandler from "./history";
+import FileHandler, { JSONReader } from "./files";
 import ClipboardHandler from "./clipboard";
 import StyleHandler, { Dash, Fill, Stroke, Style } from "./styles";
 import ActionHandler from "./action";
@@ -114,12 +114,12 @@ export default class QBoard {
           .catch(console.error);
     }
 
-    this.files = new FileHandler(this.pages);
     this.history = new HistoryHandler(
       this.baseCanvas,
       this.pages,
       this.updateState
     );
+    this.files = new FileHandler(this.pages, this.history);
     this.clipboard = new ClipboardHandler(
       this.baseCanvas,
       this.pages,
@@ -283,10 +283,7 @@ export default class QBoard {
     await this.updateCursor(iEvent);
     this.setDragActive(false);
 
-    const historyCommand = await this.files.processFiles(
-      (iEvent.e as DragEvent).dataTransfer!.files
-    );
-    this.history.execute(historyCommand);
+    await this.files.processFiles((iEvent.e as DragEvent).dataTransfer!.files);
   };
 
   pathCreated: FabricHandler<PathEvent> = (e) => {