Repository navigation
Conversation
|
@TupiC is attempting to deploy a commit to the walterlow's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
 |
 [Medium impact] The changes reach text-clip settings, keyframe tools, and shared text output paths. Fix the counter resets caused by keyframe insertion and splitting before merging. FindingsFix with agent prompt### Issue 1
src/features/keyframes/utils/animated-text-item.ts:49
The keyframe editor uses `textCounter.from` for an unkeyframed `counterValue`, not the number currently shown. For a 101-frame counter from 0 to 100, the row shows 0 at frame 50 while the preview shows 50. Clicking the row’s diamond inserts 0 and makes the counter display 0 throughout the clip. Use `resolveTextCounterValue` for both the row value and keyframe insertion.
### Issue 2
src/shared/typography/text-counter.ts:31-33
Splitting an unkeyframed counter copies the same `textCounter` into both pieces but gives each a shorter duration. This calculation then runs the full range separately in each piece. A 0-to-100 counter split halfway reaches 100 before the cut and jumps back to 0 after it. Preserve the original count and timing when splitting, for example by storing the original ramp window or baking it before the split.
### Issue 3
src/features/editor/components/properties-sidebar/clip-panel/text-counter-controls.tsx:124-127
Both endpoint inputs use `step={1}`, so `NumberInput` displays fractional endpoints as whole numbers even though it saves the fraction. Entering 12.5 shows 13 while the counter still uses 12.5. Focusing and leaving that field again can save 13. Use formatting that preserves the supported decimal places for both `from` and `to`.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.SummaryAdds animated number counters to text clips.
Fix keyframe-editor insertion and split behavior before merging. Fractional endpoint fields also need accurate formatting. Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Text clip with counter enabled] --> B{Counter Value keyframes?}
B -->|Yes| C[Read value from keyframes]
B -->|No| D[Calculate count from local frame and clip duration]
C --> E[Format number and add prefix or suffix]
D --> E
E --> F[Replace resolved text and first span]
F --> G[Preview and export]
Reviews (1) · Last reviewed commit: "feat(text): add animated number counter ..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| textShadowOffsetY: (item) => item.textShadow?.offsetY ?? 0, | ||
| textShadowBlur: (item) => item.textShadow?.blur ?? 0, | ||
| strokeWidth: (item) => item.stroke?.width ?? 0, | ||
| counterValue: (item) => item.textCounter?.from ?? 0, |
There was a problem hiding this comment.
The keyframe editor uses textCounter.from for an unkeyframed counterValue, not the number currently shown. For a 101-frame counter from 0 to 100, the row shows 0 at frame 50 while the preview shows 50. Clicking the row’s diamond inserts 0 and makes the counter display 0 throughout the clip. Use resolveTextCounterValue for both the row value and keyframe insertion.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/features/keyframes/utils/animated-text-item.ts
Line: 49
Comment:
**First keyframe resets the number**
The keyframe editor uses `textCounter.from` for an unkeyframed `counterValue`, not the number currently shown. For a 101-frame counter from 0 to 100, the row shows 0 at frame 50 while the preview shows 50. Clicking the row’s diamond inserts 0 and makes the counter display 0 throughout the clip. Use `resolveTextCounterValue` for both the row value and keyframe insertion.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| const lastFrame = Math.max(1, durationInFrames - 1) | ||
| const progress = applyEasing(relativeFrame / lastFrame, counter.easing) | ||
| return counter.from + (counter.to - counter.from) * progress |
There was a problem hiding this comment.
Splitting an unkeyframed counter copies the same textCounter into both pieces but gives each a shorter duration. This calculation then runs the full range separately in each piece. A 0-to-100 counter split halfway reaches 100 before the cut and jumps back to 0 after it. Preserve the original count and timing when splitting, for example by storing the original ramp window or baking it before the split.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/shared/typography/text-counter.ts
Line: 31-33
Comment:
**Cutting restarts the count**
Splitting an unkeyframed counter copies the same `textCounter` into both pieces but gives each a shorter duration. This calculation then runs the full range separately in each piece. A 0-to-100 counter split halfway reaches 100 before the cut and jumps back to 0 after it. Preserve the original count and timing when splitting, for example by storing the original ramp window or baking it before the split.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| <NumberInput | ||
| value={settings.from} | ||
| onChange={(from) => patchCounter({ from })} | ||
| step={1} |
There was a problem hiding this comment.
Both endpoint inputs use step={1}, so NumberInput displays fractional endpoints as whole numbers even though it saves the fraction. Entering 12.5 shows 13 while the counter still uses 12.5. Focusing and leaving that field again can save 13. Use formatting that preserves the supported decimal places for both from and to.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/features/editor/components/properties-sidebar/clip-panel/text-counter-controls.tsx
Line: 124-127
Comment:
**Fractional settings look rounded**
Both endpoint inputs use `step={1}`, so `NumberInput` displays fractional endpoints as whole numbers even though it saves the fraction. Entering 12.5 shows 13 while the counter still uses 12.5. Focusing and leaving that field again can save 13. Use formatting that preserves the supported decimal places for both `from` and `to`.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
to show a value changing over time, such as a progress text counting from 1% to 100% (see video below) you had to place a separate text clip for every value and edit each one by hand, which was slow and nearly impossible to retime.
this PR adds a Counter setting to text clips: you create one text clip, switch on Counter, and set From/To, decimals, a prefix or suffix, a thousands separator and easing. The number then counts across the clip by default, and you can add keyframes on the new Counter Value property for custom timing (holds, count-downs). The text is generated each frame in the same function every text render uses, so preview, export and the headless renderer show the same value. The settings are saved with the project, and turning the counter off brings the original text back.
counter-showcase.mp4