Skip to content

Commit 859ef1f

Browse files
committed
Consistency polish from the audit (§2.4, §2.7, harness wording)
- HighlightContextMenu now uses shared clampMenuPosition with the same measure-then-show pattern as SlideContextMenu - added previewSlideSettings() alongside the project/highlight helpers and routed every raw ui.previewSlides.get / ui.previewProject read (SlideTimingSliders, effective-settings, CodeEditor) through the helper layer - SaveRaceHarness keeps the prefix mode (it characterizes the pre-fix bug by design); stale React-era comments in the harness, toast mock and typing suite now describe the pre-migration wiring instead
1 parent adf66e2 commit 859ef1f

8 files changed

Lines changed: 65 additions & 24 deletions

File tree

src/features/editor/CodeEditor.svelte

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
type Project,
1616
} from "$lib/types";
1717
import { ui } from "$lib/stores/ui-state.svelte";
18+
import { previewProjectSetting } from "@/features/settings/preview-settings";
1819
import HighlightContextMenu from "@/features/highlights/HighlightContextMenu.svelte";
1920
import { untrack } from "svelte";
2021
import { createCodeEditorState } from "./code-editor-state.svelte";
@@ -73,7 +74,7 @@
7374
7475
const rawEditorFontSize = $derived(project.settings.editorFontSize || 14);
7576
const editorFontSize = $derived(
76-
ui.previewProject.editorFontSize ?? rawEditorFontSize,
77+
previewProjectSetting("editorFontSize") ?? rawEditorFontSize,
7778
);
7879
const lineHeight = 1.55;
7980
const lineCount = $derived(Math.max(1, code.split("\n").length));

src/features/editor/SlideTimingSliders.svelte

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
<script lang="ts">
22
import { untrack } from "svelte";
33
import {
4-
ui,
54
setPreviewProjectSetting,
65
setPreviewSlideSetting,
76
} from "$lib/stores/ui-state.svelte";
7+
import {
8+
previewProjectSettings,
9+
previewSlideSettings,
10+
} from "@/features/settings/preview-settings";
811
import {
912
updateProjectSettingsMutation,
1013
updateSlideSettingsMutation,
@@ -19,8 +22,8 @@
1922
const settingsMutation = updateSlideSettingsMutation(projectId);
2023
const projectSettingsMutation = updateProjectSettingsMutation(projectId);
2124
22-
const previewProject = $derived(ui.previewProject);
23-
const previewSlide = $derived(ui.previewSlides.get(slide.id));
25+
const previewProject = $derived(previewProjectSettings());
26+
const previewSlide = $derived(previewSlideSettings(slide.id));
2427
const globalTransitionEnabled = $derived(
2528
project.settings.useGlobalTransition,
2629
);

src/features/highlights/HighlightContextMenu.svelte

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import { Highlighter as HighlightIcon, X } from "@lucide/svelte";
99
import { Z_INDEX } from "$lib/ui/Overlay.svelte";
1010
import { pop } from "$lib/ui/transitions/pop";
11+
import { clampMenuPosition } from "$lib/lib/menu-position";
1112
1213
let {
1314
visible,
@@ -17,23 +18,47 @@
1718
}: {
1819
/** Whether the menu is visible */
1920
visible: boolean;
20-
/** Position where the menu should appear */
21+
/** Pointer point (client coordinates) */
2122
position: { x: number; y: number };
2223
/** Callback when "Add Highlight" is clicked */
2324
onAddHighlight: () => void;
2425
/** Callback to close the menu */
2526
onClose: () => void;
2627
} = $props();
28+
29+
let menuEl = $state<HTMLDivElement | null>(null);
30+
let resolved = $state({ x: -9999, y: -9999 });
31+
let positioned = $state(false);
32+
33+
// Measure-then-show before paint (same as SlideContextMenu): keeps the
34+
// menu inside the viewport without a flash at the raw click point.
35+
$effect(() => {
36+
if (!visible || !menuEl) return;
37+
void position.x;
38+
void position.y;
39+
positioned = false;
40+
const rect = menuEl.getBoundingClientRect();
41+
resolved = clampMenuPosition({
42+
x: position.x,
43+
y: position.y,
44+
width: rect.width,
45+
height: rect.height,
46+
});
47+
positioned = true;
48+
});
2749
</script>
2850

2951
{#if visible}
3052
<div
53+
bind:this={menuEl}
3154
use:clickOutside={{ onOutside: onClose, delayMs: 50 }}
3255
use:escapeKey={{ onEscape: onClose, delayMs: 50 }}
3356
class="fixed min-w-[180px] rounded-lg border border-border/80 bg-card/95 py-1 shadow-xl backdrop-blur-md"
3457
role="menu"
3558
aria-label="Highlight actions"
36-
style="left: {position.x}px; top: {position.y}px; z-index: {Z_INDEX.contextMenu};"
59+
style="left: {resolved.x}px; top: {resolved.y}px; z-index: {Z_INDEX.contextMenu}; opacity: {positioned
60+
? 1
61+
: 0};"
3762
transition:pop={{}}
3863
>
3964
<button

src/features/settings/effective-settings.svelte.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,17 @@
1-
import { ui } from "$lib/stores/ui-state.svelte";
21
import type { Project, Slide } from "$lib/types";
2+
import {
3+
previewProjectSettings,
4+
previewSlideSettings,
5+
} from "./preview-settings";
36

47
export function createEffectiveSettings(
58
project: () => Project,
69
slide?: () => Slide | undefined,
710
) {
811
const settings = $derived(project().settings);
912
const target = $derived(slide?.() ?? project().slides[0]);
10-
const previewProject = $derived(ui.previewProject);
11-
const previewSlide = $derived(
12-
target ? ui.previewSlides.get(target.id) : undefined,
13-
);
13+
const previewProject = $derived(previewProjectSettings());
14+
const previewSlide = $derived(previewSlideSettings(target?.id));
1415

1516
const result = $derived.by(() => {
1617
const globalTransitionDuration =

src/features/settings/preview-settings.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@
55
* const fontSize = $derived(previewProjectSetting("fontSize") ?? project.settings.fontSize);
66
*/
77
import { ui } from "$lib/stores/ui-state.svelte";
8-
import type { PreviewProjectSettings } from "$lib/stores/types";
8+
import type {
9+
PreviewProjectSettings,
10+
PreviewSlideSettings,
11+
} from "$lib/stores/types";
912
import type { Highlight } from "$lib/types";
1013

1114
export function previewProjectSettings(): PreviewProjectSettings {
@@ -18,6 +21,12 @@ export function previewProjectSetting<K extends keyof PreviewProjectSettings>(
1821
return ui.previewProject[key];
1922
}
2023

24+
export function previewSlideSettings(
25+
slideId: string | undefined,
26+
): PreviewSlideSettings | undefined {
27+
return slideId ? ui.previewSlides.get(slideId) : undefined;
28+
}
29+
2130
export function previewHighlightSettings(
2231
highlightId: string | undefined,
2332
): Partial<Highlight> | undefined {

tests/codeeditor-typing.test.mts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,9 @@
1313
*
1414
* Svelte 5 port note: the mounted editor renders an UNCONTROLLED textarea
1515
* (value written once at mount; typing/save cycles never re-assign it from
16-
* code), which structurally removes the regression+teleport path the React
17-
* version had. These tests keep asserting that invariant end-to-end.
16+
* code), which structurally removes the regression+teleport path the
17+
* pre-migration controlled version had. These tests keep asserting that
18+
* invariant end-to-end.
1819
*/
1920
// MUST be the first import (installs document/window for the components).
2021
import "./helpers/jsdom-env.mts";

tests/harness/SaveRaceHarness.svelte

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,17 @@
11
<script lang="ts">
22
/**
33
* SaveRaceHarness — models the Svelte CodeEditor's value wiring for the
4-
* save-race suite (the React-era harness did the same for the controlled
5-
* React textarea). Mounted by tests/editor-save-race.test.mts.
4+
* save-race suite. Mounted by tests/editor-save-race.test.mts.
65
*
7-
* mode "queued": current wiring — UNCONTROLLED textarea (the Svelte editor
8-
* never writes `value` programmatically after mount), local
9-
* code shadow + the real queued updateSlideCodeMutation hook.
10-
* mode "prefix": characterization of the pre-fix bug — CONTROLLED-ish
11-
* textarea (framework re-assigns value on every computed
12-
* change, like the old React component did) + an unqueued
13-
* save path with the exact onSuccess the pre-fix hook had.
6+
* mode "queued": current wiring — UNCONTROLLED textarea (the editor never
7+
* writes `value` programmatically after mount), local code
8+
* shadow + the real queued updateSlideCodeMutation hook.
9+
* mode "prefix": deliberate characterization of the pre-fix bug — a
10+
* CONTROLLED-ish textarea (the harness re-assigns value on
11+
* every computed change, like the pre-migration controlled
12+
* editor did) + an unqueued save path with the exact
13+
* onSuccess the pre-fix hook had. Kept on purpose: the
14+
* suite asserts the bug stays dead across both wirings.
1415
*/
1516
import { createQuery } from "@tanstack/svelte-query";
1617
import {

tests/mocks/toast.mock.mts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/** Mock of src/lib/toast — the real one pulls in sonner/React components. */
1+
/** Mock of $lib/lib/toast — keeps the notify surface without the toast UI layer. */
22
export const toastCalls: Array<{ kind: string; msg: string }> = [];
33
export const notify = {
44
success: (msg: string) => {

0 commit comments

Comments
 (0)