feat: Guided Tours Framework (First Pipeline)#2347
Conversation
🎩 PreviewA preview build has been created at: |
f262b2f to
42a8c86
Compare
64f6366 to
da1c54e
Compare
ebfb573 to
3fa9ad5
Compare
da1c54e to
019a78f
Compare
019a78f to
fe394fe
Compare
3fa9ad5 to
fd171b1
Compare
3f64df6 to
9485123
Compare
eb23e01 to
257e52e
Compare
9485123 to
9c75e0b
Compare
257e52e to
7602419
Compare
9c75e0b to
f96ded5
Compare
13c6185 to
553d9ca
Compare
4c860ee to
f2fe192
Compare
a9aed41 to
8ae6b11
Compare
f2fe192 to
4358282
Compare
8ae6b11 to
bc842cc
Compare
4358282 to
11031f0
Compare
bc842cc to
598d4be
Compare
25b9c6f to
f879945
Compare
598d4be to
3e48035
Compare
3e48035 to
0dd9520
Compare
|
|
||
| update(); | ||
| const observer = new MutationObserver(update); | ||
| observer.observe(document.body, { childList: true, subtree: true }); |
There was a problem hiding this comment.
🤖 This is an AI-generated code review comment.
const observer = new MutationObserver(update);
observer.observe(document.body, { childList: true, subtree: true });update() runs document.querySelectorAll(sel) for every ring selector on every childList mutation anywhere in the document. In the v2 editor, ReactFlow adds/removes node DOM as the user pans/zooms/edits — i.e. during exactly the interactions a ring step accompanies — so this re-queries the whole tree in bursts.
Suggestion: coalesce into a single requestAnimationFrame (the same rAF-guard pattern followWindowPosition already uses in this file) so a burst of mutations triggers at most one update() per frame, and optionally narrow the observed root from document.body to the editor container.
| return spec.tasks.some((task) => | ||
| task.arguments.some( | ||
| (arg) => | ||
| arg.name === targetArgumentName && |
There was a problem hiding this comment.
🤖 This is an AI-generated code review comment.
return spec.tasks.some((task) =>
task.arguments.some(
(arg) => arg.name === targetArgumentName && ... ,
),
);This scans all tasks, so if two tasks expose an argument with the same name, setting it on the wrong task still satisfies the step. This is the same class of issue raised on #2299 for select-task — which you fixed here by checking data-task-name; set-argument still lacks a task qualifier, and TourStep already carries targetTaskName.
If set-argument steps target a specific task, restrict the .some to the task whose name matches step.targetTaskName (when present). If matching any task is intentional, a one-line comment would keep it from reading as an oversight.
| if (!libraryDragAllow) return undefined; | ||
| const allow = libraryDragAllow.toLowerCase(); | ||
| const handleDragStart = (event: DragEvent) => { | ||
| const target = event.target as Element | null; |
There was a problem hiding this comment.
🤖 This is an AI-generated code review comment.
Each new event handler casts event.target as Element | null and then calls .closest(...) / .matches(...) on it (here and at lines 372 and 442). Per typescript-standards, prefer a narrowing guard over as:
if (event.target instanceof Element && event.target.closest(selector)) { ... }Since the pattern now repeats across four+ handlers in this file, a small shared helper — e.g. closestFromEvent(event, selector) returning Element | null — would remove the cast everywhere and read cleaner. Low priority.
0dd9520 to
7a95d8f
Compare

Description
The interaction primitives, popover positioning, and CSS utilities needed to power the second guided tour (#2312 — Build Your First Pipeline). No visible behavior on its own; all testing happens in #2312.
Builds directly on #2299 (the first three interactions). This PR introduces the remaining interactions a build-from-scratch tour needs, plus a handful of cross-cutting tour-step features.
What's new
New interaction types (
EditorTourBridge.tsx)Each is declared on a tour step via
interaction: "<name>". Following the model established in #2299, a detected interaction marks the step complete in the shared tour-progress state — it does not advance the tour itself; the gated "Next" / auto-advance in #2340 handle progression.expand-folder— completes when the named folder in the component library is expanded (matches[data-folder-name=...] [aria-expanded="true"]).library-search— completes when the search box value contains a target substring; debounced so the user finishes typing first.add-task— completes when a task whose name matches a target substring is added to the canvas. Baseline counted at step start so pre-existing tasks don't false-trigger.add-input/add-output— count-based: complete whenspec.inputs.length/spec.outputs.lengthincreases.connect-edge— completes when a specific edge is drawn, matched by(sourceTaskName, sourcePortName, targetTaskName, targetPortName). The target task/port pair is optional.set-argument— completes when a specific argument on any task is set to a non-empty string.Existing
select-task(from #2299) gets atargetTaskNamenarrowing — only the named task counts, not any task.Contextual checklist labels
Adds the checklist copy for the interactions this PR introduces (using the contextual bold-target support from #2340):
add-task→ "Add {targetTaskName} to the canvas"expand-folder→ "Open the {targetFolderName} folder"library-search→ "Search the library for {targetSearchTerm}"set-argument→ "Set the {targetArgumentName} value"add-input/add-output/connect-edge→ generic phrasingsEach falls back to a generic label when its target field isn't set.
Cross-cutting step features
ringSelectors— paint per-element rings via the new.tour-ringCSS class (reactour mergeshighlightedSelectorsinto one bounding box; per-handle rings need a separate mechanism).requiresTaskSelected— auto-rewind to a previousselect-taskstep if the required task is no longer selected.ensureWindowRestored— auto-restore a minimized/hidden dock window at step start.resetLibrarySearch— clear the component-library search box before the step's selector is queried.libraryDragAllow— block drags of library items that don't match the step's target.Popover positioning
computeDefaultPopoverPositiongets a symmetric left-anchored tall-strip branch (mirrors the right-anchored branch from feat: Guided Tours Foundation #2348) for steps that highlight the component library.POPOVER_STYLES.clickAreaset topointer-events: noneso the spotlight click-area doesn't intercept drags onto the canvas.Misc
TourStepregistry types extended with the new interaction fields (targetFolderName,targetSearchTerm,targetTaskName,targetComponentName,targetEdge,targetArgumentName, plus the cross-cutting flags above)..tour-ringCSS utility ineditor.css, with a React Flow handle override (position: absolute).defaultPipelineYamlWithNamenowJSON.stringifys the name (pipelines named with YAML-special characters were breaking the YAML).Related Issue and Pull requests
Progresses https://github.com/Shopify/oasis-frontend/issues/583
Builds on #2299. Consumed by #2312 (First Pipeline tour content).
Type of Change
Checklist
Test Instructions
No visible behavior on its own — every new utility is unused until #2312 lands. Regression-only checks in isolation:
select-taskwithouttargetTaskNamestill completes on click of any task (the narrowing is opt-in).Additional Comments
Originally bundled into #2312 alongside the tour content; split out so reviewers can read the mechanics and the tour JSON separately.