Indicate current occlusion type in pop-up menu (#2760)

* Simplify handling of occlusion types in editor code

- Unify updateIONoteInEditMode(), setOcclusionFieldInner() and
setOcclusionField() into updateOcclusionsField()
- Don't use `includeInactive` property of Shape class in editor code
- Drop `isEditMode`

+
Update the occlusions field every time a mask or text is updated, not
only in editing mode but also in adding mode, so that IO cards can be
previewed correctly in the card layout screen

* Indicate current occlusion type in pop-up menu

https://forums.ankiweb.net/t/anki-23-10-beta-5-6/35677/46

* Fix a11y warnings in Toolbar.svelte

* Drop `occludeInactive` parameter from `MaskEditorAPI.getShapes()`

qt/aqt/addcards.py

@@ -373,7 +373,7 @@ class AddCards(QMainWindow):
         self.ifCanClose(doClose)
 
     def add_io_note(self) -> None:
-        self.editor.web.eval("setOcclusionFieldInner()")
+        self.editor.web.eval("updateOcclusionsField();")
         self.add_current_note()
         self.editor.web.eval("resetIOImageLoaded()")
 

qt/aqt/editor.py

@@ -586,7 +586,6 @@ require("anki/ui").loaded.then(() => require("anki/NoteEditor").instances[0].too
             setCloseHTMLTags({json.dumps(self.mw.col.get_config("closeHTMLTags", True))});
             triggerChanges();
             setIsImageOcclusion({json.dumps(self.current_notetype_is_image_occlusion())});
-            setIsEditMode({json.dumps(self.editorMode != EditorMode.ADD_CARDS)});
             """
 
         if self.addMode:

ts/editor/NoteEditor.svelte

@@ -245,11 +245,6 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
         $ioMaskEditorVisible = val;
     }
 
-    let isEditMode = false;
-    function setIsEditMode(val: boolean) {
-        isEditMode = val;
-    }
-
     let cols: ("dupe" | "")[] = [];
     export function setBackgrounds(cls: ("dupe" | "")[]): void {
         cols = cls;
@@ -450,26 +445,12 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
     }
     globalThis.setImageField = setImageField;
 
-    // update cloze deletions and set occlusion fields, it call in saveNow to update cloze deletions
-    function updateIONoteInEditMode() {
-        if (isEditMode) {
-            const clozeNote = get(fieldStores[ioFields.occlusions]);
-            if (clozeNote.includes("oi=1")) {
-                setOcclusionField(true);
-            } else {
-                setOcclusionField(false);
-            }
-        }
-    }
-
-    function setOcclusionFieldInner() {
+    function updateOcclusionsField(): void {
         if (isImageOcclusion) {
             const occlusionsData = exportShapesToClozeDeletions($hideAllGuessOne);
             fieldStores[ioFields.occlusions].set(occlusionsData.clozes);
         }
     }
-    // global for calling this method in desktop note editor
-    globalThis.setOcclusionFieldInner = setOcclusionFieldInner;
 
     // reset for new occlusion in add mode
     function resetIOImageLoaded() {
@@ -482,14 +463,6 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
     }
     globalThis.resetIOImageLoaded = resetIOImageLoaded;
 
-    function setOcclusionField(occludeInactive: boolean) {
-        // set fields data for occlusion and image fields for io notes type
-        if (isImageOcclusion) {
-            const occlusionsData = exportShapesToClozeDeletions(occludeInactive);
-            fieldStores[ioFields.occlusions].set(occlusionsData.clozes);
-        }
-    }
-
     /** hide occlusions and image */
     function hideFieldInOcclusionType(
         index: number,
@@ -583,10 +556,8 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
             setCloseHTMLTags,
             triggerChanges,
             setIsImageOcclusion,
-            setIsEditMode,
             setupMaskEditor,
-            setOcclusionField,
-            setOcclusionFieldInner,
+            updateOcclusionsField,
             ...oldEditorAdapter,
         });
 
@@ -651,7 +622,7 @@ the AddCards dialog) should be implemented in the user of this component.
         <div style="display: {$ioMaskEditorVisible ? 'block' : 'none'}">
             <ImageOcclusionPage
                 mode={imageOcclusionMode}
-                on:change={updateIONoteInEditMode}
+                on:change={updateOcclusionsField}
                 on:image-loaded={onImageLoaded}
             />
         </div>

ts/image-occlusion/Toolbar.svelte

@@ -2,7 +2,7 @@
 Copyright: Ankitects Pty Ltd and contributors
 License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 -->
-<script>
+<script lang="ts">
     import * as tr from "@tslib/ftl";
     import DropdownItem from "components/DropdownItem.svelte";
     import IconButton from "components/IconButton.svelte";
@@ -10,6 +10,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
     import WithFloating from "components/WithFloating.svelte";
 
     import { mdiEye, mdiFormatAlignCenter, mdiSquare, mdiViewDashboard } from "./icons";
+    import { emitChangeSignal } from "./MaskEditor.svelte";
     import { hideAllGuessOne } from "./store";
     import { drawEllipse, drawPolygon, drawRectangle, drawText } from "./tools/index";
     import { makeMaskTransparent } from "./tools/lib";
@@ -72,16 +73,10 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
         canvas.selectionColor = "rgba(100, 100, 255, 0.3)";
     };
 
-    const setOcclusionFieldForDesktop = () => {
-        const clist = document.body.classList;
-        if (
-            clist.contains("isLin") ||
-            clist.contains("isMac") ||
-            clist.contains("isWin")
-        ) {
-            globalThis.setOcclusionFieldInner();
-        }
-    };
+    function changeOcclusionType(occlusionType: "all" | "one"): void {
+        $hideAllGuessOne = occlusionType === "all";
+        emitChangeSignal();
+    }
 </script>
 
 <div class="tool-bar-container">
@@ -100,46 +95,36 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 </div>
 
 <div class="top-tool-bar-container">
-    <div class="undo-redo-button" on:click={() => (showFloating = !showFloating)}>
-        <WithFloating
-            show={showFloating}
-            closeOnInsideClick
-            inline
-            style="line-height: unset !important"
-            on:close={() => (showFloating = false)}
+    <WithFloating
+        show={showFloating}
+        closeOnInsideClick
+        inline
+        on:close={() => (showFloating = false)}
+    >
+        <IconButton
+            class="top-tool-icon-button right-border-radius dropdown-tool-mode"
+            slot="reference"
+            {iconSize}
+            on:click={() => (showFloating = !showFloating)}
         >
-            <IconButton
-                class="top-tool-icon-button right-border-radius dropdown-tool-mode"
-                slot="reference"
-                {iconSize}
-            >
-                {#if $hideAllGuessOne}
-                    {@html mdiViewDashboard}
-                {:else}
-                    {@html mdiSquare}
-                {/if}
-            </IconButton>
+            {@html $hideAllGuessOne ? mdiViewDashboard : mdiSquare}
+        </IconButton>
 
-            <Popover slot="floating" --popover-padding-inline="0">
-                <DropdownItem
-                    on:click={() => {
-                        $hideAllGuessOne = true;
-                        setOcclusionFieldForDesktop();
-                    }}
-                >
-                    <span>{tr.notetypesHideAllGuessOne()}</span>
-                </DropdownItem>
-                <DropdownItem
-                    on:click={() => {
-                        $hideAllGuessOne = false;
-                        setOcclusionFieldForDesktop();
-                    }}
-                >
-                    <span>{tr.notetypesHideOneGuessOne()}</span>
-                </DropdownItem>
-            </Popover>
-        </WithFloating>
-    </div>
+        <Popover slot="floating">
+            <DropdownItem
+                active={$hideAllGuessOne}
+                on:click={() => changeOcclusionType("all")}
+            >
+                <span>{tr.notetypesHideAllGuessOne()}</span>
+            </DropdownItem>
+            <DropdownItem
+                active={!$hideAllGuessOne}
+                on:click={() => changeOcclusionType("one")}
+            >
+                <span>{tr.notetypesHideOneGuessOne()}</span>
+            </DropdownItem>
+        </Popover>
+    </WithFloating>
 
     <!-- undo & redo tools -->
     <div class="undo-redo-button">

ts/image-occlusion/shapes/base.ts

@@ -20,15 +20,14 @@ export class Shape {
     top: number;
     fill: string = SHAPE_MASK_COLOR;
     /** Whether occlusions from other cloze numbers should be shown on the
-     * question side.
+     * question side. Used only in reviewer code.
      */
-    occludeInactive = false;
+    occludeInactive?: boolean;
     /* Cloze ordinal */
     ordinal = 0;
 
     constructor(
-        { left = 0, top = 0, fill = SHAPE_MASK_COLOR, occludeInactive = false, ordinal = 0 }: ConstructorParams<Shape> =
-            {},
+        { left = 0, top = 0, fill = SHAPE_MASK_COLOR, occludeInactive, ordinal = 0 }: ConstructorParams<Shape> = {},
     ) {
         this.left = left;
         this.top = top;
@@ -45,7 +44,6 @@ export class Shape {
             left: floatToDisplay(this.left),
             top: floatToDisplay(this.top),
             ...(this.fill === SHAPE_MASK_COLOR ? {} : { fill: this.fill }),
-            ...(this.occludeInactive ? { oi: "1" } : {}),
         };
     }
 

ts/image-occlusion/shapes/to-cloze.ts

@@ -17,12 +17,12 @@ export function exportShapesToClozeDeletions(occludeInactive: boolean): {
     clozes: string;
     noteCount: number;
 } {
-    const shapes = baseShapesFromFabric(occludeInactive);
+    const shapes = baseShapesFromFabric();
 
     let clozes = "";
     let index = 0;
     shapes.forEach((shapeOrShapes) => {
-        clozes += shapeOrShapesToCloze(shapeOrShapes, index);
+        clozes += shapeOrShapesToCloze(shapeOrShapes, index, occludeInactive);
         if (!(shapeOrShapes instanceof Text)) {
             index++;
         }
@@ -34,7 +34,7 @@ export function exportShapesToClozeDeletions(occludeInactive: boolean): {
 /** Gather all Fabric shapes, and convert them into BaseShapes or
  * BaseShape[]s.
  */
-export function baseShapesFromFabric(occludeInactive: boolean): ShapeOrShapes[] {
+export function baseShapesFromFabric(): ShapeOrShapes[] {
     const canvas = globalThis.canvas as Canvas;
     makeMaskTransparent(canvas, false);
     const activeObject = canvas.getActiveObject();
@@ -53,7 +53,6 @@ export function baseShapesFromFabric(occludeInactive: boolean): ShapeOrShapes[]
             return fabricObjectToBaseShapeOrShapes(
                 canvas,
                 object,
-                occludeInactive,
                 parent,
             );
         })
@@ -64,7 +63,6 @@ export function baseShapesFromFabric(occludeInactive: boolean): ShapeOrShapes[]
 function fabricObjectToBaseShapeOrShapes(
     size: Size,
     object: FabricObject,
-    occludeInactive: boolean,
     parentObject?: FabricObject,
 ): ShapeOrShapes | null {
     let shape: Shape;
@@ -91,14 +89,12 @@ function fabricObjectToBaseShapeOrShapes(
                 return fabricObjectToBaseShapeOrShapes(
                     size,
                     child,
-                    occludeInactive,
                     object,
                 );
             });
         default:
             return null;
     }
-    shape.occludeInactive = occludeInactive;
     if (parentObject) {
         const newPosition = fabric.util.transformPoint(
             { x: shape.left, y: shape.top },
@@ -117,6 +113,7 @@ function fabricObjectToBaseShapeOrShapes(
 function shapeOrShapesToCloze(
     shapeOrShapes: ShapeOrShapes,
     index: number,
+    occludeInactive: boolean,
 ): string {
     let text = "";
     function addKeyValue(key: string, value: string) {
@@ -127,7 +124,7 @@ function shapeOrShapesToCloze(
     let type: string;
     if (Array.isArray(shapeOrShapes)) {
         return shapeOrShapes
-            .map((shape) => shapeOrShapesToCloze(shape, index))
+            .map((shape) => shapeOrShapesToCloze(shape, index, occludeInactive))
             .join("");
     } else if (shapeOrShapes instanceof Rectangle) {
         type = "rect";
@@ -144,6 +141,9 @@ function shapeOrShapesToCloze(
     for (const [key, value] of Object.entries(shapeOrShapes.toDataForCloze())) {
         addKeyValue(key, value);
     }
+    if (occludeInactive) {
+        addKeyValue("oi", "1");
+    }
 
     let ordinal: number;
     if (type === "text") {

ts/image-occlusion/tools/api.ts

@@ -40,8 +40,8 @@ export class MaskEditorAPI {
         return { clozes, cardCount };
     }
 
-    getShapes(occludeInactive: boolean): ShapeOrShapes[] {
-        return baseShapesFromFabric(occludeInactive);
+    getShapes(): ShapeOrShapes[] {
+        return baseShapesFromFabric();
     }
 
     redraw(): void {