Skip to content

fix(prompt): require entry animations to preserve their final visible…#95

Open
crimsondhaks wants to merge 3 commits into
nexu-io:mainfrom
crimsondhaks:fix/animation-fill-mode-prompt-guardrail
Open

fix(prompt): require entry animations to preserve their final visible…#95
crimsondhaks wants to merge 3 commits into
nexu-io:mainfrom
crimsondhaks:fix/animation-fill-mode-prompt-guardrail

Conversation

@crimsondhaks

Copy link
Copy Markdown

Closes #89

Why

@lefarcen's diagnosis on the issue: the model paired inline opacity: 0
with a non-persistent animation: fadeIn 0.6s ease-out; — no forwards
fill mode — so every animated element fell back to opacity: 0 once the
keyframes finished, leaving the page blank except for the (non-animated)
title.

The fix lives at the prompt layer because the broken pattern can be
emitted by any of the 75 skills, and the directive that allowed it
("仅在必要处使用 transition-all 或入场 fade-in") didn't constrain how
the animation had to terminate. A single edit to SHARED_DESIGN_DIRECTIVES
covers every skill via assemblePrompt, so I didn't need to touch any
individual SKILL.md.

What changed

  • next/src/lib/templates/shared.ts (+9 lines) adds an
    【入场动画安全规则】 section that:

    1. accepts three concrete strategies — animation-fill-mode: forwards
      (or both), IntersectionObserver writing
      element.style.opacity = '1', or CSS transition (whose final
      state survives by definition);
    2. explicitly forbids the opacity: 0 + non-persistent fadeIn
      pairing the model was emitting;
    3. requires prefers-reduced-motion: reduce to restore visibility
      (opacity: 1; transform: none; animation: none;), not just
      shorten duration;
    4. requires a JS-failure fallback when visibility is gated on
      IntersectionObserver / scroll listeners.
  • next/src/lib/templates/__tests__/shared.test.ts (+84 lines) pins
    the directive shape with 7 regression tests so a future copy-edit
    cannot silently drop the guardrail.

Verification

pnpm exec tsx scripts/guard.ts                          # passed
pnpm -F @html-anything/next typecheck                   # passed
pnpm -F @html-anything/e2e typecheck                    # passed
pnpm -F @html-anything/next test                        # 145 passed (138 → 145)
pnpm -F @html-anything/next build                       # passed
pnpm -F @html-anything/e2e test                         # 4 passed

End-to-end check with Codex (codex-cli 0.130.0, model = default)

Ran POST /api/convert against the dev server with agent=codex,
templateId=saas-landing (this skill's body explicitly mentions
"滚动入场动画", which is exactly the trigger that produced the bug in
#89), and a small landing-page prompt for a fictitious product. Codex
returned a 29.7 KB single-file HTML in ~2.5 minutes (8,653 output
tokens). The generated styles:

.reveal { animation: fadeUp 0.7s ease-out both; }   /* 'both' = preserves final state */

@keyframes fadeUp {
  from { opacity: 0; transform: translateY(14px); }
  to   { opacity: 1; transform: translateY(0); }
}

@media (prefers-reduced-motion: reduce) {
  *, *::before, *::after { animation: none !important; transition: none !important; }
  .reveal { opacity: 1 !important; transform: none !important; }
}

Compared to the broken sample @lihu-001 attached to the issue, this is
the opposite: the keyframe sequence ends at opacity: 1, the both
fill mode locks that final state in, and the reduced-motion branch
restores visibility instead of just disabling motion.

I also opened the generated HTML in headless chromium, waited 4 seconds
(well past the 0.7s entry animation), and probed every text-bearing
element under <body> for computed opacity, dimensions, display, and
inherited opacity:

Total probes: 89, invisible after 4s: 0

i.e. all 89 header / section / article / h1 / h2 / h3 / p / li nodes
render at inheritedOpacity = 1.00 with non-zero bounding boxes after
the animation finishes.

[attach post-animation.png and reduced-motion.png here]
post-animation
reduced-motion

… state

Closes nexu-io#89

The agent was emitting pages where every animated element fell back to
inline opacity:0 after the entry animation finished, leaving the page
blank except for the (non-animated) title.

Root cause was at the prompt layer, not at any individual skill: the
shared design directives mentioned 'fade-in' as an acceptable motion but
did not constrain how it had to terminate. The model paired inline
opacity:0 with a non-persistent 'animation: fadeIn 0.6s ease-out' (no
forwards), so once the keyframe sequence ended the inline rule won and
hid the body content.

Adds a dedicated '入场动画安全规则' section to SHARED_DESIGN_DIRECTIVES
that:

- accepts three concrete strategies (animation-fill-mode: forwards/both,
  IntersectionObserver writing element.style.opacity = '1', or CSS
  transition where the final state is the post-toggle declaration);
- forbids the broken pairing the model produced;
- requires prefers-reduced-motion to restore visibility (not just
  shorten duration);
- requires a JS-failure fallback so blocked or slow scripts cannot leave
  the page invisible.

Lives in shared.ts so a single edit covers all 75 skills via
assemblePrompt — no per-skill change needed.

Adds shared.test.ts pinning the directive shape so a future copy-edit
cannot silently drop the guardrail.
@lefarcen lefarcen requested a review from Siri-Ray May 28, 2026 06:46
@lefarcen lefarcen added size/S Small change: 20-99 changed lines risk/medium Medium risk change type/bugfix Bug fix labels May 28, 2026

@lefarcen lefarcen 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.

Hi @crimsondhaks! 🎉 First PR to the repo — and a solid one.

The approach here is exactly right: a single edit to SHARED_DESIGN_DIRECTIVES (via assemblePrompt) means every skill inherits the guardrail without touching 75 individual template files. The e2e walk-through — 89 text-bearing elements all at inheritedOpacity = 1.00 after 4 seconds, plus the reduced-motion capture — is exactly the kind of evidence that makes a prompt-layer fix easy to land with confidence.

Two small things inline — a P2 on the JS-failure fallback example and a P3 nit on a redundant test assertion. Nothing blocking the core fix.

Comment thread next/src/lib/templates/shared.ts Outdated
3. 用 CSS \`transition\` 替代 \`@keyframes\`: 初始 \`opacity: 0; transform: translateY(8px); transition: opacity 0.6s, transform 0.6s;\`, 加 class 后改成 \`opacity: 1; transform: translateY(0);\` — transition 的最终态自动保留。
- **绝对禁止**: 把元素 inline / 在 CSS 里写死 \`opacity: 0\`, 然后只用一个不带 \`forwards\` 的 \`animation: fadeIn 0.6s ease-out;\` 当揭示动画。这会导致动画播完后元素回到 \`opacity: 0\`, 内容**消失看不见** (典型 bug: 标题外的所有正文动画结束后被隐藏)。
- **必须支持** \`@media (prefers-reduced-motion: reduce)\`: 在该条件下取消所有入场隐藏 (\`opacity: 1; transform: none; animation: none;\`), 让内容**直接可见, 不依赖任何动画完成**。
- **必须支持 JS 失败 fallback**: 如果用 IntersectionObserver / scroll 监听控制可见性, 必须在 \`<noscript>\` 里或者用 CSS \`@supports not (selector(:has(*)))\` 之类的兜底, 确保 JS 不执行 / 报错 / 慢加载时, 内容仍然可见 — 不要让用户因为浏览器扩展拦截脚本就看不到文字。

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 The @supports not (selector(:has(*))) example targets browsers that don't support the :has() CSS pseudo-class (pre-2022 engines), not JavaScript-disabled or JS-blocked scenarios. So an AI model following this literally would produce a fallback that only helps on very old browsers — not the browser-extension-blocking case the comment mentions.

The canonical CSS-progressive-enhancement pattern for JS failure is to default to visible and let JS opt in to the hiding:

/* Default: content visible (JS off / blocked) */
.reveal { opacity: 1; }

/* JS adds this class to <body> on load; only then do we animate */
.js-ready .reveal { opacity: 0; }
.js-ready .reveal.in-view {
  opacity: 1;
  transition: opacity 0.6s ease-out;
}

Then in JS:

document.addEventListener('DOMContentLoaded', () => {
  document.body.classList.add('js-ready');
});

The <noscript> case right before this is already correct. Consider replacing the @supports line with a brief reference to this pattern (or just a note that the <noscript> approach covers the disable case, and a .js-ready body class covers the extension-block case).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

yeah you're right, my @supports example was wrong , it actually checks for :has() support, which has nothing to do with JS being blocked. switched to your progressive-enhancement pattern in 74326a4. content stays visible by default, JS adds .js-ready on DOMContentLoaded which then enables the hide - reveal path. extension blocking, CSP errors, slow load , none of them can hide content anymore.

// The three accepted strategies all need to be discoverable in the
// prompt body, otherwise the model has no concrete target to copy.
expect(SHARED_DESIGN_DIRECTIVES).toMatch(/animation-fill-mode:\s*forwards/);
expect(SHARED_DESIGN_DIRECTIVES).toMatch(/forwards/);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3 toMatch(/forwards/) is a strict subset of the toMatch(/animation-fill-mode:\s*forwards/) assertion three lines above — any string satisfying the first also satisfies this one. Can drop this line; the more specific assertion already covers it.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

oh good catch, didn't notice that. dropped it in 749697f.

…pattern

Address P2 review feedback on nexu-io#95.

The previous @supports not (selector(:has(*))) example targeted browsers
that lack the :has() CSS pseudo-class (pre-2022 engines), not
JavaScript-disabled or extension-blocked scenarios. An AI literally
following it would have produced a fallback that only helps very old
browsers — not the browser-extension-blocking case the directive was
trying to address.

Replace with the canonical CSS-progressive-enhancement pattern: default
state is visible, JS opts into the hiding by adding a .js-ready body
class. <noscript> covers full JS-disabled, .js-ready covers blocked /
slow / errored JS. The two together cover every scenario where JS does
not finish initialising, and the model has a concrete target to copy.
Address P3 review feedback on nexu-io#95.

/forwards/ is a strict subset of /animation-fill-mode:\s*forwards/ —
any string satisfying the more specific assertion three lines above
also satisfies the broader one. Drop the redundant assertion; the
specific one already covers it.
@crimsondhaks

Copy link
Copy Markdown
Author

thanks! both fixed in two fixup commits, happy to squash if you'd rather have one. checks all pass locally (guard, typecheck, 145 tests, build).

@lefarcen

Copy link
Copy Markdown

Both fixes look exactly right from your descriptions — the .js-ready body-class pattern is the cleanest guarantee (content visible by default, JS opts in to the hide/reveal path, so any failure mode leaves content readable). Dropping the redundant assertion tidies the test nicely too.

No need to squash — the two commits are self-explanatory on their own. @Siri-Ray has the review assignment; once they've looked at the current head you're good. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk/medium Medium risk change size/S Small change: 20-99 changed lines type/bugfix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

文字内容模块因为动效隐藏了

2 participants