Skip to content

fix: change play once from being per timeline, to per trigger #11

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.

Sign up for GitHub

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

Open
Arukuen wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

fix: change play once from being per timeline, to per trigger #11

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions src/frontend/scripts/class-timeline.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ export class Timeline {
this._resetAtStart = !! timelineData.reset
this._onceOnly = !! timelineData.onceOnly
this._actionStarts = {}

// Used to track if the timeline is played per each trigger.
this.timelineData._playedTriggers = new WeakSet()
}

this._targets = []
Expand All @@ -40,19 +43,19 @@ export class Timeline {

createInstance( options ) {
// If triggered only once, then we don't create anymore animations.
// We reference timelineData._played here so that it persists across other timeline instances.
const currentTrigger = this.interaction.getCurrentTrigger()
if ( this.getRunner().isFrontend ) {
if ( this.timelineData.onceOnly && this.timelineData._played ) {
if ( this.timelineData.onceOnly && this.timelineData._playedTriggers.has( currentTrigger ) ) {
return null
}
}

this.timelineData._playedTriggers.add( currentTrigger )
Comment on lines +46 to +53
Copy link

@coderabbitai coderabbitai bot Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== Finding getCurrentTrigger implementation ==="
find . -name "*.js" -type f | head -20
echo ""
echo "=== Searching for getCurrentTrigger usage and definition ==="
rg -n "getCurrentTrigger" --type=js -B2 -A5

Repository: gambitph/Interactions

Length of output: 10296

🏁 Script executed:

#!/bin/bash
echo "=== Reading class-interaction.js around getCurrentTrigger ==="
sed -n '100,130p' src/frontend/scripts/class-interaction.js

echo ""
echo "=== Reading class-timeline.js around the WeakSet usage ==="
sed -n '40,60p' src/frontend/scripts/class-timeline.js

echo ""
echo "=== Checking how triggerElements is set ==="
rg -n "triggerElements" src/frontend/scripts/class-interaction.js -B2 -A2

echo ""
echo "=== Checking initialization of timelineData._playedTriggers ==="
rg -n "_playedTriggers" src/frontend/scripts/class-timeline.js -B2 -A2

Repository: gambitph/Interactions

Length of output: 4174

🏁 Script executed:

#!/bin/bash
echo "=== Searching for where _currentTrigger is set ==="
rg -n "_currentTrigger" src/frontend/scripts/class-interaction.js -B2 -A2

echo ""
echo "=== Checking cloneWithTrigger implementation ==="
rg -n "cloneWithTrigger" src/frontend/scripts/class-interaction.js -B2 -A8

echo ""
echo "=== Checking if triggerElements can be undefined or empty ==="
sed -n '70,90p' src/frontend/scripts/class-interaction.js

Repository: gambitph/Interactions

Length of output: 2272

🏁 Script executed:

#!/bin/bash
echo "=== Check if getTargets can return empty array ==="
rg -n "getTargets" src/frontend/scripts/class-interaction.js -A10 | head -40

echo ""
echo "=== Verify what happens in non-cloned instance scenario ==="
rg -n "createInstance\|new Timeline" src/frontend/scripts/class-interaction.js -B2 -A2

echo ""
echo "=== Check initialization context for timelines ==="
sed -n '15,35p' src/frontend/scripts/class-timeline.js

Repository: gambitph/Interactions

Length of output: 2834

WeakSet operations will throw TypeError if getCurrentTrigger() returns undefined.

WeakSet only accepts objects as values; passing primitives or non-objects like undefined will result in a TypeError. The getCurrentTrigger() method (src/frontend/scripts/class-interaction.js:114) can return undefined when _currentTrigger is undefined and triggerElements is empty or falsy.

In class-timeline.js:48, the WeakSet .has() check only executes in frontend mode with onceOnly enabled, but line 53's .add() call executes unconditionally. If currentTrigger is undefined, line 53 will throw a TypeError regardless of the frontend mode or timeline configuration, crashing the interaction.

Additionally, confirm that non-page interactions without defined triggers should always have at least a fallback trigger element to avoid this edge case entirely.


// We have to empty the promises here because we are creating a new timeline.
// This is to prevent the promises from the previous timeline from affecting the new one.
this._funcPromises = {}

this.timelineData._played = true

const propsToPass = {}
if ( this.type === 'percentage' ) {
propsToPass.duration = 100 // 100% so it's easier to compute the actions later.
Expand Down