refactor: extract sequence logic and simplify TextMotion conditionals

[shubug1015]

Description

This PR refactors focusing on improving predictability and reducing implicit behavior in animation sequencing and rendering logic.

1. Sequence management improvements

   • Extracted sequence-related logic into dedicated helper functions:
     • calculateSequenceIndex for deterministic index calculation
     • isLastNode for explicit end-of-sequence checks
   • Converted wrapWithAnimatedSpan into a pure function by removing side effects and implicit state dependencies.
   • This makes animation ordering easier to reason about and safer to test.

2. Conditional logic simplification

   • Simplified conditional rendering logic in TextMotion by flattening nested conditions and making rendering paths explicit.
   • Reduced hidden branching and improved readability without changing runtime behavior.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Refactor

Checklist

• My code follows the project style guidelines
• I performed a self-review of my own code
• I added tests that prove my fix is effective
• I added necessary documentation

Summary by CodeRabbit

• Bug Fixes

  • Non-animated text now renders without animation styling while preserving accessibility labels.
  • Improved sequence propagation to ensure consistent animation start/end behavior.

• New Features

  • Added utilities to compute animation ordering and detect final items for sequencing.

• Tests

  • Added unit tests for sequencing utilities and updated node-counting tests.

• Documentation

  • Clarified comments for type-guard functions.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

TextMotion now branches rendering based on shouldAnimate: non-animating renders children directly; animating renders wrapped animated nodes. The animation sequencing was refactored: useAnimatedChildren computes per-item sequence indices and propagates a nextSequenceIndex through a private wrapWithAnimatedSpan, driven by new sequenceHelpers utilities.

Changes

Cohort / File(s)	Summary
TextMotion render changes src/components/TextMotion/TextMotion.tsx	Conditional rendering split: non-animating path uses class text-motion-inanimate and renders raw children; animating path uses text-motion and renders animatedChildren. ARIA label and animation event handlers preserved.
Animation sequencing hook src/hooks/useAnimatedChildren/useAnimatedChildren.tsx	useAnimatedChildren refactored to compute sequence indices per-item. wrapWithAnimatedSpan made private and now returns WrapResult { nodes, nextSequenceIndex }; sequenceIndexRef removed; recursive calls propagate nextSequenceIndex. Imports calculateSequenceIndex and isLastNode.
Sequence helpers (logic + barrel) src/utils/sequenceHelpers/sequenceHelpers.ts, src/utils/sequenceHelpers/index.ts	New pure utilities: calculateSequenceIndex(currentIndex, totalNodes, animationOrder) and isLastNode(sequenceIndex, totalNodes). Barrel file re-exports module.
Sequence helpers tests src/utils/sequenceHelpers/sequenceHelpers.spec.ts	Unit tests added covering both animation orders and edge cases for calculateSequenceIndex and isLastNode.
Count nodes behavior & tests src/utils/countNodes/countNodes.ts, src/utils/countNodes/countNodes.spec.tsx	Counting logic changed to count only text (animated) nodes; corresponding test expectations reduced to match new behavior.
Type guard docs src/utils/typeGuards/typeGuards.ts	JSDoc comments clarified for three exported type guards; no runtime/signature changes.

Sequence Diagram(s)

sequenceDiagram
  participant TextMotion as TextMotion (component)
  participant Hook as useAnimatedChildren (hook)
  participant Helpers as sequenceHelpers (utils)

  TextMotion->>Hook: call useAnimatedChildren(children, options)
  Hook->>Hook: split nodes (splitNodeAndExtractText)
  Hook->>Helpers: calculateSequenceIndex(currentIndex, totalNodes, order)
  Helpers-->>Hook: sequenceIndex
  Hook->>Hook: wrap node with animated span (isLastNode -> mark last)
  Hook-->>TextMotion: return animatedChildren (nodes)
  TextMotion->>DOM: render animatedChildren or children based on shouldAnimate

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• refactor: extract constants and centralize type guards for better type safety #73: Overlapping edits to TextMotion.tsx and useAnimatedChildren with related ARIA/rendering and sequencing changes.
• refactor: extract reusable utilities and simplify logic #75: Modifies useAnimatedChildren and integrates count/sequence logic; closely related refactor patterns.

Suggested reviewers

• kjyong702

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main refactoring effort: extracting sequence logic into dedicated helpers and simplifying conditional rendering in TextMotion.
Description check	✅ Passed	The description covers all required template sections with concrete details: describes the changes made, marks Refactor as type of change, and confirms all checklist items.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In @src/hooks/useAnimatedChildren/useAnimatedChildren.tsx:
- Around line 72-94: The sequenceIndex is being incremented for every node due
to sequenceIndex++ at the top of the map, which consumes indexes for non-leaf
nodes and breaks isLastNode detection; fix by only consuming/incrementing
sequenceIndex when you actually create an AnimatedSpan for a text node: for text
nodes set const currentIndex = sequenceIndex; then increment sequenceIndex++
after creating the AnimatedSpan (or immediately after using currentIndex to
compute calculatedSequenceIndex/isLast and pass it to
AnimatedSpan/onAnimationEnd); for element nodes do not increment beforehand—call
wrapWithAnimatedSpan with the current sequenceIndex and then assign
sequenceIndex = nextSequenceIndex returned from wrapWithAnimatedSpan; keep uses
of calculateSequenceIndex, isLastNode, isTextNode, isElementWithChildren,
wrapWithAnimatedSpan, and AnimatedSpan unchanged otherwise.

🧹 Nitpick comments (2)

src/utils/sequenceHelpers/sequenceHelpers.spec.ts (1)

3-15: Consider adding edge-case tests for boundary conditions.

The current tests cover the happy path well. Consider adding tests for:

• totalNodes = 1 (single element)
• totalNodes = 0 (empty sequence) — what should the behavior be?

These edge cases would help document expected behavior and guard against regressions.

src/components/TextMotion/TextMotion.tsx (1)

97-103: Unnecessary computation when shouldAnimate is false.

When shouldAnimate is false, the early return at line 111 renders {children} directly, so animatedChildren is never used. However, useAnimatedChildren is still invoked with [children], performing unnecessary work.

Consider moving the hook call inside the animated branch or conditionally skipping the computation:

♻️ Suggested approach

- const animatedChildren = useAnimatedChildren({
-   splittedNode: shouldAnimate ? splittedNode : [children],
-   initialDelay,
-   animationOrder,
-   resolvedMotion,
-   onAnimationEnd,
- });
+ const animatedChildren = useAnimatedChildren({
+   splittedNode,
+   initialDelay,
+   animationOrder,
+   resolvedMotion,
+   onAnimationEnd,
+   enabled: shouldAnimate,
+ });

Alternatively, if hook rules prevent conditional calls, consider having useAnimatedChildren accept an enabled flag to skip internal processing when false.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 673e9c6 and b8692c4.

📒 Files selected for processing (6)

• src/components/TextMotion/TextMotion.tsx
• src/hooks/useAnimatedChildren/useAnimatedChildren.tsx
• src/utils/sequenceHelpers/index.ts
• src/utils/sequenceHelpers/sequenceHelpers.spec.ts
• src/utils/sequenceHelpers/sequenceHelpers.ts
• src/utils/typeGuards/typeGuards.ts

🧰 Additional context used

📓 Path-based instructions (1)

**/*.{ts,tsx}

⚙️ CodeRabbit configuration file

**/*.{ts,tsx}: You are a senior frontend reviewer.
Review this code strictly based on the principles of "easy-to-change code"
from Frontend Fundamentals (Readability, Predictability, Cohesion, Low Coupling).

================================

1. Readability
   ================================

• Check if the code can be read top-to-bottom like a document.
• Flag places where the reader must jump around to understand logic.
• Suggest extracting logic when multiple contexts are mixed in one function or component.
• Identify magic numbers or strings and suggest meaningful named constants.
• Point out unclear or abbreviated variable/function names.
• Prefer simple control flow over nested or overly clever logic.

================================
2. Predictability

• Verify that function and variable names clearly imply their behavior.
• Ensure functions with similar responsibilities return consistent data shapes.
• Flag hidden side effects (global state mutation, implicit behavior).
• Warn when the same name is used with different meanings across the codebase.
• Suggest making implicit assumptions explicit.

================================
3. Cohesion

• Check whether code that changes together is located together.
• Flag cases where related logic is scattered across multiple files unnecessarily.
• Suggest grouping constants, helpers, and hooks near the domain they belong to.
• Identify components or functions that contain unrelated responsibilities.
• Prefer feature/domain-based structure over technical grouping when applicable.

================================
4. Coupling

• Identify code where a small change would cause wide ripple effects.
• Warn when abstractions increase coupling instead of reducing it.
• Prefer duplication over premature abstraction if it lowers change impact.
• Flag excessive prop drilling and suggest alternative patterns if appropriate.
• Ensure compo...

Files:

• src/utils/typeGuards/typeGuards.ts
• src/utils/sequenceHelpers/sequenceHelpers.spec.ts
• src/utils/sequenceHelpers/sequenceHelpers.ts
• src/utils/sequenceHelpers/index.ts
• src/components/TextMotion/TextMotion.tsx
• src/hooks/useAnimatedChildren/useAnimatedChildren.tsx

🧬 Code graph analysis (5)

src/utils/typeGuards/typeGuards.ts (1)

src/utils/typeGuards/typeGuards.spec.ts (6)

• describe (5-110)
• it (70-109)
• it (6-36)
• it (38-68)
• element (76-79)
• element (28-31)

src/utils/sequenceHelpers/sequenceHelpers.spec.ts (1)

src/utils/sequenceHelpers/sequenceHelpers.ts (2)

• calculateSequenceIndex (13-19)
• isLastNode (30-32)

src/utils/sequenceHelpers/sequenceHelpers.ts (2)

src/types/common.ts (1)

• AnimationOrder (35-35)

src/utils/generateAnimation/generateAnimation.ts (1)

• accumulator (27-54)

src/components/TextMotion/TextMotion.tsx (4)

src/utils/splitNodeAndExtractText/splitNodeAndExtractText.ts (1)

• splitNodeAndExtractText (18-55)

src/hooks/useResolvedMotion/useResolvedMotion.ts (1)

• useResolvedMotion (25-39)

src/hooks/useAnimatedChildren/useAnimatedChildren.tsx (1)

• useAnimatedChildren (36-59)

src/constants/defaults.ts (1)

• DEFAULT_ARIA_LABEL (9-9)

src/hooks/useAnimatedChildren/useAnimatedChildren.tsx (4)

src/types/common.ts (2)

• AnimationOrder (35-35)
• Motion (51-60)

src/utils/sequenceHelpers/sequenceHelpers.ts (2)

• calculateSequenceIndex (13-19)
• isLastNode (30-32)

src/utils/typeGuards/typeGuards.ts (2)

• isTextNode (11-13)
• isElementWithChildren (35-42)

src/utils/generateAnimation/generateAnimation.ts (1)

• generateAnimation (21-48)

🔇 Additional comments (12)

src/utils/sequenceHelpers/sequenceHelpers.spec.ts (1)

17-26: LGTM!

The isLastNode tests appropriately cover the true and false branches with representative indices.

src/utils/sequenceHelpers/index.ts (1)

1-1: LGTM!

Clean barrel export that provides a single entry point for the sequence helper utilities.

src/utils/typeGuards/typeGuards.ts (3)

3-13: LGTM!

The updated JSDoc improves clarity by explicitly stating the return semantics. Consistent documentation pattern across all type guards.

---

15-25: LGTM!

Documentation update aligns with the consistent pattern established for type guards.

---

27-42: LGTM!

Documentation accurately describes the function behavior.

src/utils/sequenceHelpers/sequenceHelpers.ts (2)

13-19: LGTM! Clean pure function implementation.

The function correctly computes sequence indices for both animation orders. The formula totalNodes - currentIndex - 1 correctly maps the first element (0) to the last position and vice versa.

---

30-32: LGTM!

Simple and predictable. The function name clearly conveys its purpose.

src/components/TextMotion/TextMotion.tsx (2)

111-123: LGTM! Clear separation of rendering paths.

The flattened conditional structure improves readability by making the two rendering paths explicit. Both branches correctly:

• Attach targetRef for intersection observer
• Use text || DEFAULT_ARIA_LABEL for accessibility
• Apply distinct CSS classes for styling control

---

94-94: Acceptable trade-off for unconditional split computation.

Always calling splitNodeAndExtractText ensures text is available for the aria-label in both rendering paths. While this means computing splittedNode even when not animating, the consistency and simpler control flow justify the minor overhead.

src/hooks/useAnimatedChildren/useAnimatedChildren.tsx (3)

18-21: Good addition of explicit return type.

The WrapResult type makes the return shape of wrapWithAnimatedSpan explicit and predictable. This aligns well with the PR goal of removing implicit behavior.

---

43-58: Clean refactor removing ref-based state management.

The hook now delegates sequence index calculation to wrapWithAnimatedSpan with an explicit starting index of 0. This makes the flow easier to follow and removes hidden state dependencies. The memoization dependencies are correctly specified.

---

61-69: Function signature is now explicit and pure.

Converting wrapWithAnimatedSpan to accept currentSequenceIndex and return WrapResult makes the data flow explicit. This aligns well with the PR goal of improving predictability by removing implicit state dependencies. The function is now easier to test in isolation.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

src/hooks/useAnimatedChildren/useAnimatedChildren.tsx (1)

62-107: Solid refactor from mutable ref to pure function pattern.

Converting wrapWithAnimatedSpan to return { nodes, nextSequenceIndex } instead of mutating a ref is a significant predictability improvement. The function's behavior is now self-contained and easier to reason about.

A few observations on the implementation:

1. Local mutation is acceptable here: The let sequenceIndex (line 71) is scoped within the function and explicitly returned, which is a common and appropriate pattern for accumulator-style logic.

2. Clear sequence propagation: The recursive call pattern (lines 86-95) where nextSequenceIndex is captured and assigned to sequenceIndex is explicit and traceable.

3. Consider naming for clarity: The parameter currentSequenceIndex and local sequenceIndex could be confused. A slight rename like startIndex for the parameter might improve readability, but this is a minor nitpick.

src/utils/countNodes/countNodes.ts (1)

21-25: Consider renaming for predictability.

The logic is correct and well-structured. However, since this function now specifically counts text nodes rather than all nodes, consider whether renaming to countTextNodes or countAnimatableNodes would improve predictability.

The current name countNodes combined with the parameter nodes could lead readers to expect it counts all nodes, requiring them to read the JSDoc or implementation to understand the actual behavior.

Trade-off: Renaming would improve self-documentation but requires updating all call sites. If the function is only used in animation contexts where "nodes" implicitly means "animatable nodes," keeping the name may be acceptable.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b8692c4 and d6e4f9f.

📒 Files selected for processing (3)

• src/hooks/useAnimatedChildren/useAnimatedChildren.tsx
• src/utils/countNodes/countNodes.spec.tsx
• src/utils/countNodes/countNodes.ts

🧰 Additional context used

📓 Path-based instructions (1)

**/*.{ts,tsx}

⚙️ CodeRabbit configuration file

**/*.{ts,tsx}: You are a senior frontend reviewer.
Review this code strictly based on the principles of "easy-to-change code"
from Frontend Fundamentals (Readability, Predictability, Cohesion, Low Coupling).

================================

1. Readability
   ================================

• Check if the code can be read top-to-bottom like a document.
• Flag places where the reader must jump around to understand logic.
• Suggest extracting logic when multiple contexts are mixed in one function or component.
• Identify magic numbers or strings and suggest meaningful named constants.
• Point out unclear or abbreviated variable/function names.
• Prefer simple control flow over nested or overly clever logic.

================================
2. Predictability

• Verify that function and variable names clearly imply their behavior.
• Ensure functions with similar responsibilities return consistent data shapes.
• Flag hidden side effects (global state mutation, implicit behavior).
• Warn when the same name is used with different meanings across the codebase.
• Suggest making implicit assumptions explicit.

================================
3. Cohesion

• Check whether code that changes together is located together.
• Flag cases where related logic is scattered across multiple files unnecessarily.
• Suggest grouping constants, helpers, and hooks near the domain they belong to.
• Identify components or functions that contain unrelated responsibilities.
• Prefer feature/domain-based structure over technical grouping when applicable.

================================
4. Coupling

• Identify code where a small change would cause wide ripple effects.
• Warn when abstractions increase coupling instead of reducing it.
• Prefer duplication over premature abstraction if it lowers change impact.
• Flag excessive prop drilling and suggest alternative patterns if appropriate.
• Ensure compo...

Files:

• src/utils/countNodes/countNodes.ts
• src/utils/countNodes/countNodes.spec.tsx
• src/hooks/useAnimatedChildren/useAnimatedChildren.tsx

🧬 Code graph analysis (2)

src/utils/countNodes/countNodes.ts (1)

src/utils/typeGuards/typeGuards.ts (2)

• isTextNode (11-13)
• isElementWithChildren (35-42)

src/utils/countNodes/countNodes.spec.tsx (1)

src/utils/countNodes/countNodes.ts (1)

• countNodes (13-29)

🔇 Additional comments (7)

src/hooks/useAnimatedChildren/useAnimatedChildren.tsx (3)

7-8: Good extraction of sequence helpers.

The extraction of calculateSequenceIndex and isLastNode into dedicated helpers improves cohesion by grouping sequence-related logic together and makes the animation ordering logic easier to test independently.

---

18-21: Well-defined return type for clarity.

The WrapResult type makes the function's contract explicit and improves predictability by clearly communicating that the function returns both the wrapped nodes and the updated sequence index.

---

46-56: Clean hook implementation with explicit data flow.

Destructuring { nodes } from the result and passing 0 as the initial sequence index makes the data flow explicit and easier to trace compared to the previous ref-based approach.

src/utils/countNodes/countNodes.ts (1)

7-11: Documentation clearly explains the refined behavior.

The updated JSDoc accurately describes that this counts text nodes specifically (the animatable nodes). This makes the function's purpose explicit.

src/utils/countNodes/countNodes.spec.tsx (3)

13-23: Test expectations correctly updated for text-node counting.

The distinction between <>Hello World</> (single text node = 1) and {'Hello'} {'World'} (three separate text nodes including the space = 3) correctly validates the new counting behavior.

---

87-90: Good edge case: empty array returns 0.

The test description update from implicit to explicit "should return 0 for an empty array" improves test readability.

---

116-132: Comprehensive complex structure test.

This test effectively validates the recursive counting through multiple nesting levels. The expected count of 5 correctly accounts for: "Line 1", "Part 1 ", "Part 2", "Code", and " More Code".