feat!: Replace isAccessibleCallback with diagnosticsSummaryCallback in DoenetEditor

.changeset/diagnostics-summary-callback.md

@@ -0,0 +1,9 @@
+---
+"@doenet/doenetml": patch
+"@doenet/standalone": patch
+"@doenet/doenetml-iframe": patch
+"@doenet/vscode-extension": patch
+"doenet-vscode-extension": patch
+---
+
+Replace `isAccessibleCallback` with `diagnosticsSummaryCallback` in `DoenetEditor`. The new callback receives an object with counts for `warningsCount`, `errorsCount`, `infosCount`, `accessibilityLevel1Count`, and `accessibilityLevel2Count` instead of a single boolean. The callback is only invoked after diagnostics have been received from the viewer.

packages/doenetml/src/EditorViewer/EditorViewer.tsx

@@ -24,6 +24,7 @@ import { ViewerControlsBar } from "./ViewerControlsBar";
 import "./editor-viewer.css";
 import { useTabStore } from "@ariakit/react";
 import { setVariantsFromCallback } from "../utils/variants";
+import type { DiagnosticsSummary } from "./diagnostics";
 import {
     mergeDiagnosticsByType,
     toAdditionalDiagnosticsForLsp,
@@ -47,7 +48,7 @@ export function EditorViewer({
     doenetmlChangeCallback,
     immediateDoenetmlChangeCallback,
     documentStructureCallback,
-    isAccessibleCallback,
+    diagnosticsSummaryCallback,
     id: specifiedId,
     readOnly = false,
     showFormatter = true,
@@ -71,7 +72,9 @@ export function EditorViewer({
     doenetmlChangeCallback?: Function;
     immediateDoenetmlChangeCallback?: Function;
     documentStructureCallback?: Function;
-    isAccessibleCallback?: (isAccessible: boolean) => void;
+    diagnosticsSummaryCallback?: (
+        diagnosticsSummary: DiagnosticsSummary,
+    ) => void;
     id?: string;
     readOnly?: boolean;
     showFormatter?: boolean;
@@ -131,11 +134,18 @@ export function EditorViewer({
     const [infoPanelIsOpen, setInfoPanelIsOpen] = useState(false);
 
     const [diagnostics, setDiagnostics] = useState<DiagnosticRecord[]>([]);
+    // Track whether we've received diagnostics from the viewer at least once.
+    // This is used to gate when we call the diagnosticsSummaryCallback to avoid sending
+    // just the initial diagnostics on load, which would be misleading since that
+    // does not include most of the diagnostics, which are generated by the viewer.
+    const [receivedDiagnosticsFromViewer, setReceivedDiagnosticsFromViewer] =
+        useState(false);
     const [showInfoAnnotations, setShowInfoAnnotations] = useState(false);
 
     /** Receives diagnostics from DocViewer and stores them for panel/LSP sync. */
     function setDiagnosticsCallback(newDiagnostics: DiagnosticRecord[]) {
         setDiagnostics(newDiagnostics);
+        setReceivedDiagnosticsFromViewer(true);
     }
 
     useEffect(() => {
@@ -155,6 +165,9 @@ export function EditorViewer({
         errors: errorsObjs,
         infos: infoObjs,
         accessibility: accessibilityObjs,
+        warningsCount,
+        errorsCount,
+        infosCount,
         accessibilityLevel1Count,
         accessibilityLevel2Count,
     } = useMemo(
@@ -166,9 +179,44 @@ export function EditorViewer({
         [initialDiagnostics, diagnostics],
     );
 
+    /**
+     * Memoize the diagnosticsSummary object so its reference is stable when
+     * diagnostic counts don't change. This prevents unnecessary re-invocations
+     * of the effect callback and avoids potential render loops if a consumer
+     * stores this object in React state and passes an inline callback.
+     */
+    const diagnosticsSummary = useMemo(
+        () => ({
+            warningsCount,
+            errorsCount,
+            infosCount,
+            accessibilityLevel1Count,
+            accessibilityLevel2Count,
+        }),
+        [
+            warningsCount,
+            errorsCount,
+            infosCount,
+            accessibilityLevel1Count,
+            accessibilityLevel2Count,
+        ],
+    );
+
     useEffect(() => {
-        isAccessibleCallback?.(accessibilityLevel1Count === 0);
-    }, [accessibilityLevel1Count, isAccessibleCallback]);
+        // On initial load of the editor, don't call `diagnosticsSummaryCallback`
+        // until the viewer has sent diagnostics so avoid sending just the initial diagnostics.
+        // We do not need to reset `receivedDiagnosticsFromViewer`, as this guard is just
+        // needed to avoid calling `diagnosticsSummaryCallback` on the initial load of the editor.
+        if (!receivedDiagnosticsFromViewer) {
+            return;
+        }
+
+        diagnosticsSummaryCallback?.(diagnosticsSummary);
+    }, [
+        diagnosticsSummary,
+        receivedDiagnosticsFromViewer,
+        diagnosticsSummaryCallback,
+    ]);
 
     const [responses, setResponses] = useState<
         {

packages/doenetml/src/EditorViewer/diagnostics.ts

@@ -1,10 +1,11 @@
-import {
+import type {
+    AccessibilityRecord,
     DiagnosticRecord,
     ErrorRecord,
     InfoRecord,
     WarningRecord,
-    isAccessibilityRecord,
 } from "@doenet/utils";
+import { isAccessibilityRecord } from "@doenet/utils";
 import {
     Diagnostic,
     DiagnosticSeverity,
@@ -13,6 +14,15 @@ import {
 /** CodeMirror/LSP diagnostic with optional editor mark class metadata. */
 export type EditorLspDiagnostic = Diagnostic & { markClass?: string };
 
+/** Aggregated counts emitted for editor diagnostics callbacks. */
+export type DiagnosticsSummary = {
+    warningsCount: number;
+    errorsCount: number;
+    infosCount: number;
+    accessibilityLevel1Count: number;
+    accessibilityLevel2Count: number;
+};
+
 /**
  * Converts a single editor diagnostic into an LSP-compatible diagnostic.
  * Accessibility diagnostics are surfaced as warnings with custom source/markClass.
@@ -101,7 +111,12 @@ export function mergeDiagnosticsByType({
 }: {
     initialDiagnostics: DiagnosticRecord[];
     diagnostics: DiagnosticRecord[];
-}) {
+}): {
+    warnings: WarningRecord[];
+    errors: ErrorRecord[];
+    infos: InfoRecord[];
+    accessibility: AccessibilityRecord[];
+} & DiagnosticsSummary {
     const warnings = [
         ...initialDiagnostics.filter(
             (diagnostic): diagnostic is WarningRecord =>
@@ -145,6 +160,9 @@ export function mergeDiagnosticsByType({
         errors,
         infos,
         accessibility,
+        warningsCount: warnings.length,
+        errorsCount: errors.length,
+        infosCount: infos.length,
         accessibilityLevel1Count: accessibility.filter(
             (diagnostic) => diagnostic.level === 1,
         ).length,

packages/doenetml/src/doenetml.tsx

@@ -4,6 +4,7 @@ import React, { useCallback, useEffect, useRef, useState } from "react";
 import { DocViewer } from "./Viewer/DocViewer";
 import { MathJaxContext } from "better-react-mathjax";
 import { mathjaxConfig, isErrorRecord, isWarningRecord } from "@doenet/utils";
+import type { DiagnosticsSummary } from "./EditorViewer/diagnostics";
 import type {
     DiagnosticRecord,
     ErrorRecord,
@@ -323,7 +324,7 @@ export function DoenetEditor({
     doenetmlChangeCallback,
     immediateDoenetmlChangeCallback,
     documentStructureCallback,
-    isAccessibleCallback,
+    diagnosticsSummaryCallback,
     id,
     readOnly = false,
     showFormatter = true,
@@ -353,7 +354,9 @@ export function DoenetEditor({
     doenetmlChangeCallback?: Function;
     immediateDoenetmlChangeCallback?: Function;
     documentStructureCallback?: Function;
-    isAccessibleCallback?: (isAccessible: boolean) => void;
+    diagnosticsSummaryCallback?: (
+        diagnosticsSummary: DiagnosticsSummary,
+    ) => void;
     id?: string;
     readOnly?: boolean;
     showFormatter?: boolean;
@@ -425,7 +428,7 @@ export function DoenetEditor({
             doenetmlChangeCallback={doenetmlChangeCallback}
             immediateDoenetmlChangeCallback={immediateDoenetmlChangeCallback}
             documentStructureCallback={documentStructureCallback}
-            isAccessibleCallback={isAccessibleCallback}
+            diagnosticsSummaryCallback={diagnosticsSummaryCallback}
             id={id}
             readOnly={readOnly}
             showFormatter={showFormatter}

packages/test-cypress/cypress/e2e/EditorViewer/editorViewerAccessibilityReport.cy.js

@@ -89,7 +89,7 @@ describe(
                 );
         });
 
-        it("isAccessibleCallback reports false when level 1 accessibility issues exist", () => {
+        it("diagnosticsSummaryCallback reports level 1 accessibility issue", () => {
             postDoenetML(`
 <styleDefinition styleNumber="112" textColor="#ff9900" />
 <text name="p112" styleNumber="112">Low contrast text</text>
@@ -98,11 +98,14 @@ describe(
             cy.get("#p112").should("contain.text", "Low contrast text");
 
             cy.window().should((win) => {
-                expect(win.returnIsAccessibleCallbackValue()).eq(false);
+                expect(
+                    win.returnDiagnosticsSummaryCallbackValue()
+                        ?.accessibilityLevel1Count,
+                ).eq(1);
             });
         });
 
-        it("isAccessibleCallback reports true when no level 1 accessibility issues exist", () => {
+        it("diagnosticsSummaryCallback reports no level 1 accessibility issue", () => {
             postDoenetML(`
 <styleDefinition styleNumber="113" textColor="#111111" />
 <text name="p113" styleNumber="113">Good contrast text</text>
@@ -111,7 +114,44 @@ describe(
             cy.get("#p113").should("contain.text", "Good contrast text");
 
             cy.window().should((win) => {
-                expect(win.returnIsAccessibleCallbackValue()).eq(true);
+                expect(
+                    win.returnDiagnosticsSummaryCallbackValue()
+                        ?.accessibilityLevel1Count,
+                ).eq(0);
+            });
+        });
+
+        it("diagnosticsSummaryCallback reports warning", () => {
+            postDoenetML(`
+    <text name="t">hello</text>
+    $abc
+`);
+
+            cy.get("#t").should("contain.text", "hello");
+
+            cy.window().should((win) => {
+                const diagnosticsSummary =
+                    win.returnDiagnosticsSummaryCallbackValue();
+                expect(diagnosticsSummary?.warningsCount).eq(1);
+                expect(diagnosticsSummary?.errorsCount).eq(0);
+                expect(diagnosticsSummary?.infosCount).eq(0);
+            });
+        });
+
+        it("diagnosticsSummaryCallback reports error", () => {
+            postDoenetML(`
+    <text name="t">hello</text>
+    <abc />
+`);
+
+            cy.get("#t").should("contain.text", "hello");
+
+            cy.window().should((win) => {
+                const diagnosticsSummary =
+                    win.returnDiagnosticsSummaryCallbackValue();
+                expect(diagnosticsSummary?.warningsCount).eq(0);
+                expect(diagnosticsSummary?.errorsCount).eq(1);
+                expect(diagnosticsSummary?.infosCount).eq(0);
             });
         });
 

packages/test-cypress/src/CypressTest.tsx

@@ -110,21 +110,25 @@ export function CypressTest() {
     const [includeVariantSelector, setIncludeVariantSelector] = useState(
         testSettings.includeVariantSelector,
     );
-    const [isAccessible, setIsAccessible] = useState<boolean | null>(null);
-    const isAccessibleRef = useRef<boolean | null>(null);
-    isAccessibleRef.current = isAccessible;
+    const diagnosticsSummaryRef = useRef<Record<string, number> | null>(null);
 
     useEffect(() => {
-        (window as any).returnIsAccessibleCallbackValue = ():
-            | boolean
-            | null => {
-            return isAccessibleRef.current;
+        (window as any).returnDiagnosticsSummaryCallbackValue = (): Record<
+            string,
+            number
+        > | null => {
+            return diagnosticsSummaryRef.current;
         };
         return () => {
-            delete (window as any).returnIsAccessibleCallbackValue;
+            delete (window as any).returnDiagnosticsSummaryCallbackValue;
         };
     }, []);
 
+    // Reset diagnosticsSummaryRef when new DoenetML is posted to avoid stale values
+    useEffect(() => {
+        diagnosticsSummaryRef.current = null;
+    }, [doenetMLstring]);
+
     const solutionDisplayMode = "button";
 
     // requestedVariantIndex is undefined by default so that viewer
@@ -590,8 +594,10 @@ export function CypressTest() {
                 showAnswerResponseButton={answerResponseCounts !== undefined}
                 answerResponseCounts={answerResponseCounts}
                 readOnly={readOnly}
-                isAccessibleCallback={(nextIsAccessible: boolean) => {
-                    setIsAccessible(nextIsAccessible);
+                diagnosticsSummaryCallback={(
+                    nextDiagnosticsSummary: Record<string, number>,
+                ) => {
+                    diagnosticsSummaryRef.current = nextDiagnosticsSummary;
                 }}
             />
         );