R8a: gate notifications on type preference, wire up node font styling (findings #10, #11) - #174
Merged
Merged
Conversation
… (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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Two settings from the R8a audit persisted real state that nothing consumed:
SettingsManager.set_notification_preferences, andget_notification_type_enabledalready existed to read them back, butNotificationState.show()setvisible = Trueunconditionally. Unchecking a type had no effect on whether its banner appeared.setFontFamily/setFontSize/setFontColorintents into scene state, but nothing on the frontend ever consumedscene.fontFamily/fontSizePt/fontColoras CSS. Changing any of the three controls visibly changed nothing on the canvas.Change
backend/notifications.py:register_notificationsnow takes an optionalSettingsManager.NotificationState.show()early-returns (leaving any currently-visible banner untouched) when the message's type is disabled in preferences. The gate lives in the one method every existingnotifications.show(...)call site already goes through (canvas.py, agents.py, chat_library.py, settings.py, plugins.py, autosave.py, crash_recovery.py), so all of them are covered without individual changes.backend/app.pynow passes the session's real settings manager through.web_ui/src/app/canvas/SceneCanvas.tsx: writesscene.fontFamily/fontSizePt/fontColoronto the canvas wrapper as--gl-node-font-family/-size/-colorcustom properties.fontSizePtis points (backend fieldfont_size_pt, range 8-16, default 9) —ptis a real CSS unit, so no conversion needed beyond the correct suffix.web_ui/src/app/styles.css:.scene-node,.scene-node-title, and.scene-node-bodyreference the new tokens with their prior literal values as fallbacks. Scoped to the shared base rules (title bar, generic body text) rather than each node kind's own content styling, so markdown heading hierarchy and monospace code blocks keep their own intentional typography.Test plan
npx tsc --noEmitnpx vitest run(full suite)python -m pytest -q(full suite)npm run lintNew coverage:
backend/tests/test_notifications.py(gating on/off per type, a suppressedshow()doesn't clobber a currently-visible banner of a different type,dismiss()unaffected by preferences,register_notificationswiring, and the pre-existing no-settings-manager call sites still always show).Live-verified end to end against a running instance (real WS round-trip, not a mock): dragging the font-size slider, picking a font family, and picking a font color each immediately restyled a real canvas node's title and body text to the new values; the initial page load already reflected the backend's default font settings correctly.