Media options: improvements (#10888)

Author: adoriandoran

apps/client/src/translations/en/translation.json

@@ -1877,7 +1877,6 @@
     "images_section_title": "Images",
     "download_images_automatically": "Download images automatically",
     "download_images_description": "Download referenced online images from pasted HTML so they are available offline.",
-    "enable_image_compression": "Image compression",
     "automatic_image_compression": "Automatically compress images",
     "enable_image_compression_description": "Compress and resize images when they are uploaded or pasted.",
     "max_image_dimensions": "Max image dimensions",

apps/client/src/widgets/react/Card.css

@@ -23,13 +23,24 @@
 }
 
 .tn-card-heading {
+    display: flex;
+    align-items: center;
     margin-bottom: 10px;
     font-size: .75rem;
     font-weight: 600;
     letter-spacing: .4pt;
     text-transform: uppercase;
 }
 
+/* Kept at the far end of the heading line, where the options page's own sections keep theirs, and
+   sized against the heading rather than against a full-height toolbar. */
+.tn-card-heading-actions {
+    --icon-button-size: 28px;
+    display: flex;
+    gap: 4px;
+    margin-inline-start: auto;
+}
+
 .tn-card-heading:has(+ .tn-card-description) {
     margin-bottom: 4px;
 }

apps/client/src/widgets/react/Card.tsx

@@ -33,11 +33,23 @@ export interface CardProps {
     heading?: string;
     /** Sentence introducing the card, shown between the heading and the first section. */
     description?: ComponentChildren;
+    /**
+     * Controls for the card as a whole, kept at the far end of its heading — a help mark, or a
+     * button that adds to what the card holds.
+     *
+     * Taken as children rather than named one by one, so that the card needs no import of its own
+     * for them: `Card` is in the login and setup bundles, which have no business pulling in the app
+     * a help mark would reach for.
+     */
+    actions?: ComponentChildren;
 }
 
 export function Card(props: {children: ComponentChildren} & CardProps) {
     return <div className={clsx("tn-card", props.className)}>
-        {props.heading && <h5 class="tn-card-heading">{props.heading}</h5>}
+        {(props.heading || props.actions) && <h5 class="tn-card-heading">
+            {props.heading}
+            {props.actions && <span className="tn-card-heading-actions">{props.actions}</span>}
+        </h5>}
         {props.description && <p className="tn-card-description">{props.description}</p>}
         <div className="tn-card-body">
             {props.children}

apps/client/src/widgets/type_widgets/options/media.css

@@ -1,11 +1,18 @@
-/* A real card where the rest of the page is options sections. It keeps the card styling the theme
-   already gives every other one — the wrapper is here for the page's own spacing, so the card sits
-   where a section would rather than flush against the one above it. */
-.media-image-compression {
-    /* The choice buttons and their labels sit on one line in a dialog sized to its content; a
-       settings page is only as wide as its pane, and a narrow one has to be allowed to stack them
-       rather than push the card wider than the page. */
-    .image-compression-section {
-        flex-wrap: wrap;
-    }
+/* The choice buttons and their labels sit on one line in a dialog sized to its content; a settings
+   page is only as wide as its pane, and a narrow one has to be allowed to stack them rather than
+   push the card wider than the page. */
+.media-images .image-compression-section {
+    flex-wrap: wrap;
+}
+
+/* The width the compression sliders were given, so the two cards read as one page. */
+.media-ocr .slider {
+    width: 140px;
+}
+
+/* Wide enough for the sentence written across it, and fixed: the label beside it would otherwise
+   take the row's slack and leave the bar too narrow to read its own progress in. */
+.media-batch-ocr .media-batch-ocr-progress {
+    flex: 0 0 200px;
+    height: 24px;
 }

apps/client/src/widgets/type_widgets/options/media.spec.tsx

@@ -3,8 +3,9 @@ import { act } from "preact/test-utils";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 
 /**
- * The image compression card: the tool's own rows, driving the options that govern every image
- * arriving in the database rather than a run the user is watching.
+ * The images card: everything that happens to an image on its way in, compression included — the
+ * tool's own rows, driving the options that govern every image arriving in the database rather than
+ * a run the user is watching.
  *
  * What is worth holding here is the wiring, since the rows themselves are tested where they live —
  * that each row reads and writes the option it stands for, that the group hangs off the switch
@@ -73,13 +74,46 @@ function open(overrides: Record<string, string> = {}) {
 
 /** The card's rows, in the order they are drawn, named by their titles. */
 function rowTitles(): (string | undefined)[] {
-    return [ ...host.querySelectorAll(".media-image-compression .image-compression-section") ]
-        .map((row) => row.querySelector(".image-compression-section-title")?.textContent ?? undefined);
+    return rows().map(titleOf);
+}
+
+function rows(): HTMLElement[] {
+    return [ ...host.querySelectorAll<HTMLElement>(".media-images .tn-card-section") ];
 }
 
 function row(title: string): HTMLElement | undefined {
-    return [ ...host.querySelectorAll<HTMLElement>(".media-image-compression .image-compression-section") ]
-        .find((candidate) => candidate.querySelector(".image-compression-section-title")?.textContent === title);
+    return rows().find((candidate) => titleOf(candidate) === title);
+}
+
+/**
+ * What a row is called, whichever of the two kinds it is: the page's own option rows carry the
+ * sentence inside the label, so the title is the text ahead of it, where the compression tool's
+ * rows keep the title in an element of its own.
+ */
+function titleOf(row: Element): string | undefined {
+    const label = row.querySelector(".tn-card-option-label");
+
+    return label
+        ? label.childNodes[0]?.textContent ?? undefined
+        : row.querySelector(".image-compression-section-title")?.textContent ?? undefined;
+}
+
+/** The sentence beneath a row's title, from whichever of the two kinds of row it is. */
+function describes(title: string): string | undefined {
+    const row = rowOrFail(title);
+
+    return row.querySelector(".tn-card-option-description")?.textContent
+        ?? row.querySelector(".image-compression-section-description")?.textContent
+        ?? undefined;
+}
+
+function rowOrFail(title: string): HTMLElement {
+    const found = row(title);
+    if (!found) {
+        throw new Error(`No row titled "${title}".`);
+    }
+
+    return found;
 }
 
 /** Presses one of a row's choice buttons by its label. */
@@ -102,15 +136,17 @@ afterEach(() => {
     vi.clearAllMocks();
 });
 
-describe("the image compression card", () => {
+describe("the images card", () => {
     it("hangs the whole group off its switch, nesting what qualifies each choice", () => {
         open();
 
-        // What an untouched install shows, in order: the switch, scaling with its bound, then one
-        // exclusive choice per format. Recompressing a JPEG brings a quality with it; optimizing a
-        // PNG does not, there being no quality to reducing it to a palette — so only one of the two
-        // choices carries a nested row here.
+        // What an untouched install shows, in order: fetching a referenced image at all, then the
+        // switch over what is done to the ones that arrive, scaling with its bound, and one
+        // exclusive choice per format. Recompressing a JPEG brings a quality with it; optimizing
+        // a PNG does not, there being no quality to reducing it to a palette — so only one of the
+        // two choices carries a nested row here.
         expect(rowTitles()).toEqual([
+            "images.download_images_automatically",
             "images.automatic_image_compression",
             "space_usage.compress_resize",
             "space_usage.compress_max_dimensions",
@@ -120,9 +156,13 @@ describe("the image compression card", () => {
         ]);
 
         // Switched off, the settings are not merely greyed out but gone: there is nothing for them
-        // to govern, and a bound sitting there would read as one still in force.
+        // to govern, and a bound sitting there would read as one still in force. What the switch
+        // does not govern stays where it was — the card is not the compression alone.
         open({ compressImages: "false" });
-        expect(rowTitles()).toEqual([ "images.automatic_image_compression" ]);
+        expect(rowTitles()).toEqual([
+            "images.download_images_automatically",
+            "images.automatic_image_compression"
+        ]);
     });
 
     it("drops a quality that no longer qualifies anything", () => {
@@ -131,6 +171,7 @@ describe("the image compression card", () => {
         // Neither format is being re-encoded, so neither quality is in force — and resizing is
         // still on offer, being the one step that reaches an image whatever its encoding.
         expect(rowTitles()).toEqual([
+            "images.download_images_automatically",
             "images.automatic_image_compression",
             "space_usage.compress_resize",
             "space_usage.compress_max_dimensions",
@@ -146,9 +187,6 @@ describe("the image compression card", () => {
     it("carries the sentences the settings had before they were rows", () => {
         open();
 
-        const describes = (title: string) =>
-            row(title)?.querySelector(".image-compression-section-description")?.textContent;
-
         expect(describes("images.automatic_image_compression")).toBe("images.enable_image_compression_description");
         expect(describes("space_usage.compress_resize")).toBe("images.max_image_dimensions_description");
         // Both qualities take the same advice, being the same scale read twice.