Campaign #2143
Campaign #2143
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@Parvinmh is attempting to deploy a commit to the Pean Team on Vercel. A member of the Team first needs to authorize it. |
| })); | ||
|
|
||
| // Create and configure tour | ||
| const tour = new Tour(); |
There was a problem hiding this comment.
executeCampaign is currently tied to Tour only but what if we want to launch a Hint, or a banner in the future? This function shouldn't really care about what needs to be executed, instead, there needs to be a factory that takes the type of experience that needs to be launched (e.g. tour, hint, etc) and then launches it.
| this.setupCampaignCallbacks(tour, campaign); | ||
|
|
||
| // Store active tour | ||
| this.activeTours.set(campaignId, tour); |
There was a problem hiding this comment.
Similar to the previous comment, we shouldn't maintain activeTours specifically, but rather an active experience type -- Also, how this activeTours is being used? what's the purpose?
| } | ||
| break; | ||
|
|
||
| case "element_visible": |
There was a problem hiding this comment.
I really like these triggers
| /** | ||
| * Setup scroll to element trigger | ||
| */ | ||
| private setupScrollToElementTrigger( |
There was a problem hiding this comment.
These triggers should be moved to separate files to make the testing and maintainablitiy easier
| * Resume idle timers when page becomes visible | ||
| */ | ||
| private resumeIdleTimers(): void { | ||
| // Implementation for resuming idle timers |
There was a problem hiding this comment.
These need to be implemented I assume?
| updatedAt?: string; | ||
| author?: string; | ||
| tags?: string[]; | ||
| priority?: number; // Priority for when multiple campaigns match (higher = higher priority) |
There was a problem hiding this comment.
Is this currently implemented?
| tourOptions?: Partial<TourOptions>; | ||
|
|
||
| // Hint configuration - uses existing HintOptions structure | ||
| hintOptions?: Partial<HintOptions>; |
There was a problem hiding this comment.
I don't think we need to have tourOptions or hintOptions, but rather a generic "options" that defines the options of that particular experience based on what "mode" is passed in. Make sure we don't union the types though, there needs to be a BaseCampaign type and Campaign type the conditionally changes the "options" type based the "mode".
| global?: { | ||
| targeting?: CampaignTargeting; | ||
| tourDefaults?: Partial<TourOptions>; | ||
| hintDefaults?: Partial<HintOptions>; |
There was a problem hiding this comment.
Why do we need defaults here? And again, it shouldn't be tied to Tour or Hint.
| if (this.isInitialized) return; | ||
|
|
||
| // Track session count | ||
| const sessionCountKey = "introjs-campaign-session-count"; |
There was a problem hiding this comment.
This needs to be in configs.
| updateStatus('Initializing campaigns from JSON file...'); | ||
|
|
||
| // Initialize campaigns from JSON file (correct path for this directory) | ||
| await introJs.initializeCampaigns("./campaigns/first-visit-tour.json"); |
There was a problem hiding this comment.
How this would work in a browser environment? How do we do file I/O when running in browser context?
2 participants
No description provided.