Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion backend/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
22 changes: 18 additions & 4 deletions backend/notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]


Expand All @@ -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
Expand All @@ -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():
Expand Down
106 changes: 106 additions & 0 deletions backend/tests/test_notifications.py
Original file line number Diff line number Diff line change
@@ -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
24 changes: 23 additions & 1 deletion web_ui/src/app/canvas/SceneCanvas.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1189,6 +1189,28 @@ function CanvasInner({ store }: { store: SceneStore }) {
const [smartGuideLines, setSmartGuideLines] = useState<GuideLine[]>([]);
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<HTMLDivElement | null>(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
Expand Down Expand Up @@ -1465,7 +1487,7 @@ function CanvasInner({ store }: { store: SceneStore }) {

return (
<>
<div className="scene-canvas" onDoubleClick={onDoubleClick}>
<div className="scene-canvas" ref={canvasWrapperRef} onDoubleClick={onDoubleClick}>
<ReactFlow
nodes={nodes}
edges={edges}
Expand Down
17 changes: 14 additions & 3 deletions web_ui/src/app/styles.css
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,16 @@ body,
border-radius: 8px;
overflow: hidden;
font-size: 12px;
/* R8a (UI/UX issue list finding #11): the View popover's FONT section -
family/size/color, --gl-node-font-* set on .scene-canvas by
SceneCanvas.tsx - used to round-trip into scene state with nothing
consuming it. Applied here, the shared base every node kind renders
through, rather than per-kind content rules (markdown headers, code
blocks) so headings/monospace keep their own intentional typography;
this is the node's title bar and any plain body text that doesn't set
its own font-size. Falls back to the pre-existing literal when unset
(e.g. in tests that render a node outside SceneCanvas). */
font-family: var(--gl-node-font-family, inherit);
}

.scene-node.selected {
Expand All @@ -208,7 +218,8 @@ body,
.scene-node-title {
padding: 6px 10px;
font-weight: 600;
color: var(--gl-surface-text);
font-size: var(--gl-node-font-size, inherit);
color: var(--gl-node-font-color, var(--gl-surface-text));
background-color: var(--gl-surface-inset, var(--gl-surface-node-body));
border-bottom: 1px solid var(--gl-surface-border);
}
Expand All @@ -219,8 +230,8 @@ body,

.scene-node-body {
padding: 8px 10px;
color: var(--gl-surface-text-muted);
font-size: 11px;
color: var(--gl-node-font-color, var(--gl-surface-text-muted));
font-size: var(--gl-node-font-size, 11px);
}

.scene-canvas .react-flow__edge-path {
Expand Down
Loading