Conversation
ameerabuf
left a comment
ameerabuf
left a comment
There was a problem hiding this comment.
I agree with the direction, and it does seem to be the same information in much less text, but I do not feel comfortable removing so much without first testing to see there is no regression. We can use Monty's QA-flow on this branch or something like this...
Added customEffect rules to hover and click; Fixes to viewprogress.md
Added missing composite effect property in types
Added TOCs
ameerabuf
left a comment
ameerabuf
left a comment
There was a problem hiding this comment.
read click and hover. There were already quite a few comments, some are critical IMO, so I submit now and I will continue with the other triggers.
| Use `keyframeEffect` or `namedEffect` when the click should play an animation (CSS or WAAPI). Pair with `PointerTriggerParams` to control playback behavior. | ||
|
|
||
| **Pattern**: | ||
| Always include `fill: 'both'` so the effect remains applied while finished and is not garbage-collected, allowing it to be efficiently toggled. |
There was a problem hiding this comment.
I believe we can add a keyword like CRITICAL before such statements that will result in breaking the visuals if ignored. Same for the A11y statement at the top.
| trigger: 'click', | ||
| params: { | ||
| type: 'state' | ||
| type: '[POINTER_TYPE]' |
There was a problem hiding this comment.
Using the word 'pointer' in context of click/hover can confuse LLMs. Many times I asked for pointer-animations and got hover/click-animations. I believe we should minimize those by trying to avoid using 'pointer' in this context.
| { | ||
| key: '[TARGET_KEY]', | ||
| [EFFECT_TYPE]: [EFFECT_DEFINITION], | ||
| fill: '[FILL_MODE]', |
There was a problem hiding this comment.
if fill is always both in this case there is no reason for the variable
| ] | ||
| } | ||
| ``` | ||
| - `[SOURCE_KEY]` — identifier matching the `data-interact-key` attribute on the element that listens for clicks. |
There was a problem hiding this comment.
We need an alternative phrasing here that will also work for React integration
| - `'add'` — adds the state on enter. | ||
| - `'remove'` — removes the state on enter. | ||
| - `'toggle'` — adds the state on enter, removes on leave. Default. | ||
| - `'clear'` — clears all previously applied states on enter. |
There was a problem hiding this comment.
also here I don't get this one, is this for overriding on breakpoint?
| - Card hover affecting image, text, and button | ||
| - Navigation item hover affecting icon and text | ||
| - Complex component state changes | ||
| Use `transition` when all properties share timing. Use `transitionProperties` when each property needs independent `duration`, `delay`, or `easing`. |
There was a problem hiding this comment.
similarities like this between rules of the different triggers make me think that maybe having transition-effect.md, custom-effect.md, etc...
There was a problem hiding this comment.
It could also help us make sure there are no inconsistencies as there will be one source of truth (imagine editing all the relevant triggers accordingly every time you change slightly the interface of an effect-type)
| ## Rule 4: Sequences | ||
|
|
||
| **Pattern**: | ||
| Use sequences when a hover should sync/stagger animations across multiple elements. |
There was a problem hiding this comment.
mention the shared container parent?
| duration: [DURATION_MS], | ||
| easing: '[EASING_FUNCTION]', | ||
| fill: '[FILL_MODE]', | ||
| keyframeEffect: { |
| - `[SOURCE_KEY]` — same as Rule 1. | ||
| - `[POINTER_TYPE]` — same as Rule 1. | ||
| - `[OFFSET_MS]` — time offset between each child's animation start, in milliseconds. | ||
| - `[OFFSET_EASING]` — easing curve for the stagger distribution (e.g. `'sineOut'`, `'linear'`). |
There was a problem hiding this comment.
same comment about the different easing format
| # ViewEnter Trigger Rules for @wix/interact | ||
|
|
||
| These rules help generate viewport-based interactions using the `@wix/interact` library. ViewEnter triggers use Intersection Observer to detect when elements enter the viewport and are ideal for entrance animations, lazy loading effects, and scroll-triggered content reveals. | ||
| This document contains rules for generating viewport-based interactions using the `@wix/interact`. ViewEnter triggers use IntersectionObserver to detect when elements enter the viewport and are ideal for entrance animations, scroll-triggered content reveals, and lazy-loading effects. |
There was a problem hiding this comment.
viewport-based is factually correct but confusing. The use of the phrase scroll-triggered is also not ideal and can cause confusion.
| - If possible SHOULD be called server-side or at build time - possible also in client, e.g. when entire content is initially hidden | ||
| - MUST set `data-interact-initial="true"` on the root element, i.e. with `data-interact-key`, or `<Interaction initial={true}>` for React integration | ||
| - Only valid for `viewEnter` + `params.type: 'once'` where source and target are the same element | ||
| - Do NOT use for `hover`, `click`, or `viewEnter` with `repeat`/`alternate`/`state` types |
There was a problem hiding this comment.
why are we talking about hover, click here?
There was a problem hiding this comment.
Also, what "Do NOT use" mean? generate(config) should still be called for other effects that might need it, but initial should be false. This should be made clear.
| - MUST set `data-interact-initial="true"` on the root element, i.e. with `data-interact-key`, or `<Interaction initial={true}>` for React integration | ||
| - Only valid for `viewEnter` + `params.type: 'once'` where source and target are the same element | ||
| - Do NOT use for `hover`, `click`, or `viewEnter` with `repeat`/`alternate`/`state` types | ||
| - For `repeat`/`alternate`/`state` types, MUST manually apply the initial keyframe as style on the target element and use `fill: 'backwards'` or `fill: 'both'` to keep the effect applied when finished, rewound, or reversed |
There was a problem hiding this comment.
I don't think fill: 'backwards' ever makes sense here, you must use fill: 'both' no?
| { | ||
| key: 'hero', | ||
| trigger: 'viewEnter', | ||
| params: { type: 'once', threshold: 0.2 }, |
There was a problem hiding this comment.
I think we should keep the properties that are not fixed-value templated like in the rules (like threshold, duration, namedEffect), or with placeholder syntax like threshold: <threshold> instead of fixed values
| const css = generate(config); | ||
|
|
||
| **When to Apply**: | ||
| const html = ` |
There was a problem hiding this comment.
maybe also a react example?
| key: 'feature-card', | ||
| key: '[SOURCE_KEY]', | ||
| trigger: 'viewProgress', | ||
| conditions: ['[CONDITION_NAME]'], // optional |
| { opacity: '0', transform: 'translateY(-40px)' } | ||
| ] | ||
| key: '[TARGET_KEY]', | ||
| customEffect: (element: Element, progress: number) => { |
There was a problem hiding this comment.
let's at least make this whole rule match the time-based structure of the customEffect rule
| - **Tall wrapper** (`[SOURCE_KEY]`): An element with enough height to create scroll distance (e.g., `height: 300vh`). This is the ViewTimeline source. | ||
| - **Sticky container** (`[TARGET_KEY]` or parent of targets): A direct child with `position: sticky; top: 0; height: 100vh` that stays fixed in the viewport while the wrapper scrolls past. | ||
| - **Animated elements**: Children of the sticky container that receive the effects. |
There was a problem hiding this comment.
I feel like this is considerably less detailed and more limited than the previous version. There are many pitfalls here that make the animation look broken that are not mentioned.
There was a problem hiding this comment.
Also there are some case where you can get a similar effect using the children as the sources, with easier range calculations
|
|
||
| - **Unexpected behavior:** Check range names match intent; verify source visibility; ensure target elements exist. | ||
| - **Janky custom effects:** Simplify calculations; avoid layout-triggering reads in the callback. | ||
| - `[TALL_WRAPPER_KEY]`: Key for the tall outer element that defines the scroll distance — this is the ViewTimeline source |
There was a problem hiding this comment.
I think that some direction as to how tall it should be will go a long way here.
| - `[TALL_WRAPPER_KEY]`: Key for the tall outer element that defines the scroll distance — this is the ViewTimeline source | ||
| - `[STICKY_CHILD_KEY]`: Key for the animated element inside the sticky container | ||
| - `[EFFECT_NAME]` / `[EFFECT_KEYFRAMES]`: Same as Rule 1 | ||
| - `[START_PERCENTAGE]` / `[END_PERCENTAGE]`: 0–100 within the `contain` range, i.e. the phase where the sticky container is fully stuck |
There was a problem hiding this comment.
maybe separate to start and end for clarity
|
|
||
| ### Effect Types for PointerMove | ||
| - [Trigger Source Elements with `hitArea: 'self'`](#trigger-source-elements-with-hitarea-self) | ||
| - [PointerMoveParams](#pointermoveparams) |
There was a problem hiding this comment.
Do you see my point about the confusing naming?
| - [Rule 1: namedEffect](#rule-1-namedeffect) | ||
| - [Rule 2: keyframeEffect with Single Axis](#rule-2-keyframeeffect-with-single-axis) | ||
| - [Rule 3: Two keyframeEffects with Two Axes and `composite`](#rule-3-two-keyframeeffects-with-two-axes-and-composite) | ||
| - [Rule 4: customEffect](#rule-4-customeffect) |
There was a problem hiding this comment.
Not for this PR but we should consider now having the progress be always a single number and use that axis param also in customEffects
|
|
||
| ### Hit Area Configuration (`hitArea`) | ||
| - The source element **MUST NOT** have `pointer-events: none` — it needs to receive pointer events to track mouse movement. | ||
| - **MUST AVOID** using the same element as both source and target with `transform` effects. The transform shifts the hit area, causing jittery re-entry cycles. Instead, use `selector` to target a child element for the animation. |
There was a problem hiding this comment.
The description from hover.md is a bit better IMO, e.g. talking about size and position changes rather than specifically about transform.
| const velocity = progress.v || { x: 0, y: 0 }; | ||
| const speed = Math.sqrt(velocity.x ** 2 + velocity.y ** 2); | ||
| - `hitArea` — determines where mouse movement is tracked: | ||
| - `'self'` — tracks mouse within the source element's bounds only. Use for local hover effects. |
There was a problem hiding this comment.
using the word 'hover' might cause confusion.
| - `hitArea` — determines where mouse movement is tracked: | ||
| - `'self'` — tracks mouse within the source element's bounds only. Use for local hover effects. | ||
| - `'root'` — tracks mouse anywhere in the viewport. Use for global cursor followers, ambient effects. | ||
| - `axis` — restricts pointer tracking to a single axis. Only relevant when using `keyframeEffect`: |
There was a problem hiding this comment.
is it really though? Do we know to ignore it with other types of effects?
| ## Critical CSS (FOUC Prevention) | ||
|
|
||
| ### Viewport Entrance | ||
| `generate(config)` produces CSS that hides entrance elements until their animation plays. See [viewenter.md](./viewenter.md) for full details. |
There was a problem hiding this comment.
"entrance elements" is vague
| ``` | ||
|
|
||
| ### Interactive Toggle (Click) | ||
| Inside `<head>`: |
There was a problem hiding this comment.
I like this more than what is in viewEnter.md that sets the HTML as string to a js const.
| | `Interact.create(config)` | Initialize with a config. Returns the instance. | | ||
| | `Interact.registerEffects(presets)` | Register named effect presets before `create`. | | ||
| | `Interact.destroy()` | Tear down all instances. | | ||
| | `Interact.forceReducedMotion` | `boolean` — force reduced-motion behavior regardless of OS setting. | |
| | `Interact.destroy()` | Tear down all instances. | | ||
| | `Interact.forceReducedMotion` | `boolean` — force reduced-motion behavior regardless of OS setting. | | ||
| | `Interact.allowA11yTriggers` | `boolean` — enable accessibility triggers. | | ||
| | `Interact.setup(options)` | Configure global scroll/pointer/viewEnter options. | |
|
|
||
| --- | ||
|
|
||
| ## Static API |
There was a problem hiding this comment.
I think the importance of handling instances is high enough to be included here.
| - Create the full configuration up‑front and pass it in a single `create` call to avoid unintended overrides; subsequent calls replace the previous config. | ||
|
|
||
| **For web (Custom Elements):** | ||
| # @wix/interact — Rules |
There was a problem hiding this comment.
Upfront I can see that the order and structure of this file completely changed.
The idea of this one was to provide the most lean stand-alone guide for stable usage of LLMs. Meaning if it was the only textual context for the agent, it should be able to get basic working results.
It should go into detail only if necessary to not break, and so it did not review all of the different intricate differences of the triggers.
Also - trouble-shooting and general guidelines were promoted to top after it was usually ignored when at the bottom, and was deemed much more relevant than anything else by the results.
There was a problem hiding this comment.
Basically the idea is less "full documentation" and more "quick guide for dummies"
|
|
||
| ## Quick Start | ||
|
|
||
| Create the full config up-front and pass it in a single `create` call. Subsequent calls create new `Interact` instances. |
There was a problem hiding this comment.
Maybe best practice of when to intentionally use multiple instances?
| const config = { | ||
| // config-props | ||
| }; | ||
| Do NOT add observers/listeners manually. The runtime binds triggers and effects via element keys. |
There was a problem hiding this comment.
should be marked as CRITICAL IMO
| params?: TriggerParams; // trigger-specific options | ||
| effects?: (Effect | EffectRef)[]; | ||
| sequences?: (SequenceConfig | SequenceConfigRef)[]; | ||
| conditions?: string[]; // condition ids; all must pass |
There was a problem hiding this comment.
should match to ids from the top-level conditions object
| 1. **`Effect.key`** — if provided, the target is the `<interact-element>` with matching `data-interact-key`. | ||
| 2. **Registry Effect's `key`** — if the effect is an `EffectRef`, the `key` from the referenced registry entry is used. | ||
| 3. **Fallback to `Interaction.key`** — the source element acts as the target. | ||
| 4. After resolving the root target, `selector`, `listContainer`, and `listItemSelector` on the effect further refine which child elements are animated, following the same priority order as source resolution above. |
There was a problem hiding this comment.
If I understand correctly this could be problematic, no? If the Interaction has some key interaction-key and some of these selectors to select the correct source, but the effect also has a different key (effect-key), do these selectors apply to the target to? It might not be the intention..
| ### Source element resolution (Interaction) | ||
|
|
||
| The source element is the element the trigger attaches to. Resolved in priority order: | ||
|
|
||
| 1. **`listContainer` + `listItemSelector`** — trigger attaches to each element matching `listItemSelector` within the `listContainer`. | ||
| 2. **`listContainer` only** — trigger attaches to each immediate child of the container. | ||
| 3. **`listContainer` + `selector`** — trigger attaches to the element found via `querySelector` within each immediate child of the container. | ||
| 4. **`selector` only** — trigger attaches to all elements matching `querySelectorAll` within the root `<interact-element>`. | ||
| 5. **Fallback** — first child of `<interact-element>` (web) or the root element (react/vanilla). | ||
|
|
||
| ### Target element resolution (Effect) | ||
|
|
||
| The target element is the element the effect animates. Resolved in priority order: | ||
|
|
||
| 1. **`Effect.key`** — if provided, the target is the `<interact-element>` with matching `data-interact-key`. | ||
| 2. **Registry Effect's `key`** — if the effect is an `EffectRef`, the `key` from the referenced registry entry is used. | ||
| 3. **Fallback to `Interaction.key`** — the source element acts as the target. |
There was a problem hiding this comment.
This part is really relevant only when using the list-features or selector-features, because when not using it the resolution is simple. Since this file intention is to first allow simple usage I think it will be better to not delve into it so early in the rules and instead reference a later section that handles "selectors and lists resolving".
| ```ts | ||
| import { generate } from '@wix/interact/web'; | ||
| const css = generate(config); | ||
| // Include in <head>: <style>${css}</style> |
| - Only valid for `viewEnter` + `type: 'once'` where source and target are the same element. | ||
| - Do NOT use for `hover`, `click`, or `viewEnter` with `repeat`/`alternate`/`state`. |
There was a problem hiding this comment.
This 2 are the same rule, but if you feel it needs to be emphasized then OK
|
|
||
| --- | ||
|
|
||
| ## Common Pitfalls |
There was a problem hiding this comment.
this section should be much higher IMO, as all of its points are critical. Each entry here will break animations if ignored, and as far it is down the file, the more likely that LLM will deem it less important
| - **Stacking contexts and `viewProgress`**: Avoid `transform`, `filter`, `perspective`, `opacity < 1`, `will-change`, `contain: paint/layout/size` on the target or its ancestors. These can prevent or freeze ViewTimeline. Apply such styles to an inner child instead. | ||
| - **Perspective**: Prefer `transform: perspective(...)` inside keyframes. Use the CSS `perspective` property only when multiple children share the same `perspective-origin`. | ||
| - **Unknown preset options**: If you don't know the expected type/structure for a `namedEffect` param, omit it — rely on defaults rather than guessing. | ||
| - **Reduced motion**: Use conditions to provide gentler alternatives (shorter durations, fewer transforms, no perpetual motion) for users who prefer reduced motion. |
There was a problem hiding this comment.
mention forceReduceMotion?
| ## Static API | ||
|
|
||
| | Method / Property | Description | | ||
| | :---------------------------------- | :------------------------------------------------------------------ | | ||
| | `Interact.create(config)` | Initialize with a config. Returns the instance. | | ||
| | `Interact.registerEffects(presets)` | Register named effect presets before `create`. | | ||
| | `Interact.destroy()` | Tear down all instances. | | ||
| | `Interact.forceReducedMotion` | `boolean` — force reduced-motion behavior. | | ||
| | `Interact.allowA11yTriggers` | `boolean` — enable accessibility triggers (`interest`, `activate`). | | ||
| | `Interact.setup(options)` | Configure global scroll/pointer/viewEnter options. | |
There was a problem hiding this comment.
Each entry here is either already explained and documented above (create, registerEffects), or the complete opposite it was never mentioned and its importance or when to use it is unclear at all (forceReducedMotion, setup, ...)
2 participants
Description
hover.mdclick.mdviewprogress.mdviewenter.mdpointermove.mdintegration.mdscroll-list.mdfull-lean.md