-
Notifications
You must be signed in to change notification settings - Fork 6
feat: Guided Tours End Screen #2339
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 05-27-fix_guided_tours_ephemeral_state
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| import { useEffect, useState } from "react"; | ||
|
|
||
| import { PipelineNameDialog } from "@/components/shared/Dialogs"; | ||
| import useToastNotification from "@/hooks/useToastNotification"; | ||
| import { serializeComponentSpecToText } from "@/models/componentSpec"; | ||
| import { useTourMode } from "@/providers/TourProvider/TourModeContext"; | ||
| import { registerSaveExploreHandler } from "@/providers/TourProvider/TourPopover"; | ||
| import { usePipelineActions } from "@/routes/v2/pages/Editor/store/actions/usePipelineActions"; | ||
| import { useSharedStores } from "@/routes/v2/shared/store/SharedStoreContext"; | ||
|
|
||
| export function TourSaveExploreDialog() { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This PR adds a new React component under src/providers, but react-compiler.config.js only enables src/routes and selected provider subdirectories; src/providers/TourProvider is still outside compiler coverage. The project review standard treats missing React Compiler registration for new components/hooks as a High-severity adoption gap. Suggestion: Add this file explicitly to REACT_COMPILER_ENABLED_DIRS, or enable the appropriate TourProvider subdirectory after confirming it is compiler-compatible. Rule: react-patterns / tangle-ui-review — new components and hooks must be covered by react-compiler.config.js; new files under non-enabled directories are not covered automatically. |
||
| const tourMode = useTourMode(); | ||
| const { navigation } = useSharedStores(); | ||
| const { renamePipeline } = usePipelineActions(); | ||
| const notify = useToastNotification(); | ||
| const [open, setOpen] = useState(false); | ||
|
|
||
| useEffect(() => { | ||
| if (!tourMode) return; | ||
| return registerSaveExploreHandler(() => setOpen(true)); | ||
| }, [tourMode]); | ||
|
camielvs marked this conversation as resolved.
|
||
|
|
||
| if (!tourMode) return null; | ||
|
|
||
| const onSubmit = async (name: string) => { | ||
| const rootSpec = navigation.rootSpec; | ||
| if (!rootSpec) { | ||
| notify( | ||
| "Pipeline isn't ready to save yet — try again in a moment.", | ||
| "error", | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| renamePipeline(rootSpec, name); | ||
| const yamlContent = serializeComponentSpecToText(rootSpec); | ||
| await tourMode.promoteToPipeline(name, yamlContent); | ||
| } catch (error) { | ||
| const message = | ||
| error instanceof Error ? error.message : "Failed to save pipeline"; | ||
| notify(message, "error"); | ||
| } | ||
| }; | ||
|
|
||
| return ( | ||
| <PipelineNameDialog | ||
| open={open} | ||
| onOpenChange={setOpen} | ||
| title="Save pipeline" | ||
| description="Convert this demo pipeline into a regular pipeline you can keep editing." | ||
| initialName={tourMode.tour.displayName ?? tourMode.tour.id} | ||
| onSubmit={onSubmit} | ||
| submitButtonText="Save" | ||
| /> | ||
| ); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The save/explore action is registered by mutating a module-level variable and then read during TourCompletionActions render. Because registering the handler is not React state or context, changes do not cause a re-render; if the completion actions render before the effect in TourSaveExploreDialog registers, the Save demo pipeline button is omitted until some unrelated render happens. This also makes the feature depend on mount/effect ordering instead of explicit React data flow.
Suggestion: Move the save-dialog opener into React-owned state/context, e.g. include an openSavePipelineDialog callback or capability flag in TourModeValue/TourProvider, and render TourCompletionActions from that context instead of reading a mutable module singleton.
Rule: docs/react-best-practices.md / react-patterns — context/provider values should model shared state explicitly; avoid unstable/non-reactive values that bypass React data flow.