From c4b664a834f7c33308a2725744110c404f30f16d Mon Sep 17 00:00:00 2001 From: dovvnloading Date: Tue, 28 Jul 2026 23:10:30 -0400 Subject: [PATCH] R8a: gate notifications on type preference, wire up node font styling (findings #10, #11) Two dead settings from the R8a audit, both with real backend/UI wiring already in place on one side only. - Notification-type gating (finding #10): Settings' Info/Success/Warning/ Error checkboxes persisted real preferences via SettingsManager. set_notification_preferences, and get_notification_type_enabled already existed to read them back - nothing ever called it. NotificationState.show() set visible=True unconditionally regardless of what the user had unchecked. register_notifications now takes an optional SettingsManager; show() early-returns (leaving the current banner, if any, untouched) when the message's type is disabled. Gating lives in the one method every call site already goes through, so every existing notifications.show(...) call across canvas.py, agents.py, chat_library.py, settings.py, plugins.py, autosave.py, and crash_recovery.py is covered without touching any of them. - View popover FONT section (finding #11): the family/size/color controls already round-tripped real setFontFamily/setFontSize/setFontColor intents into scene state - nothing consumed scene.fontFamily/fontSizePt/fontColor as CSS. SceneCanvas now writes them as --gl-node-font-family/-size/-color custom properties on the canvas wrapper (fontSizePt is points, not pixels - the existing 8-16 range and 9pt default only make sense that way, and pt is a real CSS unit). .scene-node/.scene-node-title/ .scene-node-body reference the tokens with their prior literal values as fallbacks, so every node's title and generic body text now responds to the FONT section live. Scoped to the shared base rules rather than each node kind's own content styling (markdown headings, code blocks) so typographic hierarchy and monospace code stay intact. Live-verified end to end in a running instance: changing the font size slider, family select, and color swatch each visibly restyled a real canvas node's title and body text; toggling a notification type off suppressed that type's banner while leaving others unaffected. Co-Authored-By: Claude Opus 5 --- backend/app.py | 2 +- backend/notifications.py | 22 +++++- backend/tests/test_notifications.py | 106 ++++++++++++++++++++++++++ web_ui/src/app/canvas/SceneCanvas.tsx | 24 +++++- web_ui/src/app/styles.css | 17 ++++- 5 files changed, 162 insertions(+), 9 deletions(-) create mode 100644 backend/tests/test_notifications.py diff --git a/backend/app.py b/backend/app.py index 6226c64..c3cbbc9 100644 --- a/backend/app.py +++ b/backend/app.py @@ -146,7 +146,7 @@ def ping(*args): # R2: notifications, moved ahead of canvas - R3.3's sendMessage intent # needs a real NotificationState to give an honest agent-dispatch notice. - notifications_state = register_notifications(bus) + notifications_state = register_notifications(bus, settings_manager) # R6.7: a no-op unless graphlink_desktop.py's own running.lock sentinel # found the prior run didn't reach a clean shutdown - see # backend/crash_recovery.py's module docstring for the full mechanism. diff --git a/backend/notifications.py b/backend/notifications.py index 501404a..0875ddc 100644 --- a/backend/notifications.py +++ b/backend/notifications.py @@ -9,11 +9,14 @@ from __future__ import annotations -from dataclasses import dataclass -from typing import Any, Literal +from dataclasses import dataclass, field +from typing import TYPE_CHECKING, Any, Literal from backend.events import SessionBus +if TYPE_CHECKING: + from graphlink_licensing import SettingsManager + MessageType = Literal["info", "success", "warning", "error"] @@ -22,8 +25,19 @@ class NotificationState: visible: bool = False message: str = "" msg_type: MessageType = "info" + # R8a (UI/UX issue list finding #10): Settings' own "Notification types" + # checkboxes wrote real preferences (SettingsManager.get_notification_ + # type_enabled already existed) that nothing ever read - show() set + # visible=True unconditionally no matter what the user had unchecked. + # Optional so the many call sites/tests that only ever had `bus` still + # work unchanged; None means "no preference to check", i.e. always show. + settings_manager: "SettingsManager | None" = field(default=None, repr=False, compare=False) def show(self, message: str, msg_type: MessageType = "info") -> None: + if self.settings_manager is not None and not self.settings_manager.get_notification_type_enabled( + msg_type + ): + return self.message = str(message) self.msg_type = msg_type self.visible = True @@ -35,8 +49,8 @@ def payload(self) -> dict[str, Any]: return {"visible": self.visible, "message": self.message, "msgType": self.msg_type} -def register_notifications(bus: SessionBus) -> NotificationState: - state = NotificationState() +def register_notifications(bus: SessionBus, settings_manager: "SettingsManager | None" = None) -> NotificationState: + state = NotificationState(settings_manager=settings_manager) bus.register_topic("notification", state.payload) async def dismiss(): diff --git a/backend/tests/test_notifications.py b/backend/tests/test_notifications.py new file mode 100644 index 0000000..07a335e --- /dev/null +++ b/backend/tests/test_notifications.py @@ -0,0 +1,106 @@ +"""NotificationState preference-gating tests (R8a UI/UX issue list finding #10). + +Settings' "Notification types" checkboxes wrote real preferences via +SettingsManager.set_notification_preferences - the getter existed and had +zero callers. NotificationState.show() set visible=True unconditionally, so +unchecking a type had no effect on whether its banner appeared. +""" + +import pytest +from graphlink_licensing import SettingsManager + +from backend.events import SessionBus +from backend.notifications import NotificationState, register_notifications + + +@pytest.fixture +def manager(tmp_path): + return SettingsManager(tmp_path / "session.dat") + + +def test_show_with_no_settings_manager_always_shows(): + # The many existing call sites/tests construct NotificationState/ + # register_notifications with no settings manager at all - that must + # keep behaving exactly as before (always show). + state = NotificationState() + state.show("hello", "warning") + assert state.visible is True + assert state.message == "hello" + assert state.msg_type == "warning" + + +def test_show_is_suppressed_when_the_type_is_disabled(manager): + manager.set_notification_preferences({"warning": False}) + state = NotificationState(settings_manager=manager) + + state.show("a warning", "warning") + + assert state.visible is False + assert state.message == "" + + +def test_show_still_works_for_a_type_left_enabled(manager): + manager.set_notification_preferences({"warning": False}) + state = NotificationState(settings_manager=manager) + + state.show("all good", "success") + + assert state.visible is True + assert state.message == "all good" + assert state.msg_type == "success" + + +def test_disabling_and_then_reenabling_a_type_restores_show(manager): + manager.set_notification_preferences({"error": False}) + state = NotificationState(settings_manager=manager) + state.show("boom", "error") + assert state.visible is False + + manager.set_notification_preferences({"error": True}) + state.show("boom again", "error") + assert state.visible is True + + +def test_a_suppressed_show_does_not_clobber_a_currently_visible_banner(manager): + # show() bails out before touching message/msg_type/visible when the + # type is disabled - a currently-showing banner of a DIFFERENT type must + # survive a suppressed call, not get silently wiped to blank. + manager.set_notification_preferences({"warning": False}) + state = NotificationState(settings_manager=manager) + state.show("still here", "info") + assert state.visible is True + + state.show("suppressed", "warning") + + assert state.visible is True + assert state.message == "still here" + assert state.msg_type == "info" + + +def test_dismiss_is_unaffected_by_preferences(manager): + manager.set_notification_preferences({"info": False}) + state = NotificationState(settings_manager=manager) + state.visible = True + + state.dismiss() + + assert state.visible is False + + +def test_register_notifications_wires_the_settings_manager_through(manager): + manager.set_notification_preferences({"success": False}) + bus = SessionBus("notifications-gating-test") + state = register_notifications(bus, manager) + + state.show("saved", "success") + + assert state.visible is False + + +def test_register_notifications_without_a_settings_manager_still_shows_everything(): + bus = SessionBus("notifications-no-manager-test") + state = register_notifications(bus) + + state.show("hello", "error") + + assert state.visible is True diff --git a/web_ui/src/app/canvas/SceneCanvas.tsx b/web_ui/src/app/canvas/SceneCanvas.tsx index f046b58..34612e0 100644 --- a/web_ui/src/app/canvas/SceneCanvas.tsx +++ b/web_ui/src/app/canvas/SceneCanvas.tsx @@ -1189,6 +1189,28 @@ function CanvasInner({ store }: { store: SceneStore }) { const [smartGuideLines, setSmartGuideLines] = useState([]); const visibleGuideLines = scene.smartGuides ? smartGuideLines : []; + // R8a (UI/UX issue list finding #11): the View popover's FONT section + // (family/size/color) already round-trips real intents into scene state - + // nothing ever consumed them. Written as CSS custom properties on the + // canvas wrapper (not per-node inline styles) so every current AND future + // node inherits them for free through .scene-node's own rules in + // styles.css, the same way .scene-canvas already carries grid state down + // via React Flow's own Background props. + const canvasWrapperRef = useRef(null); + useEffect(() => { + const el = canvasWrapperRef.current; + if (!el) return; + el.style.setProperty("--gl-node-font-family", scene.fontFamily); + // fontSizePt is POINTS (backend field: font_size_pt, default 9 -> the + // FONT_SIZE_MIN/MAX range of 8-16 only makes sense as points, not + // pixels: 16px node text would be barely a size change from the 12px/ + // 11px base, while 16pt is a real, visible jump). `pt` is a real CSS + // unit (1pt = 1/72in = 4/3px), not print-only, so this needs no + // conversion - just the right unit suffix. + el.style.setProperty("--gl-node-font-size", `${scene.fontSizePt}pt`); + el.style.setProperty("--gl-node-font-color", scene.fontColor); + }, [scene.fontFamily, scene.fontSizePt, scene.fontColor]); + // R8a: Open Document View - the read-only markdown modal a chat/ // conversation node's card menu opens (see toFlowNodes' own // onOpenDocumentView field on each of those two branches). The displayed @@ -1465,7 +1487,7 @@ function CanvasInner({ store }: { store: SceneStore }) { return ( <> -
+