Fix: Menu Icon, Menu unresponsive bug, checkbox behavior, constraint text, bind labelModeSelector, remove debug comment

01Parzival10 · 01Parzival10 · commit 4e13533d6bc0 · 2025-07-26T13:07:07.000+02:00
diff --git a/src/features/constraintMenu/ConstraintMenu.ts b/src/features/constraintMenu/ConstraintMenu.ts
@@ -30,6 +30,8 @@ export class ConstraintMenu extends AbstractUIExtension implements Switchable {
     private tree: AutoCompleteTree;
     private forceReadOnly: boolean;
     private optionsMenu?: HTMLDivElement;
+    private ignoreCheckboxChange = false;
+    private hasOpenedOptionsMenu = false;
 
     constructor(
         @inject(ConstraintRegistry) private readonly constraintRegistry: ConstraintRegistry,
@@ -233,7 +235,7 @@ export class ConstraintMenu extends AbstractUIExtension implements Switchable {
         const btn = document.createElement("button");
         btn.id = "constraint-options-button";
         btn.title = "Filter…";
-        btn.innerHTML = "⋮"; // or insert a font-awesome icon
+        btn.innerHTML = '<span class="codicon codicon-kebab-vertical"></span>';
         btn.onclick = () => this.toggleOptionsMenu();
         return btn;
     }
@@ -257,28 +259,39 @@ export class ConstraintMenu extends AbstractUIExtension implements Switchable {
         const allCb = document.createElement("input");
         allCb.type = "checkbox";
         allCb.value = "ALL";
-        // initially checked if no specific constraint is selected
-        allCb.checked = this.constraintRegistry.getSelectedConstraints().includes("ALL");
+        allCb.checked = this.constraintRegistry
+            .getConstraintList()
+            .map((c) => c.name)
+            .every((c) => this.constraintRegistry.getSelectedConstraints().includes(c));
 
         allCb.onchange = () => {
             if (!this.optionsMenu) return;
-            if (allCb.checked) {
-                // uncheck every other constraint-checkbox
-                this.optionsMenu.querySelectorAll<HTMLInputElement>("input[type=checkbox]").forEach((cb) => {
-                    if (cb !== allCb) cb.checked = false;
-                });
-                // dispatch with empty array to mean “all”
-                this.dispatcher.dispatch(ChooseConstraintAction.create(["ALL"]));
-            } else {
-                this.dispatcher.dispatch(ChooseConstraintAction.create([]));
+
+            this.ignoreCheckboxChange = true;
+            try {
+                if (allCb.checked) {
+                    this.optionsMenu.querySelectorAll<HTMLInputElement>("input[type=checkbox]").forEach((cb) => {
+                        if (cb !== allCb) cb.checked = true;
+                    });
+                    this.dispatcher.dispatch(
+                        ChooseConstraintAction.create(this.constraintRegistry.getConstraintList().map((c) => c.name)),
+                    );
+                } else {
+                    this.optionsMenu.querySelectorAll<HTMLInputElement>("input[type=checkbox]").forEach((cb) => {
+                        if (cb !== allCb) cb.checked = false;
+                    });
+                    this.dispatcher.dispatch(ChooseConstraintAction.create([]));
+                }
+            } finally {
+                this.ignoreCheckboxChange = false;
             }
         };
 
         allConstraints.appendChild(allCb);
         allConstraints.appendChild(document.createTextNode("All constraints"));
         this.optionsMenu.appendChild(allConstraints);
 
-        // 2) pull your dynamic items (replace with your real API)
+        // 2) pull your dynamic items
         const items = this.constraintRegistry.getConstraintList();
 
         // 3) for each item build a checkbox
@@ -292,14 +305,14 @@ export class ConstraintMenu extends AbstractUIExtension implements Switchable {
             cb.checked = this.constraintRegistry.getSelectedConstraints().includes(cb.value);
 
             cb.onchange = () => {
-                if (cb.checked) allCb.checked = false;
+                if (this.ignoreCheckboxChange) return;
 
-                const selected = Array.from(
-                    this.optionsMenu!.querySelectorAll<HTMLInputElement>("input[type=checkbox]:checked"),
-                ).map((cb) => cb.value);
+                const checkboxes = this.optionsMenu!.querySelectorAll<HTMLInputElement>("input[type=checkbox]");
+                const individualCheckboxes = Array.from(checkboxes).filter((cb) => cb !== allCb);
+                const selected = individualCheckboxes.filter((cb) => cb.checked).map((cb) => cb.value);
+
+                allCb.checked = individualCheckboxes.every((cb) => cb.checked);
 
-                // dispatch your action with either an array or
-                // a comma-joined string—whatever your action expects
                 this.dispatcher.dispatch(ChooseConstraintAction.create(selected));
             };
 
@@ -312,15 +325,16 @@ export class ConstraintMenu extends AbstractUIExtension implements Switchable {
 
         // optional: click-outside handler
         const onClickOutside = (e: MouseEvent) => {
-            if (
-                this.optionsMenu &&
-                !this.optionsMenu.contains(e.target as Node) &&
-                !(e.target as Element).matches("#constraint-options-button")
-            ) {
-                this.toggleOptionsMenu();
-                document.removeEventListener("click", onClickOutside);
-            }
+            const target = e.target as Node;
+            if (!this.optionsMenu || this.optionsMenu.contains(target)) return;
+
+            const button = document.getElementById("constraint-options-button");
+            if (button && button.contains(target)) return;
+
+            this.optionsMenu.remove();
+            this.optionsMenu = undefined;
+            document.removeEventListener("click", onClickOutside);
         };
-        setTimeout(() => document.addEventListener("click", onClickOutside), 0);
+        document.addEventListener("click", onClickOutside);
     }
 }
diff --git a/src/features/constraintMenu/commands.ts b/src/features/constraintMenu/commands.ts
@@ -20,10 +20,16 @@ export class ChooseConstraintCommand extends Command {
 
     execute(context: CommandExecutionContext): CommandReturn {
         this.annnotationsManager.clearTfgs();
-        const names = this.action.names;
+        let names = this.action.names;
         this.constraintRegistry.setSelectedConstraints(names);
+
+        if (names.includes("INITIAL_CONSTRAINT_STATE")) {
+            this.constraintRegistry.setAllConstraintsAsSelected();
+            names = this.constraintRegistry.getSelectedConstraints();
+        }
+
         const nodes = context.root.children.filter((node) => getBasicType(node) === "node") as DfdNodeImpl[];
-        if (names.includes("NO")) {
+        if (names.length === 0) {
             nodes.forEach((node) => {
                 node.setColor("#1D1C22");
             });
@@ -33,7 +39,7 @@ export class ChooseConstraintCommand extends Command {
         nodes.forEach((node) => {
             const annotations = node.annotations!;
             let wasAdjusted = false;
-            if (names.includes("ALL")) {
+            if (this.constraintRegistry.selectedContainsAllConstraints()) {
                 annotations.forEach((annotation) => {
                     if (annotation.message.startsWith("Constraint")) {
                         wasAdjusted = true;
@@ -43,7 +49,7 @@ export class ChooseConstraintCommand extends Command {
             }
             names.forEach((name) => {
                 annotations.forEach((annotation) => {
-                    if (annotation.message.startsWith("Constraint " + name)) {
+                    if (annotation.message.startsWith("Constraint ") && annotation.message.split(" ")[1] === name) {
                         node.setColor(annotation.color!);
                         wasAdjusted = true;
                         this.annnotationsManager.addTfg(annotation.tfg!);
@@ -53,14 +59,12 @@ export class ChooseConstraintCommand extends Command {
             if (!wasAdjusted) node.setColor("#1D1C22");
         });
 
-        if (!names.includes("ALL") && names.length > 0) {
-            nodes.forEach((node) => {
-                const inTFG = node.annotations!.filter((annotation) =>
-                    this.annnotationsManager.getSelectedTfgs().has(annotation.tfg!),
-                );
-                if (inTFG.length > 0) node.setColor("#77777A", false);
-            });
-        }
+        nodes.forEach((node) => {
+            const inTFG = node.annotations!.filter((annotation) =>
+                this.annnotationsManager.getSelectedTfgs().has(annotation.tfg!),
+            );
+            if (inTFG.length > 0) node.setColor("#77777A", false);
+        });
 
         return context.root;
     }
diff --git a/src/features/constraintMenu/constraintRegistry.ts b/src/features/constraintMenu/constraintRegistry.ts
@@ -9,7 +9,7 @@ export interface Constraint {
 export class ConstraintRegistry {
     private constraints: Constraint[] = [];
     private updateCallbacks: (() => void)[] = [];
-    private selectedConstraints: string[] = [];
+    private selectedConstraints: string[] = this.constraints.map((c) => c.name);
 
     public setConstraints(constraints: string[]): void {
         this.constraints = this.splitIntoConstraintTexts(constraints).map((c) => this.mapToConstraint(c));
@@ -52,6 +52,16 @@ export class ConstraintRegistry {
         return this.constraints;
     }
 
+    public selectedContainsAllConstraints(): boolean {
+        return this.getConstraintList()
+            .map((c) => c.name)
+            .every((c) => this.getSelectedConstraints().includes(c));
+    }
+
+    public setAllConstraintsAsSelected(): void {
+        this.selectedConstraints = this.constraints.map((c) => c.name);
+    }
+
     private splitIntoConstraintTexts(text: string[]): string[] {
         const constraints: string[] = [];
         let currentConstraint = "";
diff --git a/src/features/dfdElements/nodeAnnotationUi.ts b/src/features/dfdElements/nodeAnnotationUi.ts
@@ -14,7 +14,7 @@ import { DfdNodeImpl } from "./nodes";
 
 import "@fortawesome/fontawesome-free/css/all.min.css";
 import "./nodeAnnotationUi.css";
-import { SettingsUI } from "../settingsMenu/settingsMenu";
+import { SettingsManager } from "../settingsMenu/SettingsManager";
 import { Mode } from "../settingsMenu/annotationManager";
 
 export class DfdNodeAnnotationUIMouseListener extends MouseListener {
@@ -100,7 +100,7 @@ export class DfdNodeAnnotationUI extends AbstractUIExtension {
     constructor(
         @inject(DfdNodeAnnotationUIMouseListener)
         private readonly mouseListener: DfdNodeAnnotationUIMouseListener,
-        @inject(SettingsUI) private settings: SettingsUI,
+        @inject(SettingsManager) private settings: SettingsManager,
     ) {
         super();
     }
diff --git a/src/features/dfdElements/nodes.tsx b/src/features/dfdElements/nodes.tsx
@@ -152,6 +152,8 @@ export abstract class DfdNodeImpl extends DynamicChildrenNode implements WithEdi
             opacity: this.opacity.toString(),
         };
 
+        style["--border"] = "#FFFFFF";
+
         if (this.color) style["--color"] = this.color;
 
         /*
diff --git a/src/features/serialize/load.ts b/src/features/serialize/load.ts
@@ -336,7 +336,7 @@ export async function postLoadActions(
     // fit to screen is done after auto layouting because that may change the bounds of the diagram
     // requiring another fit to screen.
     await actionDispatcher.dispatch(createDefaultFitToScreenAction(newRoot, false));
-    actionDispatcher.dispatch(ChooseConstraintAction.create(["ALL"]));
+    actionDispatcher.dispatch(ChooseConstraintAction.create(["INITIAL_CONSTRAINT_STATE"]));
 }
 
 let initialPageTitle: string | undefined;
diff --git a/src/features/settingsMenu/SettingsManager.ts b/src/features/settingsMenu/SettingsManager.ts
@@ -2,6 +2,7 @@ import { inject, injectable } from "inversify";
 import { ActionDispatcher, TYPES } from "sprotty";
 import { ChangeEdgeLabelVisibilityAction, CompleteLayoutProcessAction, SimplifyNodeNamesAction } from "./actions";
 import { LayoutMethod } from "./LayoutMethod";
+import { Mode } from "./annotationManager";
 
 @injectable()
 export class SettingsManager {
@@ -11,6 +12,7 @@ export class SettingsManager {
     private _hideEdgeLabelsCheckbox?: HTMLInputElement;
     private _simplifyNodeNames = false;
     private _simplifyNodeNamesCheckbox?: HTMLInputElement;
+    private _labelModeSelector?: HTMLSelectElement;
     private static readonly layoutMethodLocalStorageKey = "dfdwebeditor:settings";
 
     constructor(@inject(TYPES.IActionDispatcher) protected readonly dispatcher: ActionDispatcher) {
@@ -80,4 +82,13 @@ export class SettingsManager {
             );
         });
     }
+
+    public bindLabelModeSelector(labelModeSelector: HTMLSelectElement) {
+        this._labelModeSelector = labelModeSelector;
+        labelModeSelector.value = Mode.INCOMING;
+    }
+
+    public getCurrentLabelMode(): Mode {
+        return this._labelModeSelector!.value as Mode;
+    }
 }
diff --git a/src/features/settingsMenu/settingsMenu.ts b/src/features/settingsMenu/settingsMenu.ts
@@ -117,12 +117,7 @@ export class SettingsUI extends AbstractUIExtension {
             this.dispatcher.dispatch(ChangeEditorModeAction.create(readOnlyCheckbox.checked ? "view" : "edit"));
         });
 
-        const modeSelect = containerElement.querySelector("#setting-mode-option") as HTMLSelectElement;
-        modeSelect.value = Mode.INCOMING;
-    }
-
-    public getCurrentLabelMode(): Mode {
-        const modeSelect = document.getElementById("setting-mode-option") as HTMLSelectElement;
-        return modeSelect.value as Mode;
+        const labelModeSelector = containerElement.querySelector("#setting-mode-option") as HTMLSelectElement;
+        this.settings.bindLabelModeSelector(labelModeSelector);
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -336,7 +336,7 @@ export async function postLoadActions(`
`336`	`336`	`// fit to screen is done after auto layouting because that may change the bounds of the diagram`
`337`	`337`	`// requiring another fit to screen.`
`338`	`338`	`await actionDispatcher.dispatch(createDefaultFitToScreenAction(newRoot, false));`
`339`		`- actionDispatcher.dispatch(ChooseConstraintAction.create(["ALL"]));`
	`339`	`+ actionDispatcher.dispatch(ChooseConstraintAction.create(["INITIAL_CONSTRAINT_STATE"]));`
`340`	`340`	`}`
`341`	`341`
`342`	`342`	`let initialPageTitle: string \| undefined;`
Original file line number	Diff line number	Diff line change
`@@ -117,12 +117,7 @@ export class SettingsUI extends AbstractUIExtension {`
`117`	`117`	`this.dispatcher.dispatch(ChangeEditorModeAction.create(readOnlyCheckbox.checked ? "view" : "edit"));`
`118`	`118`	`});`
`119`	`119`
`120`		`- const modeSelect = containerElement.querySelector("#setting-mode-option") as HTMLSelectElement;`
`121`		`- modeSelect.value = Mode.INCOMING;`
`122`		`- }`
`123`		`-`
`124`		`- public getCurrentLabelMode(): Mode {`
`125`		`- const modeSelect = document.getElementById("setting-mode-option") as HTMLSelectElement;`
`126`		`- return modeSelect.value as Mode;`
	`120`	`+ const labelModeSelector = containerElement.querySelector("#setting-mode-option") as HTMLSelectElement;`
	`121`	`+ this.settings.bindLabelModeSelector(labelModeSelector);`
`127`	`122`	`}`
`128`	`123`	`}`