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 ( <> -
+