Skip to content

Normalize reverse/loop ordering in keyframe animation blocks#489

Merged
tracygardner merged 1 commit intomainfrom
codex/fix-animate-keyframe-order
Mar 29, 2026
Merged

Normalize reverse/loop ordering in keyframe animation blocks#489
tracygardner merged 1 commit intomainfrom
codex/fix-animate-keyframe-order

Conversation

@tracygardner
Copy link
Copy Markdown
Contributor

@tracygardner tracygardner commented Mar 29, 2026

Motivation

  • The animate_keyframes and animation blocks had the LOOP and REVERSE checkboxes in the opposite order compared to other animate blocks, causing UI inconsistency.
  • The generator output and localized block text also reflected the old ordering, so generated option objects and displayed labels were out of sync with the rest of the animate category.

Description

  • Reordered the REVERSE/LOOP field placement in animate_keyframes and animation in blocks/animate.js so REVERSE appears before LOOP.
  • Updated generators/generators-animate.js to read REVERSE before LOOP and to emit the generated animation options with reverse before loop for consistency.
  • Updated localized strings in locale/en.js, locale/fr.js, locale/es.js, locale/de.js, locale/it.js, locale/pl.js, locale/pt.js, and locale/sv.js so tooltips/labels reflect the new reverse-then-loop wording order.

Testing

  • Ran npx eslint blocks/animate.js generators/generators-animate.js locale/en.js locale/fr.js locale/es.js locale/de.js locale/it.js locale/pl.js locale/pt.js locale/sv.js, which completed without errors for the changed files.
  • Ran the repository lint via npm run -s lint, which failed due to many pre-existing repository-wide lint issues unrelated to this change.
  • No behavior changes beyond ordering and label/text updates; block generators now produce options with reverse before loop to match the UI.

Codex Task

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Fixed parameter ordering for animation blocks to ensure loop and reverse animation settings are correctly mapped and displayed across the interface in all supported languages.

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages bot commented Mar 29, 2026

Deploying flockdev with  Cloudflare Pages  Cloudflare Pages

Latest commit: b16b0e0
Status: ✅  Deploy successful!
Preview URL: https://da3c2a40.flockdev.pages.dev
Branch Preview URL: https://codex-fix-animate-keyframe-o.flockdev.pages.dev

View logs

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 29, 2026

📝 Walkthrough

Walkthrough

A coordinated parameter reordering swaps the LOOP and REVERSE options in animation blocks. Block field definitions are swapped, code generators output the new parameter order, and localization strings across 8 languages are updated with adjusted placeholder indices to reflect the new mapping.

Changes

Cohort / File(s) Summary
Block Definition
blocks/animate.js
Swapped checkbox field mappings for LOOP and REVERSE in the animate_keyframes and animation block definitions, causing each checkbox to associate with the opposite option name.
Code Generator
generators/generators-animate.js
Reordered parameter assignments in both animation generators to output loop after reverse, reflecting the new field order without changing logic or computed values.
Localizations
locale/de.js, locale/en.js, locale/es.js, locale/fr.js, locale/it.js, locale/pl.js, locale/pt.js, locale/sv.js
Updated placeholder indices across all 8 language locales to swap the positions of REVERSE and LOOP in animate_keyframes and animation block message templates, ensuring correct parameter binding in rendered block labels.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • Generators refactor #470: Directly related refactor involving the same LOOP/REVERSE field swaps and corresponding generator parameter reordering in animation blocks.

Poem

🐰 Hops of joy! The reverse and loop swap places with grace,
Parameters pirouette in a new embrace,
Eight tongues now speak the order anew,
Blockly blocks dance—old becomes true! 🎞️✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: reordering REVERSE and LOOP fields in keyframe animation blocks to achieve consistency across the animate category.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-animate-keyframe-order

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
generators/generators-animate.js (1)

266-280: Add a generator regression test for REVERSE/LOOP mapping.

This change is wiring-sensitive, and current coverage appears API-focused rather than generator-output-focused. A focused generator assertion for REVERSE/LOOP combinations would prevent silent field-order regressions.

Also applies to: 379-381

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@generators/generators-animate.js` around lines 266 - 280, Add a focused
regression test that exercises the generators/generators-animate.js output for
REVERSE/LOOP combinations by invoking the animation generator (the code that
emits the createAnimation call using animationGroupVar, meshVariable, property,
keyframesCode, easing, reverse, loop, mode) and asserting the generated string
includes the expected mapping for reverse and loop (and their relative ordering)
for each combination of REVERSE/LOOP values; ensure the test checks the exact
generated snippet (or a strict regex) that contains "reverse: <value>" and
"loop: <value>" within the createAnimation call so future wiring/field-order
regressions are caught.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@generators/generators-animate.js`:
- Around line 266-280: Add a focused regression test that exercises the
generators/generators-animate.js output for REVERSE/LOOP combinations by
invoking the animation generator (the code that emits the createAnimation call
using animationGroupVar, meshVariable, property, keyframesCode, easing, reverse,
loop, mode) and asserting the generated string includes the expected mapping for
reverse and loop (and their relative ordering) for each combination of
REVERSE/LOOP values; ensure the test checks the exact generated snippet (or a
strict regex) that contains "reverse: <value>" and "loop: <value>" within the
createAnimation call so future wiring/field-order regressions are caught.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c5090c97-f7a9-4664-b1f1-5ac83202cbf0

📥 Commits

Reviewing files that changed from the base of the PR and between 999498f and b16b0e0.

📒 Files selected for processing (10)
  • blocks/animate.js
  • generators/generators-animate.js
  • locale/de.js
  • locale/en.js
  • locale/es.js
  • locale/fr.js
  • locale/it.js
  • locale/pl.js
  • locale/pt.js
  • locale/sv.js

@tracygardner tracygardner merged commit 71d13eb into main Mar 29, 2026
9 checks passed
@tracygardner tracygardner deleted the codex/fix-animate-keyframe-order branch March 29, 2026 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant