Skip to content

docs(references): add Lottie animations guide#10

Merged
kherembourg merged 2 commits into
mainfrom
docs/lottie-reference
May 28, 2026
Merged

docs(references): add Lottie animations guide#10
kherembourg merged 2 commits into
mainfrom
docs/lottie-reference

Conversation

@kherembourg

Copy link
Copy Markdown
Contributor

Summary

  • Added references/concepts/lottie-animations.md for iOS/Android Lottie weak-dependency setup and troubleshooting
  • Linked the Lottie reference from concepts README, Purchasely skills, and sdk-expert
  • Updated CHANGELOG.md under Unreleased

Test Plan

  • npm test
  • git diff --check
  • npx --yes skills add . --list

@kherembourg kherembourg requested a review from Copilot May 28, 2026 13:05
@kherembourg

Copy link
Copy Markdown
Contributor Author

@greptileai review

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds a new universal reference guide for Lottie animations in Purchasely Screens, describing the weak-dependency model and the native iOS/Android bridge code teams must add. The guide is wired into the concepts README, the three Purchasely skills (review, integrate, debug), and the sdk-expert agent so each entry point can route Lottie questions to the new file. The changelog is updated under Unreleased.

Changes:

  • New references/concepts/lottie-animations.md covering iOS PLYLottieBridge, Android PLYLottieInterface + Purchasely.lottieView, cross-platform host-project notes, and troubleshooting.
  • Reference indexing/linking added in the concepts README, purchasely-integrate, purchasely-review (new checklist section), purchasely-debug (new pattern row), and sdk-expert (new rule + routing entry).
  • Changelog Unreleased entry describing the new reference.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
purchasely/references/concepts/lottie-animations.md New reference document with iOS/Android setup, cross-platform notes, and troubleshooting table.
purchasely/references/concepts/README.md Adds the Lottie reference to the concepts index and the "When to use what" table.
purchasely/skills/purchasely-integrate/SKILL.md Links the new reference and adds a Lottie row to the optional bonus integrations table.
purchasely/skills/purchasely-review/SKILL.md Links the reference and adds a new 3.13 Lottie review checklist section; renumbers Analytics to 3.14.
purchasely/skills/purchasely-debug/SKILL.md Links the reference and adds a debug pattern row for blank/static Lottie blocks.
purchasely/agents/sdk-expert.md Adds a Lottie rule, reference link, and routing entry; mentions Lottie in Console-driven topic list.
CHANGELOG.md Documents the new reference under Unreleased / Added.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread purchasely/skills/purchasely-review/SKILL.md Outdated
@greptile-apps

greptile-apps Bot commented May 28, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds a new lottie-animations.md reference covering the iOS PLYLottieBridge and Android PLYLottieInterface weak-dependency pattern, and wires it into the concepts README, sdk-expert agent, and all three skills.

  • New reference guide (lottie-animations.md): documents iOS Swift bridge, Android Kotlin bridge/factory, cross-platform host-project requirements, and a troubleshooting table.
  • Agent & skill updates: rule 17 in sdk-expert.md, reference-list entries in all three skills, and a new checklist section (3.13) in the review skill.
  • Two code-quality issues in the iOS/Android bridge samples: a transient retain cycle in the iOS factory closure and a behavioral mismatch between iOS stop() (resets to frame 0) and Android stop() (pauses in place).

Confidence Score: 4/5

Safe to merge; the documentation changes are well-structured but the two bridge code samples have a minor retain-cycle risk and an iOS/Android behavioral inconsistency that developers will copy verbatim.

The new reference file and agent/skill wiring are accurate and internally consistent. The iOS factory closure captures result strongly while the LottieAnimationView it creates holds that closure, forming a cycle that exists until Lottie releases the block on load completion. Separately, stop() resets to frame 0 on iOS but only pauses on Android, which will produce different UX for any app copying both samples. Neither issue causes a runtime crash in the happy path, but the code samples are reference material intended to be copied directly — both should be fixed before the guide is published.

purchasely/references/concepts/lottie-animations.md — iOS bridge closure and Android stop() implementation; purchasely/skills/purchasely-review/SKILL.md — section numbering gap

Important Files Changed

Filename Overview
purchasely/references/concepts/lottie-animations.md New reference file for Lottie weak-dependency setup; iOS bridge has a transient retain cycle in the factory closure, and Android stop() diverges in behavior from iOS stop()
purchasely/skills/purchasely-review/SKILL.md Added section 3.13 for Lottie and renumbered Analytics to 3.14; leaves a gap (no 3.11) in the section sequence
purchasely/agents/sdk-expert.md Added rule 17 and response guideline 14 for Lottie; renumbered prior rule 14 to 15; reference list updated correctly
purchasely/skills/purchasely-debug/SKILL.md Lottie entry added to the reference list and the known-issues table; accurate and consistent with the new guide
purchasely/skills/purchasely-integrate/SKILL.md Lottie entry added to optional-integrations reference list and the optional-features table; no issues found
purchasely/references/concepts/README.md Lottie row added to the concepts table and to the use-case routing section; link and description are accurate
CHANGELOG.md Unreleased entry added for the new lottie-animations.md reference; format is consistent with the rest of the changelog

Sequence Diagram

sequenceDiagram
    participant App
    participant PurchaselySDK as Purchasely SDK
    participant Bridge as PLYLottieBridge / PLYLottieInterface
    participant Lottie as Airbnb Lottie

    App->>PurchaselySDK: fetchPresentation(placementId)
    PurchaselySDK-->>App: presentation (with Lottie block)
    App->>PurchaselySDK: presentPresentation / display()
    PurchaselySDK->>Bridge: "bridge(with: animationURL) / lottieView { context }"
    Bridge->>Lottie: LottieAnimationView(url:) / setAnimationFromUrl()
    Lottie-->>Bridge: animation loaded → play()
    Bridge-->>PurchaselySDK: view() / AnimationView instance
    PurchaselySDK->>Bridge: loop(Bool) / fill(Bool)
    Bridge->>Lottie: loopMode / scaleType
    Note over PurchaselySDK,Bridge: SDK calls play/pause/stop on lifecycle events
    PurchaselySDK->>Bridge: stop()
    Bridge->>Lottie: iOS stop() resets to frame 0 vs Android pauseAnimation() pauses in place
Loading

Fix All in Claude Code Fix All in Cursor Fix All in Codex

Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
purchasely/references/concepts/lottie-animations.md:40-42
The completion closure captures `result` strongly, while `result.animationView` holds the `LottieAnimationView` that owns the closure — creating a reference cycle (`result → animationView → closure → result`). Lottie releases the completion block after the URL loads, so the cycle is transient rather than permanent, but while the animation is in flight any attempt to release the bridge will be blocked. Using `[weak result]` is the standard defensive pattern here and prevents surprises if Lottie's internal lifecycle changes across versions.

```suggestion
        result.animationView = LottieAnimationView(url: animationURL, closure: { [weak result] _ in
            result?.animationView?.play()
        }, animationCache: nil)
```

### Issue 2 of 3
purchasely/references/concepts/lottie-animations.md:120-122
The Android `stop()` implementation calls `pauseAnimation()`, which freezes the animation at its current frame. The iOS counterpart calls `animationView?.stop()`, which resets the animation back to frame 0. Developers copying both snippets will get different playback behavior on each platform when the SDK calls `stop()`. If the intent is to reset, the Android implementation should call `cancelAnimation()`.

```suggestion
    override fun stop() {
        cancelAnimation()
    }
```

### Issue 3 of 3
purchasely/skills/purchasely-review/SKILL.md:231
The section numbering now has a gap: after this change the sequence is 3.10 → 3.12 → 3.13 → 3.14, with no 3.11. The original file already had 3.12 (BYOS) physically before 3.11 (Analytics); this PR renamed Analytics from 3.11 to 3.14 and inserted Lottie as 3.13, but nothing now carries the 3.11 label. The Lottie section should be numbered 3.11 to fill the gap and keep the list contiguous.

```suggestion
### 3.11 Lottie Animations (if the Screen uses Lottie, or the user reports blank / static animations)
```

Reviews (1): Last reviewed commit: "docs(references): add Lottie animations ..." | Re-trigger Greptile

Comment thread purchasely/references/concepts/lottie-animations.md Outdated
Comment thread purchasely/references/concepts/lottie-animations.md
Comment thread purchasely/skills/purchasely-review/SKILL.md Outdated
@kherembourg

Copy link
Copy Markdown
Contributor Author

Addressed the 3 Greptile findings in 5ba052a:\n\n| # | Finding | Resolution |\n|---|---------|------------|\n| 1 | iOS Lottie closure captured the bridge strongly | Added [weak result] and optional access |\n| 2 | Android stop() paused while iOS stop() reset | Switched Android sample to cancelAnimation() |\n| 3 | Review checklist numbering gap | Renumbered BYOS/Lottie/Analytics to 3.11/3.12/3.13 |\n\nCI is green on the latest run.

@kherembourg kherembourg merged commit 4e94847 into main May 28, 2026
2 checks passed
@kherembourg kherembourg deleted the docs/lottie-reference branch May 28, 2026 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants