Enhancement: More events #171
Conversation
WalkthroughAdds tap lifecycle callbacks (onTapStart/onTap/onTapCancel) with keyboard accessibility and focusability logic, integrates them into MotionContainer and interaction utilities, expands tests (unit and E2E), introduces an a11y utility for native focusability, updates docs, adds a demo page, adjusts ESLint rules, and bumps a Trunk linter version. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant MC as MotionContainer
participant I as attachWhileTap
participant A as animate()
note over MC: Props include whileTap, onTapStart, onTap, onTapCancel
U->>MC: pointerdown / Enter keydown
MC->>I: start tap with callbacks
I->>A: animate(whileTap)
I-->>MC: onTapStart()
U->>MC: pointerup / Enter keyup
I->>A: animate(reset to baseline)
I-->>MC: onTap()
alt Element still hovered
I->>A: animate(hoverDef, hoverTransition)
note right of I: Reapply hover if active
else Not hovered
note right of I: Stay at baseline
end
U->>MC: blur / pointercancel
I->>A: animate(reset to baseline)
I-->>MC: onTapCancel()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 87
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/lib/html/Fegaussianblur.svelte (1)
8-10: Use correct SVG tag casing — feGaussianBlur (MotionContainer doesn't normalize)MotionContainer renders dynamic elements with <svelte:element this={tag}> and I found no xmlns/namespace or casing normalization in _MotionContainer.svelte; change the wrapper to the SVG-canonical name.
src/lib/html/Fegaussianblur.svelte:8-10
-<MotionContainer tag="fegaussianblur" {...rest}>
+<MotionContainer tag="feGaussianBlur" {...rest}>src/lib/html/Fepointlight.svelte (1)
8-10: Use canonical camelCase for the SVG tagMotionContainer currently passes tag="fepointlight" — change it to tag="fePointLight"; SVG element names are canonical/case‑sensitive in the DOM/CSS and Svelte compiles to SVG, so lowercase may break selectors/DOM APIs.
File: src/lib/html/Fepointlight.svelte (lines 8-10)
<MotionContainer tag="fepointlight" {...rest}> {@render children?.()} </MotionContainer>src/lib/utils/interaction.ts (2)
132-142: Consider hover reapply on blur as well (minor)If an element blurs while still hovered (possible via programmatic focus moves), reapplying hover keeps visual continuity. Optional, but consistent.
const handleBlur = () => { if (!keyboardActive) return keyboardActive = false callbacks?.onTapCancel?.() - if (initial || animateDef) { + if (reapplyHoverIfActive()) return + if (initial || animateDef) { const resetRecord = buildTapResetRecord(initial ?? {}, animateDef ?? {}, whileTap ?? {}) if (Object.keys(resetRecord).length > 0) { animate(el, resetRecord as unknown as DOMKeyframesDefinition) } } }
5-16: Doc nit: clarify omission when baseline lacks a key present in whileTapThe implementation omits keys that aren’t in initial/animate (good). Update the comment to reflect this behavior.
* For each key present in `whileTap`, this returns the value from `animate` - * if provided, otherwise from `initial`. Keys not present in `whileTap` are - * omitted to avoid unintended style changes. + * if provided, otherwise from `initial`. Keys present in `whileTap` but absent + * from both `animate` and `initial` are omitted. Keys not present in `whileTap` + * are omitted to avoid unintended style changes.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (182)
.trunk/trunk.yaml(1 hunks)PRD.md(3 hunks)README.md(2 hunks)e2e/motion/hover-and-tap.test.ts(1 hunks)eslint.config.mjs(1 hunks)scripts/_template.template(1 hunks)scripts/_template_void.template(1 hunks)src/lib/html/A.svelte(1 hunks)src/lib/html/Abbr.svelte(1 hunks)src/lib/html/Address.svelte(1 hunks)src/lib/html/Animate.svelte(1 hunks)src/lib/html/Animatemotion.svelte(1 hunks)src/lib/html/Animatetransform.svelte(1 hunks)src/lib/html/Area.svelte(1 hunks)src/lib/html/Article.svelte(1 hunks)src/lib/html/Aside.svelte(1 hunks)src/lib/html/Audio.svelte(1 hunks)src/lib/html/B.svelte(1 hunks)src/lib/html/Base.svelte(1 hunks)src/lib/html/Bdi.svelte(1 hunks)src/lib/html/Bdo.svelte(1 hunks)src/lib/html/Blockquote.svelte(1 hunks)src/lib/html/Br.svelte(1 hunks)src/lib/html/Button.svelte(1 hunks)src/lib/html/Canvas.svelte(1 hunks)src/lib/html/Caption.svelte(1 hunks)src/lib/html/Circle.svelte(1 hunks)src/lib/html/Cite.svelte(1 hunks)src/lib/html/Clippath.svelte(1 hunks)src/lib/html/Code.svelte(1 hunks)src/lib/html/Col.svelte(1 hunks)src/lib/html/Colgroup.svelte(1 hunks)src/lib/html/Cursor.svelte(1 hunks)src/lib/html/Data.svelte(1 hunks)src/lib/html/Datalist.svelte(1 hunks)src/lib/html/Dd.svelte(1 hunks)src/lib/html/Defs.svelte(1 hunks)src/lib/html/Del.svelte(1 hunks)src/lib/html/Desc.svelte(1 hunks)src/lib/html/Details.svelte(1 hunks)src/lib/html/Dfn.svelte(1 hunks)src/lib/html/Dialog.svelte(1 hunks)src/lib/html/Div.svelte(1 hunks)src/lib/html/Dl.svelte(1 hunks)src/lib/html/Dt.svelte(1 hunks)src/lib/html/Ellipse.svelte(1 hunks)src/lib/html/Em.svelte(1 hunks)src/lib/html/Embed.svelte(1 hunks)src/lib/html/Feblend.svelte(1 hunks)src/lib/html/Fecolormatrix.svelte(1 hunks)src/lib/html/Fecomponenttransfer.svelte(1 hunks)src/lib/html/Fecomposite.svelte(1 hunks)src/lib/html/Feconvolvematrix.svelte(1 hunks)src/lib/html/Fediffuselighting.svelte(1 hunks)src/lib/html/Fedisplacementmap.svelte(1 hunks)src/lib/html/Fedistantlight.svelte(1 hunks)src/lib/html/Feflood.svelte(1 hunks)src/lib/html/Fefunca.svelte(1 hunks)src/lib/html/Fefuncb.svelte(1 hunks)src/lib/html/Fefuncg.svelte(1 hunks)src/lib/html/Fefuncr.svelte(1 hunks)src/lib/html/Fegaussianblur.svelte(1 hunks)src/lib/html/Feimage.svelte(1 hunks)src/lib/html/Femerge.svelte(1 hunks)src/lib/html/Femergenode.svelte(1 hunks)src/lib/html/Femorphology.svelte(1 hunks)src/lib/html/Feoffset.svelte(1 hunks)src/lib/html/Fepointlight.svelte(1 hunks)src/lib/html/Fespecularlighting.svelte(1 hunks)src/lib/html/Fespotlight.svelte(1 hunks)src/lib/html/Fetile.svelte(1 hunks)src/lib/html/Feturbulence.svelte(1 hunks)src/lib/html/Fieldset.svelte(1 hunks)src/lib/html/Figcaption.svelte(1 hunks)src/lib/html/Figure.svelte(1 hunks)src/lib/html/Filter.svelte(1 hunks)src/lib/html/Footer.svelte(1 hunks)src/lib/html/Foreignobject.svelte(1 hunks)src/lib/html/Form.svelte(1 hunks)src/lib/html/G.svelte(1 hunks)src/lib/html/H1.svelte(1 hunks)src/lib/html/H2.svelte(1 hunks)src/lib/html/H3.svelte(1 hunks)src/lib/html/H4.svelte(1 hunks)src/lib/html/H5.svelte(1 hunks)src/lib/html/H6.svelte(1 hunks)src/lib/html/Header.svelte(1 hunks)src/lib/html/Hgroup.svelte(1 hunks)src/lib/html/Hr.svelte(1 hunks)src/lib/html/I.svelte(1 hunks)src/lib/html/Iframe.svelte(1 hunks)src/lib/html/Image.svelte(1 hunks)src/lib/html/Img.svelte(1 hunks)src/lib/html/Input.svelte(1 hunks)src/lib/html/Ins.svelte(1 hunks)src/lib/html/Kbd.svelte(1 hunks)src/lib/html/Label.svelte(1 hunks)src/lib/html/Legend.svelte(1 hunks)src/lib/html/Li.svelte(1 hunks)src/lib/html/Line.svelte(1 hunks)src/lib/html/Lineargradient.svelte(1 hunks)src/lib/html/Main.svelte(1 hunks)src/lib/html/Map.svelte(1 hunks)src/lib/html/Mark.svelte(1 hunks)src/lib/html/Marker.svelte(1 hunks)src/lib/html/Mask.svelte(1 hunks)src/lib/html/Math.svelte(1 hunks)src/lib/html/Menu.svelte(1 hunks)src/lib/html/Metadata.svelte(1 hunks)src/lib/html/Meter.svelte(1 hunks)src/lib/html/Mpath.svelte(1 hunks)src/lib/html/Nav.svelte(1 hunks)src/lib/html/Noscript.svelte(1 hunks)src/lib/html/Object.svelte(1 hunks)src/lib/html/Ol.svelte(1 hunks)src/lib/html/Optgroup.svelte(1 hunks)src/lib/html/Option.svelte(1 hunks)src/lib/html/Output.svelte(1 hunks)src/lib/html/P.svelte(1 hunks)src/lib/html/Path.svelte(1 hunks)src/lib/html/Pattern.svelte(1 hunks)src/lib/html/Picture.svelte(1 hunks)src/lib/html/Polygon.svelte(1 hunks)src/lib/html/Polyline.svelte(1 hunks)src/lib/html/Pre.svelte(1 hunks)src/lib/html/Progress.svelte(1 hunks)src/lib/html/Q.svelte(1 hunks)src/lib/html/Radialgradient.svelte(1 hunks)src/lib/html/Rect.svelte(1 hunks)src/lib/html/Rp.svelte(1 hunks)src/lib/html/Rt.svelte(1 hunks)src/lib/html/Ruby.svelte(1 hunks)src/lib/html/S.svelte(1 hunks)src/lib/html/Samp.svelte(1 hunks)src/lib/html/Script.svelte(1 hunks)src/lib/html/Search.svelte(1 hunks)src/lib/html/Section.svelte(1 hunks)src/lib/html/Select.svelte(1 hunks)src/lib/html/Selectedcontent.svelte(1 hunks)src/lib/html/Set.svelte(1 hunks)src/lib/html/Slot.svelte(1 hunks)src/lib/html/Small.svelte(1 hunks)src/lib/html/Source.svelte(1 hunks)src/lib/html/Span.svelte(1 hunks)src/lib/html/Stop.svelte(1 hunks)src/lib/html/Strong.svelte(1 hunks)src/lib/html/Style.svelte(1 hunks)src/lib/html/Sub.svelte(1 hunks)src/lib/html/Summary.svelte(1 hunks)src/lib/html/Sup.svelte(1 hunks)src/lib/html/Svg.svelte(1 hunks)src/lib/html/Switch.svelte(1 hunks)src/lib/html/Symbol.svelte(1 hunks)src/lib/html/Table.svelte(1 hunks)src/lib/html/Tbody.svelte(1 hunks)src/lib/html/Td.svelte(1 hunks)src/lib/html/Template.svelte(1 hunks)src/lib/html/Text.svelte(1 hunks)src/lib/html/Textarea.svelte(1 hunks)src/lib/html/Textpath.svelte(1 hunks)src/lib/html/Tfoot.svelte(1 hunks)src/lib/html/Th.svelte(1 hunks)src/lib/html/Thead.svelte(1 hunks)src/lib/html/Time.svelte(1 hunks)src/lib/html/Title.svelte(1 hunks)src/lib/html/Tr.svelte(1 hunks)src/lib/html/Track.svelte(1 hunks)src/lib/html/Tref.svelte(1 hunks)src/lib/html/Tspan.svelte(1 hunks)src/lib/html/U.svelte(1 hunks)src/lib/html/Ul.svelte(1 hunks)src/lib/html/Use.svelte(1 hunks)src/lib/html/Var.svelte(1 hunks)src/lib/html/Video.svelte(1 hunks)src/lib/html/View.svelte(1 hunks)src/lib/html/Wbr.svelte(1 hunks)src/lib/html/_MotionContainer.svelte(8 hunks)src/lib/types.ts(2 hunks)src/lib/utils/interaction.spec.ts(2 hunks)src/lib/utils/interaction.ts(3 hunks)src/lib/utils/layout.spec.ts(3 hunks)src/routes/tests/motion/hover-and-tap/+page.svelte(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.svelte
📄 CodeRabbit inference engine (.cursor/rules/svelte.mdc)
**/*.svelte: Use Svelte 5 syntax in all Svelte components
When adding comments to Svelte components, follow the guidance at https://svelte.dev/docs/svelte/faq#How-do-I-document-my-components
Files:
src/lib/html/Ellipse.sveltesrc/lib/html/Canvas.sveltesrc/lib/html/Ruby.sveltesrc/lib/html/Td.sveltesrc/lib/html/Dfn.sveltesrc/lib/html/Svg.sveltesrc/lib/html/Feimage.sveltesrc/lib/html/Marker.sveltesrc/lib/html/Header.sveltesrc/lib/html/Animate.sveltesrc/lib/html/S.sveltesrc/lib/html/Fetile.sveltesrc/lib/html/Ol.sveltesrc/lib/html/B.sveltesrc/lib/html/Filter.sveltesrc/lib/html/Animatetransform.sveltesrc/routes/tests/motion/hover-and-tap/+page.sveltesrc/lib/html/Tbody.sveltesrc/lib/html/Main.sveltesrc/lib/html/H3.sveltesrc/lib/html/Iframe.sveltesrc/lib/html/Polygon.sveltesrc/lib/html/Femorphology.sveltesrc/lib/html/Colgroup.sveltesrc/lib/html/Dialog.sveltesrc/lib/html/Tspan.sveltesrc/lib/html/Femerge.sveltesrc/lib/html/Selectedcontent.sveltesrc/lib/html/Feflood.sveltesrc/lib/html/Code.sveltesrc/lib/html/Bdo.sveltesrc/lib/html/Noscript.sveltesrc/lib/html/Fefuncb.sveltesrc/lib/html/Figcaption.sveltesrc/lib/html/Fespecularlighting.sveltesrc/lib/html/Embed.sveltesrc/lib/html/P.sveltesrc/lib/html/Symbol.sveltesrc/lib/html/Data.sveltesrc/lib/html/Var.sveltesrc/lib/html/Stop.sveltesrc/lib/html/Abbr.sveltesrc/lib/html/Button.sveltesrc/lib/html/Foreignobject.sveltesrc/lib/html/Pre.sveltesrc/lib/html/Map.sveltesrc/lib/html/Math.sveltesrc/lib/html/Fefunca.sveltesrc/lib/html/Form.sveltesrc/lib/html/Optgroup.sveltesrc/lib/html/Fecolormatrix.sveltesrc/lib/html/Track.sveltesrc/lib/html/Area.sveltesrc/lib/html/Li.sveltesrc/lib/html/Tref.sveltesrc/lib/html/Input.sveltesrc/lib/html/Ins.sveltesrc/lib/html/Switch.sveltesrc/lib/html/Fecomponenttransfer.sveltesrc/lib/html/Progress.sveltesrc/lib/html/Cite.sveltesrc/lib/html/Fedistantlight.sveltesrc/lib/html/Img.sveltesrc/lib/html/Col.sveltesrc/lib/html/Table.sveltesrc/lib/html/Em.sveltesrc/lib/html/Clippath.sveltesrc/lib/html/Article.sveltesrc/lib/html/Details.sveltesrc/lib/html/Tr.sveltesrc/lib/html/Set.sveltesrc/lib/html/Span.sveltesrc/lib/html/Label.sveltesrc/lib/html/G.sveltesrc/lib/html/Object.sveltesrc/lib/html/Ul.sveltesrc/lib/html/Feturbulence.sveltesrc/lib/html/Fespotlight.sveltesrc/lib/html/Div.sveltesrc/lib/html/Time.sveltesrc/lib/html/Dl.sveltesrc/lib/html/Small.sveltesrc/lib/html/Animatemotion.sveltesrc/lib/html/Menu.sveltesrc/lib/html/Option.sveltesrc/lib/html/Textpath.sveltesrc/lib/html/Base.sveltesrc/lib/html/H5.sveltesrc/lib/html/A.sveltesrc/lib/html/View.sveltesrc/lib/html/Datalist.sveltesrc/lib/html/Metadata.sveltesrc/lib/html/Defs.sveltesrc/lib/html/Fedisplacementmap.sveltesrc/lib/html/Summary.sveltesrc/lib/html/Rt.sveltesrc/lib/html/Hr.sveltesrc/lib/html/Output.sveltesrc/lib/html/Line.sveltesrc/lib/html/Use.sveltesrc/lib/html/Slot.sveltesrc/lib/html/Fefuncr.sveltesrc/lib/html/Image.sveltesrc/lib/html/H4.sveltesrc/lib/html/Rp.sveltesrc/lib/html/Feconvolvematrix.sveltesrc/lib/html/H6.sveltesrc/lib/html/Mpath.sveltesrc/lib/html/Kbd.sveltesrc/lib/html/Mask.sveltesrc/lib/html/Picture.sveltesrc/lib/html/Del.sveltesrc/lib/html/Strong.sveltesrc/lib/html/Pattern.sveltesrc/lib/html/Fieldset.sveltesrc/lib/html/Tfoot.sveltesrc/lib/html/Fediffuselighting.sveltesrc/lib/html/Lineargradient.sveltesrc/lib/html/Cursor.sveltesrc/lib/html/Samp.sveltesrc/lib/html/Search.sveltesrc/lib/html/Sub.sveltesrc/lib/html/Fegaussianblur.sveltesrc/lib/html/H2.sveltesrc/lib/html/Meter.sveltesrc/lib/html/Feblend.sveltesrc/lib/html/Style.sveltesrc/lib/html/Audio.sveltesrc/lib/html/Mark.sveltesrc/lib/html/Wbr.sveltesrc/lib/html/Th.sveltesrc/lib/html/Feoffset.sveltesrc/lib/html/Bdi.sveltesrc/lib/html/Fecomposite.sveltesrc/lib/html/Sup.sveltesrc/lib/html/Text.sveltesrc/lib/html/U.sveltesrc/lib/html/Hgroup.sveltesrc/lib/html/Caption.sveltesrc/lib/html/I.sveltesrc/lib/html/Q.sveltesrc/lib/html/Fefuncg.sveltesrc/lib/html/Thead.sveltesrc/lib/html/Desc.sveltesrc/lib/html/H1.sveltesrc/lib/html/Section.sveltesrc/lib/html/Fepointlight.sveltesrc/lib/html/Polyline.sveltesrc/lib/html/Script.sveltesrc/lib/html/Aside.sveltesrc/lib/html/Title.sveltesrc/lib/html/Dd.sveltesrc/lib/html/Nav.sveltesrc/lib/html/Circle.sveltesrc/lib/html/Blockquote.sveltesrc/lib/html/Select.sveltesrc/lib/html/Textarea.sveltesrc/lib/html/Video.sveltesrc/lib/html/Rect.sveltesrc/lib/html/Dt.sveltesrc/lib/html/Figure.sveltesrc/lib/html/Footer.sveltesrc/lib/html/Path.sveltesrc/lib/html/Br.sveltesrc/lib/html/Femergenode.sveltesrc/lib/html/Address.sveltesrc/lib/html/Radialgradient.sveltesrc/lib/html/Template.sveltesrc/lib/html/Source.sveltesrc/lib/html/_MotionContainer.sveltesrc/lib/html/Legend.svelte
🧠 Learnings (1)
📚 Learning: 2025-09-16T00:20:46.284Z
Learnt from: CR
PR: humanspeak/svelte-motion#0
File: .cursor/rules/svelte.mdc:0-0
Timestamp: 2025-09-16T00:20:46.284Z
Learning: Applies to **/*.svelte : Use Svelte 5 syntax in all Svelte components
Applied to files:
src/lib/html/H5.svelte
🧬 Code graph analysis (3)
src/lib/utils/layout.spec.ts (1)
src/lib/utils/layout.ts (3)
computeFlipTransforms(31-51)runFlipAnimation(63-95)observeLayoutChanges(120-157)
src/lib/utils/interaction.ts (1)
src/lib/utils/hover.ts (2)
isHoverCapable(13-21)splitHoverDefinition(29-37)
src/lib/utils/interaction.spec.ts (1)
src/lib/utils/interaction.ts (2)
buildTapResetRecord(17-33)attachWhileTap(48-159)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e-tests (1/2, 1-2)
- GitHub Check: e2e-tests (2/2, 2-2)
🔇 Additional comments (144)
.trunk/trunk.yaml (1)
75-75: LGTM!The TruffleHog version update from 3.90.6 to 3.90.8 is valid and includes recent improvements including dependency updates and bug fixes for GitLab enumeration and GitHub repository filtering.
src/lib/html/Summary.svelte (1)
5-5: Improved immutability pattern.The change from
lettoconstfor destructured props enhances code safety by preventing accidental reassignment of the extracted properties.src/lib/html/Fefuncr.svelte (1)
5-5: Improved immutability pattern.The change from
lettoconstfor destructured props enhances code safety by preventing accidental reassignment of the extracted properties.src/lib/html/H3.svelte (1)
5-5: Improved immutability pattern.The change from
lettoconstfor destructured props enhances code safety by preventing accidental reassignment of the extracted properties.src/lib/html/I.svelte (1)
5-5: Improved immutability pattern.The change from
lettoconstfor destructured props enhances code safety by preventing accidental reassignment of the extracted properties.src/lib/html/H2.svelte (1)
5-5: Improved immutability pattern.The change from
lettoconstfor destructured props enhances code safety by preventing accidental reassignment of the extracted properties.src/lib/html/Var.svelte (1)
5-5: Improved immutability pattern.The change from
lettoconstfor destructured props enhances code safety by preventing accidental reassignment of the extracted properties.src/lib/html/Embed.svelte (1)
5-5: Improved immutability pattern.The change from
lettoconstfor destructured props enhances code safety by preventing accidental reassignment of the extracted properties.src/lib/html/Mask.svelte (1)
3-3: Type props as SVG, not HTML for .mask is an SVG element; using HTMLElementProps will mis-type its attributes. File: src/lib/html/Mask.svelte imports HTMLElementProps while rendering .
- import type { HTMLElementProps } from '$lib/types.js' + import type { SVGElementProps } from '$lib/types.js'No exported SVG*ElementProps was found in the repo during verification — confirm an appropriate SVG props type exists in $lib/types and switch the import (or add one).
src/lib/html/Fetile.svelte (1)
8-8: Incorrect — keep the existing lowercasefetileto match project convention
- The repo consistently uses lowercase tag strings for SVG filter primitives (e.g., src/lib/html/Fecolormatrix.svelte, Fecomposite.svelte, Feflood.svelte). <MotionContainer tag="fetile" {...rest}> is consistent with that pattern; changing only this file to
feTilewould be inconsistent.- If you want camelCase (
feTile) across the library, do a global refactor: rename files and tag values under src/lib/html, and update typings (MotionContainer'stagshould include SvelteSVGElements or be widened; verify/replace HTMLElementProps → SVGElementProps where appropriate).Likely an incorrect or invalid review comment.
src/lib/html/Feconvolvematrix.svelte (1)
5-5: Good refactor to immutable destructuring.The change from
lettoconstfor$props()destructuring aligns with Svelte 5 best practices and the ESLint plugin recommendations. According to thesvelte/prefer-constESLint rule,const { a, b } = $props()is the preferred pattern for props destructuring in Svelte 5, as these values should not be reassigned directly in the component.src/lib/html/Option.svelte (1)
5-5: Correct immutability improvement.This change follows the established pattern of using
constfor$props()destructuring, which is the recommended approach in Svelte 5. The destructured props are not intended to be reassigned within the component, makingconstthe semantically correct choice.src/lib/html/Animate.svelte (1)
5-5: Appropriate immutability enhancement.The conversion to
constfor props destructuring is consistent with Svelte 5 best practices and prevents accidental reassignment of props within the component.src/lib/html/Textpath.svelte (1)
5-5: Consistent with immutable props pattern.This change aligns with the broader refactor across HTML components to use immutable destructuring for props, which is the recommended pattern in Svelte 5.
src/lib/html/View.svelte (1)
5-5: Follows Svelte 5 conventions correctly.The migration to
constfor$props()destructuring is appropriate and follows the established pattern. The ESLint plugin for Svelte specifically recommends this pattern, treating$propsas a special case whereconstis preferred.src/lib/html/Strong.svelte (1)
5-5: Proper immutability improvement.This change is part of the systematic refactor to use
constfor props destructuring, which prevents accidental mutations and aligns with Svelte 5 best practices.src/lib/html/Fecolormatrix.svelte (1)
5-5: Correctly applied immutability pattern.The switch from
lettoconstfor props destructuring is the right approach, ensuring that props are not accidentally reassigned within the component and following Svelte 5 conventions.src/lib/html/Li.svelte (1)
5-5: Proper implementation of immutable props.This change follows the established pattern across the codebase and correctly uses
constfor$props()destructuring, which is the recommended approach in Svelte 5 for preventing accidental prop mutations.src/lib/html/Dl.svelte (1)
5-5: Immutable destructuring looks good; Svelte 5 syntax respected.Change aligns with prefer-const and matches other HTML wrappers.
Confirm that destructuring from $props() here does not need runtime reactivity for forwarded props. If props are expected to change post-mount, consider a reactive extraction or forwarding directly from $props().
src/lib/utils/layout.spec.ts (2)
43-49: Good threshold coverage for tiny translations.Accurately locks the dx/dy < 0.5 behavior and no-scale case.
248-307: Fallback path coverage is good and restores globals.Nice job verifying timeout token is cleared on cleanup.
PRD.md (1)
52-52: Status line is consistent with shipped code.Docs reflect implemented tap callbacks and keyboard support.
src/lib/html/Code.svelte (1)
5-5: Consistent immutable props extraction; Svelte 5 syntax OK.Matches the wrapper pattern; no behavior change.
Same note as Dl.svelte: if post-mount prop updates must flow through, ensure destructuring from $props() here is sufficient (or forward directly from $props()).
src/lib/html/Sub.svelte (1)
5-5: LGTM: Immutable props destructure is correct and non-breaking.src/lib/html/Tr.svelte (1)
5-5: LGTM: let→const matches the repo-wide pattern; behavior unchanged.src/lib/html/Data.svelte (1)
5-5: LGTM: Safe immutability tweak; forwarding via {...rest} intact.src/lib/html/Small.svelte (1)
5-5: LGTM: Svelte 5-friendly const destructure; no functional change.src/lib/html/Col.svelte (1)
5-5: LGTM: Void-element props typed correctly; const destructure is appropriate.src/lib/html/Br.svelte (1)
5-5: LGTM: Void-element wrapper stays correct; immutable rest binding is fine.src/lib/html/Form.svelte (1)
5-5: Confirmed: onTap callbacks are typed and forwarded. src/lib/types.ts declares onTapStart/onTap/onTapCancel (lines 108, 110, 112) and src/lib/html/_MotionContainer.svelte wires/forwards them (lines 43–45 and 159–161); {...rest} will pass them through.src/lib/html/Dfn.svelte (1)
5-5: LGTM: immutable props destructuring.Matches Svelte 5 patterns; no reassignments needed here.
src/lib/html/Ruby.svelte (1)
5-5: LGTM: const destructuring is appropriate.No bindables involved; safe and consistent with other wrappers.
src/lib/html/Object.svelte (1)
5-5: LGTM: prop binding immutability.Change is safe; forwarding {...rest} remains unaffected.
src/lib/html/Script.svelte (1)
5-5: LGTM for the const switch.Note: if this wrapper ever needs bind: semantics, revert to let for the destructured $props().
Confirm we don’t advertise bind: usage for Script.svelte in docs/examples.
src/lib/html/Use.svelte (1)
5-5: LGTM: immutable destructuring.Consistent with Svelte 5 guidance.
src/lib/html/Feimage.svelte (1)
5-5: LGTM: const destructuring.Change is safe; forwarding and children render unaffected.
src/lib/html/Del.svelte (1)
5-5: LGTM — const destructuring from $props() is correctRan the suggested checks: no $bindable occurrences and no
let { children, ...rest } = $props()wrappers found; motion tap handlers/types exist in src/lib/html/_MotionContainer.svelte and src/lib/types.ts. Keep usingconst; switch toletonly if these props become$bindable.src/lib/html/Fefuncb.svelte (1)
5-5: Const destructure from $props() is correct (Svelte 5).Immutability here is preferred; no reassignment occurs.
src/lib/html/Mark.svelte (1)
5-5: Const destructure looks good; no behavior change.src/lib/html/H5.svelte (1)
5-5: Const destructure from $props() is correct and Svelte‑5‑compliant.src/lib/html/Text.svelte (1)
5-5: OK to make bindings immutable; matches repo‑wide pattern.src/lib/html/Sup.svelte (1)
5-5: Immutability change is correct; no runtime impact.src/lib/html/Fieldset.svelte (1)
5-5: LGTM: const destructure aligns with Svelte 5 patterns.src/lib/html/Dialog.svelte (1)
5-5: LGTM: const binding for $props() is the right choice.src/lib/html/H4.svelte (1)
5-5: Approve: switch to const — verify no remaininglet { … } = $props()usagesGood switch to const; matches Svelte 5 idiom. Automated verification failed (script error from an unescaped $). Re-run locally to confirm there are no mutable destructurings:
rg -n -C2 -g '**/.svelte' -P 'let\s{\s*[^}]}\s(?::\s*[^=;\n]+)?\s*=\s*$props(' || true
src/lib/html/Fespecularlighting.svelte (3)
5-5: Immutable destructuring: good changeConst binding is fine and consistent with the repo-wide shift.
1-6: Prefer SVG-specific prop typing for SVG wrappersUse SVGElementProps from $lib/types.js instead of HTMLElementProps for this SVG wrapper. No exported SVGElementProps was found in the repository — export/add SVGElementProps or keep HTMLElementProps.
File: src/lib/html/Fespecularlighting.svelte (lines 1-6)
8-8: Incorrect — lowercase 'fespecularlighting' is acceptable; no change needed.The HTML parser normalizes lower‑case tokens for foreign (SVG) elements so 'fespecularlighting' is interpreted as . (html.spec.whatwg.org) MDN shows the canonical element name as . (developer.mozilla.org) The repo consistently uses lowercase fe* wrappers (e.g. src/lib/html/Fespecularlighting.svelte); leave as‑is.
Likely an incorrect or invalid review comment.
src/lib/html/Search.svelte (1)
5-5: Const destructuring LGTMMatches Svelte 5 patterns; no functional change.
src/lib/html/Div.svelte (1)
5-5: LGTMImmutable props destructuring is correct and consistent.
src/lib/html/Address.svelte (1)
5-5: LGTMConst props destructuring aligns with Svelte 5 best practices.
src/lib/html/Section.svelte (1)
5-5: LGTMImmutable destructuring looks good; no behavior change.
src/lib/html/Polyline.svelte (2)
5-5: Const destructuring: goodConsistent with the broader refactor.
1-6: Do not change Polyline.svelte to SVGElementProps — that type doesn't exist; add it to src/lib/types.ts or keep HTMLElementPropssrc/lib/types.ts currently exports only HTMLElementProps (see export at ~line 157); there is no SVGElementProps to import.
- Preferred: add a SVGElementProps type in src/lib/types.ts (e.g. export type SVGElementProps = MotionProps & { children?: Snippet; [key: string]: unknown }) and then update src/lib/html/Polyline.svelte (and other SVG wrappers) to import SVGElementProps.
- Alternative: keep using HTMLElementProps to avoid mass changes.
Files to act on: src/lib/types.ts, src/lib/html/Polyline.svelte.
Likely an incorrect or invalid review comment.
src/routes/tests/motion/hover-and-tap/+page.svelte (1)
5-10: A11y: mark this square decorative or give it an accessible namee2e reference: e2e/motion/hover-and-tap.test.ts:9
If purely decorative / only for interaction testing, hide from SRs:
-<motion.div +<motion.div aria-hidden="true"If it should be announced, add role + accessible name:
-<motion.div +<motion.div role="img" aria-label="Interactive motion square"src/lib/html/Selectedcontent.svelte (1)
5-5: Immutable $props destructuring looks good.Const destructuring is idiomatic Svelte 5; no behavior change expected.
src/lib/html/Noscript.svelte (1)
5-5: Immutable $props destructuring: OK.Read-only binding is appropriate; usage of children remains unchanged.
src/lib/html/Area.svelte (1)
5-5: Void-element rest destructuring with const is correct.Consistent with Svelte 5 and the component’s void nature.
src/lib/html/Input.svelte (1)
5-5: Const rest destructuring for void element: looks good.No runtime impact; keeps props read-only.
src/lib/html/Datalist.svelte (1)
5-5: Immutable $props destructuring: OK.Change is safe and consistent across the HTML wrappers.
src/lib/html/Fefunca.svelte (1)
5-5: LGTM — verify feFuncA casing & SVG namespaceTag is passed as "fefunca" — ensure MotionContainer normalizes to "feFuncA" and creates the element with the SVG namespace (use createElementNS). Automated grep failed (rg didn't recognize 'svelte'); confirm MotionContainer's element-creation code. Location: src/lib/html/Fefunca.svelte:5
src/lib/html/Image.svelte (1)
5-5: LGTM — verify SVG namespace forMotionContainer renders dynamic tags with <svelte:element this={tag}> (src/lib/html/_MotionContainer.svelte) and Image.svelte passes tag="image" (src/lib/html/Image.svelte). Svelte may not reliably infer the SVG namespace for dynamic elements — make it explicit (e.g. add xmlns="http://www.w3.org/2000/svg" for SVG tags or detect SVG tags and use createElementNS). (svelte.dev)
src/lib/html/Figcaption.svelte (1)
5-5: Const destructuring of $props is correct (Svelte 5).
No behavior change; good immutability refactor.src/lib/html/Base.svelte (1)
5-5: LGTM on const rest binding.
Matches Svelte 5 $props pattern; void element usage is correct.src/lib/html/Lineargradient.svelte (2)
5-5: Const destructuring looks good.
No functional change; consistent with repo-wide refactor.
8-8: Verify SVG tag casing and MotionContainer SVG handlingCould not locate MotionContainer in the repo (search returned no results); cannot confirm whether it normalizes SVG tag names (svelte:element / createElementNS). Update the prop to camelCase unless MotionContainer handles SVG namespacing.
File: src/lib/html/Lineargradient.svelte — around line 8
-<MotionContainer tag="lineargradient" {...rest}> +<MotionContainer tag="linearGradient" {...rest}>Confirm MotionContainer uses namespaced SVG element creation or apply the change above.
src/lib/html/Femorphology.svelte (2)
5-5: Const destructuring is OK.
No change in runtime; consistent with Svelte 5 usage.
8-8: Confirm camelCase tag: feMorphology.
Prefer "feMorphology" to ensure correct SVG element creation unless MotionContainer normalizes names.[suggest_minor_refactor]
-<MotionContainer tag="femorphology" {...rest}> +<MotionContainer tag="feMorphology" {...rest}>Use the same verification script as suggested in Lineargradient.svelte to check SVG handling.
src/lib/html/Line.svelte (1)
5-5: Good switch to const.
Matches Svelte 5 props pattern.src/lib/html/Style.svelte (1)
5-5: Const destructuring: looks good.
No behavior change; consistent with Svelte 5.src/lib/html/Aside.svelte (1)
5-5: LGTM on immutability refactor.
Safe and consistent with the pattern across html wrappers.src/lib/html/Label.svelte (1)
5-5: LGTM on const props destructuring.
No functional differences; aligns with Svelte 5 guidance.src/lib/html/Audio.svelte (1)
5-5: LGTM — idiomatic Svelte 5 const destructuring of $props().No behavior change; forwarding via {...rest} remains clear.
src/lib/html/Animatemotion.svelte (2)
5-5: LGTM — const destructuring aligns with Svelte 5 guidance.
8-10: Verify SVG tag casing & namespace for MotionContainerSVG local names are case‑sensitive. tag="animatemotion" may look okay in inline markup due to HTML parser normalization but can fail when elements are created dynamically or in the wrong namespace. Confirm MotionContainer creates elements in the SVG namespace (Svelte should emit createElementNS / namespace-aware nodes) — if not, change to tag="animateMotion" or explicitly set the SVG namespace (e.g., <svelte:options namespace="svg" /> / pass xmlns) so the real SVG element/interface is created.
File: src/lib/html/Animatemotion.svelte — lines 8–10: <MotionContainer tag="animatemotion" {...rest}>...
src/lib/html/A.svelte (1)
5-5: LGTM — const $props() destructure is fine and keeps the wrapper minimal.src/lib/html/Metadata.svelte (1)
5-5: LGTM — consistent let→const refactor; no runtime change.src/lib/html/Em.svelte (1)
5-5: LGTM — immutable destructuring reads well and matches the other wrappers.src/lib/html/Dt.svelte (1)
5-5: LGTM — const $props() destructuring maintained consistently across files.src/lib/html/Fepointlight.svelte (1)
5-5: LGTM — mirrors the pattern used elsewhere.src/lib/html/Hgroup.svelte (1)
5-5: LGTM — const destructuring is appropriate here too.src/lib/html/Svg.svelte (1)
5-5: LGTM: immutable props destructuringSwitching to const is fine here and aligns with Svelte 5’s $props usage. No behavior change expected. (svelte.dev)
src/lib/html/Fedisplacementmap.svelte (1)
5-5: LGTM: const destructuringConsistent with the repo‑wide immutability refactor; no functional changes.
src/lib/html/Set.svelte (1)
5-5: LGTM: const destructuringMatches the Svelte 5 $props pattern and avoids accidental reassignments.
src/lib/html/Switch.svelte (1)
5-5: LGTM: const destructuringNo issues spotted; consistent with other wrappers.
src/lib/html/Feturbulence.svelte (1)
5-5: LGTM: const destructuringLooks good; mirrors the established wrapper pattern.
src/lib/html/Tbody.svelte (1)
5-5: LGTM: const destructuringChange is safe and consistent.
src/lib/html/Optgroup.svelte (1)
5-5: LGTM: const destructuringNo concerns; aligns with the rest of the HTML/SVG wrappers.
src/lib/html/Symbol.svelte (1)
5-5: Const destructure is fine; confirm you don’t need reactive props.
const { … } = $props()captures an initial snapshot. If parent updates props after mount,rest/childrenwon’t update. If reactivity is needed, switch to reactive re‑destructuring:- const { children, ...rest }: HTMLElementProps = $props() + let children: HTMLElementProps['children'] + let rest: Omit<HTMLElementProps, 'children'> + $: ({ children, ...rest } = $props())src/lib/html/Mpath.svelte (1)
5-5: Same note on reactivity with const destructuring.If these props can change post‑mount, use a reactive re‑destructure as shown in Symbol.svelte comment. Otherwise this is good.
src/lib/html/Feflood.svelte (1)
5-5: Immutability OK; verify no downstream need for prop reactivity.Use the reactive pattern if MotionContainer should reflect prop updates after mount.
src/lib/html/Bdo.svelte (1)
5-5: Const switch looks good; double‑check reactivity expectations.If parent updates should flow through, prefer
let+$:re‑destructuring.src/lib/html/Tspan.svelte (1)
5-5: OK to keep as const; confirm no reactive updates required.Adopt the reactive re‑destructuring if props need to update at runtime.
src/lib/html/Figure.svelte (1)
5-5: LGTM with the same reactivity caveat.If runtime prop changes must propagate, switch to the
$:re‑destructure pattern.src/lib/html/Button.svelte (1)
5-5: Button props often change (disabled, type, handlers); validate const won’t stale them.If these may update post‑mount, use reactive re‑destructuring to forward changes to MotionContainer.
src/lib/html/H6.svelte (1)
5-5: Const destructuring approved; verify no reactive prop updates are needed.Use the reactive pattern if updates should flow through.
src/lib/html/Foreignobject.svelte (1)
5-5: Immutable $props destructuring (let→const) — LGTMMatches the repo-wide Svelte 5 pattern; passthrough remains unchanged.
src/lib/html/Feoffset.svelte (1)
5-5: Immutable $props destructuring (let→const) — LGTMConsistent with Svelte 5 usage; no runtime impact.
src/lib/html/Main.svelte (1)
5-5: Immutable $props destructuring (let→const) — LGTMSvelte 5 rune style preserved; render path unchanged.
src/lib/html/Table.svelte (1)
5-5: Immutable $props destructuring (let→const) — LGTMNo functional change; aligns with the HTML wrapper convention.
src/lib/html/Canvas.svelte (1)
5-5: Immutable $props destructuring (let→const) — LGTMGood immutability clean-up; MotionContainer passthrough unaffected.
src/lib/html/Menu.svelte (1)
5-5: Immutable $props destructuring (let→const) — LGTMSvelte 5 syntax and behavior remain correct.
src/lib/html/G.svelte (1)
5-5: Immutable $props destructuring (let→const) — LGTMConsistent with other SVG/HTML wrappers; no issues spotted.
src/lib/html/Fedistantlight.svelte (1)
5-5: Immutable $props destructuring (let→const) — LGTM; rerun verification.rg produced no output; rerun to confirm there are no remaining
let { children, ...rest }: HTMLElementProps = $props()occurrences:rg -nP --hidden -g '!node_modules' -g '**/*.svelte' -e '^\s*let\s*\{\s*children\s*,\s*\.\.\.rest\s*\}\s*:\s*HTMLElementProps\s*=\s*\$props\(\)' # broader check (matches any `let { children, ...rest` in .svelte files) rg -nP --hidden -g '!node_modules' -g '**/*.svelte' -e '^\s*let\s*\{\s*children\s*,\s*\.\.\.rest'src/lib/html/Source.svelte (1)
5-5: LGTM: const + HTMLVoidElementProps is appropriate for a void element wrapper.No children usage; spread remains correct.
src/lib/html/Article.svelte (1)
5-5: LGTM: immutable $props destructuring matches Svelte 5 guidance.Pass‑through and children render unchanged.
src/lib/html/Map.svelte (1)
5-5: LGTM: switch to const is safe and idiomatic.No behavioral change; props still forwarded to MotionContainer.
src/lib/html/Rp.svelte (1)
5-5: LGTM: const destructuring improves intent (no reassignment).Pattern consistent with other HTML wrappers.
src/lib/html/P.svelte (1)
5-5: LGTM: Svelte 5‑style const destructuring and children rendering are correct.No further changes needed.
src/lib/html/Ol.svelte (1)
5-5: LGTM: immutability refactor is correct and consistent.Props forwarding remains the same.
src/lib/html/Defs.svelte (1)
5-5: LGTM: const destructuring aligns with Svelte 5 runes usage.No API/behavior change.
src/lib/html/Abbr.svelte (1)
5-5: Approve — const destructuring from $props() is Svelte 5–compliant; no mutable destructures found.Scanned src/lib/html — no
let { ... } = $props()matches; files useconst { ... } = $props()(example: src/lib/html/Abbr.svelte line 5).const { children, ...rest }: HTMLElementProps = $props()src/lib/html/Tref.svelte (1)
5-5: LGTM! Consistent with immutable props pattern.The change from
lettoconstfor props destructuring aligns with the broader refactor across HTML components and follows Svelte 5 best practices for immutable prop bindings.src/lib/html/Picture.svelte (1)
5-5: LGTM! Proper immutable props destructuring.The migration from
lettoconstfor props destructuring is correct and consistent with the broader codebase refactor to enforce immutability.src/lib/html/H1.svelte (1)
5-5: LGTM! Follows Svelte 5 immutable props pattern.The change to
constfor props destructuring is appropriate and aligns with the project-wide refactor to enforce immutable prop bindings.src/lib/html/Details.svelte (1)
5-5: LGTM! Consistent immutable props approach.Converting the props destructuring from
lettoconstis correct and maintains consistency with the broader refactoring effort across HTML components.src/lib/html/S.svelte (1)
5-5: LGTM! Proper const destructuring implementation.The change from
lettoconstfor props destructuring follows best practices and is consistent with the project-wide immutable props refactor.src/lib/html/B.svelte (1)
5-5: LGTM! Correct immutable props pattern.The migration to
constfor props destructuring is appropriate and consistent with the broader codebase changes to enforce immutable prop bindings.src/lib/html/Rect.svelte (1)
5-5: LGTM! Follows immutable props best practice.The change from
lettoconstfor props destructuring is correct and aligns with the project-wide refactor to use immutable prop bindings.src/lib/html/Iframe.svelte (1)
5-5: LGTM! Consistent with immutable props refactor.The conversion to
constfor props destructuring is appropriate and maintains consistency with the broader effort to enforce immutable prop bindings across HTML components.src/lib/html/Fediffuselighting.svelte (1)
5-5: LGTM: const destructure with $props() is Svelte 5‑idiomatic.No behavior change; immutability is fine here.
src/lib/html/Legend.svelte (1)
5-5: LGTM: matches Svelte 5 props rune usage.Const destructure is appropriate and keeps the component lean.
src/lib/html/Caption.svelte (1)
5-5: LGTM: safe refactor to const with $props().No functional differences; reads clean.
src/lib/html/Ellipse.svelte (1)
5-5: Confirm prop type covers SVG attributes.You're importing HTMLElementProps while wrapping an SVG tag. Ensure HTMLElementProps includes SVG attribute types (or switch to the SVG variant) so consumers get correct typing.
src/lib/html/Path.svelte (1)
5-5: Confirm prop type covers SVG attributes.Same note as Ellipse: verify HTMLElementProps accounts for SVG props for Path; adjust to SVG props type if not.
src/lib/html/Polygon.svelte (1)
5-5: Confirm prop type covers SVG attributes.Polygon is SVG; please verify the props type includes SVG attributes.
src/lib/html/Femergenode.svelte (1)
5-5: Confirm prop type covers SVG attributes.feMergeNode is SVG; ensure the imported props type isn’t HTML‑only.
README.md (1)
82-82: Nice addition of Hover + Tap example.Looks consistent with the other rows.
src/lib/html/Progress.svelte (1)
5-5: Immutable destructure is fine; confirm no reactive need.Switching to const doesn’t change semantics (destructuring from $props() is non‑reactive either way). If this wrapper ever needs to reflect prop updates at runtime, avoid destructuring and read directly off the $props() object.
src/lib/html/Clippath.svelte (1)
5-5: Consistent with repo‑wide const props; double‑check tag handling.Change is good. As a sanity check, ensure MotionContainer correctly renders this SVG element (clipPath) in the proper namespace when tag="clippath".
src/lib/html/Fecomponenttransfer.svelte (1)
5-5: LGTM; aligns with immutable props pattern.Same note as Clippath: verify MotionContainer maps tag="fecomponenttransfer" to SVG .
src/lib/types.ts (2)
70-74: Underscored param names: good to silence TS unused warnings.
107-112: New MotionProps: onTapStart/onTap/onTapCancel — LGTM.Names match common motion libraries and read well alongside hover callbacks.
src/lib/utils/interaction.spec.ts (7)
21-27: Good edge‑case: animate fallback to initial.
74-87: pointercancel path without baseline covered — nice.
89-100: pointercancel restores baseline — good regression guard.
102-132: Keyboard flow (Enter/blur) covered — great a11y signal.
134-183: Hover reapplication on pointerup looks correct, including transition pass‑through.
185-226: Not‑hovered restore to baseline — clear and necessary.
228-275: Safe path when matches throws — good defensive test.src/lib/html/_MotionContainer.svelte (4)
83-89: Confirm mergeInlineStyles output is HTML‑safe and won’t fight Motion’s runtime stylesWe’re merging initial/animate into the inline style. Verify mergeInlineStyles returns a serialized style string (or acceptable style object for Svelte 5) and that it doesn’t overwrite in‑flight animations (e.g., when runAnimation starts).
If needed, I can add a quick unit to assert string output and precedence rules.
124-125: LGTM: FLIP now uses typed AnimationOptionsThe typed cast on mergedTransition aligns runFlipAnimation’s expectations.
151-166: LGTM: whileTap wiring + hover fallbackPassing onTapStart/onTap/onTapCancel and a hoverFallbackTransition solves the “hover not restored after tap” bug and enables keyboard parity.
Please confirm tests cover: mouse tap→hover restore, keyboard Enter tap→hover restore, and touch devices where hover is not reapplied.
231-239: LGTM: Unified svelte:element pathSingle render path via derivedAttrs reduces drift between void/non‑void branches.
| ### 4.1) Next Focus (next 1–2 weeks) | ||
|
|
||
| - Priority 1 — whileFocus interaction | ||
| - Add `whileFocus` prop with capability parity to `whileHover`/`whileTap`. | ||
| - Tests: utils where needed and `_MotionContainer` behavior (enter/leave focus, nested transition support). | ||
| - Docs: brief API reference and example. | ||
|
|
||
| - Priority 2 — Shared Layout (Prototype) | ||
| - Introduce `layoutId` for same-tree shared layout transitions (translate + scale). | ||
| - Coalesce measurements with RAF; use compositor hints; no overlay/portal in MVP. | ||
| - Tests: card grid reorder and list item move using `layoutId`. | ||
|
|
||
| - Priority 3 — Variants (MVP) | ||
| - Support `variants` object and `initial`/`animate` by variant key. | ||
| - Allow per-property `transition` inside variants. | ||
| - Tests: component-level resolution and overrides; no `exit` yet. | ||
|
|
||
| - Priority 4 — Presence (MVP) | ||
| - Minimal `AnimatePresence`-like wrapper enabling exit animations on unmount for simple lists. | ||
| - SSR/hydration-safe behavior; component tests verifying exit timing. | ||
|
|
||
| - Priority 5 — Testing and docs | ||
| - E2E: Fancy Like Button press-and-hold spawns + un-like stop. | ||
| - Docs site skeleton: Quickstart + API for `motion.<tag>`, `MotionConfig`, `while*` props. | ||
|
|
||
| - Ongoing — Performance hardening | ||
| - Coalesce observers via `requestAnimationFrame` across utilities (done for layout observers); audit other hotspots. | ||
| - Add canary perf test with many animated nodes and frequent layout changes. | ||
|
|
||
| ### 4.2) Shared Layout Plan (post-Variants/Presence) |
There was a problem hiding this comment.
🧹 Nitpick
Roadmap clarity is good; add whileFocus a11y detail.
Call out expected focus sources (Tab/Shift+Tab) and blur/cancel semantics mirroring tap for consistency.
🤖 Prompt for AI Agents
In PRD.md around lines 74 to 103, the whileFocus item lacks accessibility
details: update the Priority 1 whileFocus bullet to explicitly call out expected
focus sources (Tab and Shift+Tab), define blur/cancel semantics that mirror
whileTap (i.e., animate on focus, cancel on blur or when focus is
programmatically removed), and add tests/docs guidance to cover keyboard
navigation (Tab/Shift+Tab), nested transition behavior on focus/blur, and
examples describing cancel behavior for focus loss and programmatic focus
changes.
src/lib/html/Audio.svelte
Outdated
| import type { HTMLElementProps } from '$lib/types.js' | ||
| let { children, ...rest }: HTMLElementProps = $props() | ||
| const { children, ...rest }: HTMLElementProps = $props() |
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
Confirm reactivity of rest after let→const.
This relies on Svelte 5 keeping destructured $props() bindings reactive. Please confirm with docs.
🌐 Web query:
In Svelte 5, does `const { ...rest } = $props()` remain reactive to prop updates (including spread usage), or should we avoid destructuring for full reactivity?
💡 Result:
Short answer: Avoid it if you need full, reliable reactivity.
Why: simple destructuring (especially with rest/spread) can break the live link to incoming props in Svelte 5 — aliases and rest objects may not update in every case. The docs recommend "to get all properties, don't destructure: let props = $props();" and several GitHub issues show bugs/limitations when using const/destructured rest forms (they can stop reflecting updates). (svelte.dev)
Recommendation: if you need every prop to stay fully reactive, either
- don't destructure: let props = $props() and read props.x, or
- use a reactive assignment when destructuring: $: ({ a, b, ...rest } = $props()), and test for edge cases (spreading/rest may still have known issues). (svelte.dev)
Citations:
- 1: https://svelte.dev/docs/svelte/%24props?utm_source=openai
- 2: https://svelte.dev/docs/svelte/%24props?utm_source=openai
Don't destructure $props(); preserve reactivity
Replace
const { children, ...rest }: HTMLElementProps = $props()
with either:
- let props = $props(); use props.children and derive rest from props
- or a reactive destructure: $: ({ children, ...rest } = $props())
Location: src/lib/html/Audio.svelte (line 5)
🤖 Prompt for AI Agents
In src/lib/html/Audio.svelte around line 5, the code destructures $props()
directly which breaks Svelte reactivity; replace the direct destructure with
either: assign let props = $props() and reference props.children and derive rest
from props when needed, or use a reactive assignment so the destructure updates
with changes (e.g. use a $: ({ children, ...rest } = $props()) reactive
statement) and update downstream uses to read from the chosen variables.
| /** Tap lifecycle callbacks */ | ||
| export type MotionOnTapStart = (() => void) | undefined | ||
| export type MotionOnTap = (() => void) | undefined | ||
| export type MotionOnTapCancel = (() => void) | undefined | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick
Tap lifecycle callback types added: OK; consider future payloads.
Current shape (() => void) is fine. If you later need context (pointerType, keyboard), introduce a narrow TapContext payload to avoid leaking raw events.
🤖 Prompt for AI Agents
In src/lib/types.ts around lines 80 to 84, the tap callback signatures currently
use (() => void); introduce a narrow TapContext interface (e.g. pointerType?:
'mouse'|'touch'|'pen'|'keyboard', isPrimary?: boolean, maybe modifier keys) and
update the three types to accept that payload while preserving backward
compatibility by allowing either no-arg or context-arg callbacks (i.e. (() =>
void) | ((ctx: TapContext) => void) | undefined); add the new TapContext export
so callers can type their handlers.
| it('attachWhileTap: animates on down and restores on up, fires callbacks', async () => { | ||
| const el = document.createElement('div') | ||
| animateMock.mockClear() | ||
| const onTapStart = vi.fn() | ||
| const onTap = vi.fn() | ||
| const onTapCancel = vi.fn() | ||
| const cleanup = attachWhileTap( | ||
| el, | ||
| { scale: 0.9, backgroundColor: '#f00' }, | ||
| { scale: 1, backgroundColor: '#111' }, | ||
| { scale: 1.1, backgroundColor: '#000' } | ||
| { scale: 1.1, backgroundColor: '#000' }, | ||
| { onTapStart, onTap, onTapCancel } | ||
| ) | ||
| el.dispatchEvent(new PointerEvent('pointerdown')) | ||
| await Promise.resolve() | ||
| expect(animateMock).toHaveBeenCalled() | ||
| const downCall = animateMock.mock.calls.at(-1) | ||
| expect(downCall?.[1]).toMatchObject({ scale: 0.9, backgroundColor: '#f00' }) | ||
| expect(onTapStart).toHaveBeenCalledTimes(1) | ||
|
|
||
| el.dispatchEvent(new PointerEvent('pointerup')) | ||
| await Promise.resolve() | ||
| const upCall = animateMock.mock.calls.at(-1) | ||
| expect(upCall?.[1]).toMatchObject({ scale: 1.1, backgroundColor: '#000' }) | ||
| expect(onTap).toHaveBeenCalledTimes(1) | ||
| cleanup() | ||
| }) |
There was a problem hiding this comment.
🧹 Nitpick
Happy path + callbacks verified — solid.
Minor: you could also assert call counts to protect against duplicate animations.
🤖 Prompt for AI Agents
In src/lib/utils/interaction.spec.ts around lines 34 to 60, the test currently
verifies the last animate call's arguments but not how many times animate was
invoked; add explicit call-count assertions to guard against duplicate
animations by asserting animateMock was called exactly once immediately after
dispatching the 'pointerdown' (e.g.
expect(animateMock).toHaveBeenCalledTimes(1)) and then asserting the expected
total count after the 'pointerup' (e.g.
expect(animateMock).toHaveBeenCalledTimes(2)); optionally assert no extra calls
after calling cleanup to ensure no leaked listeners.
🧩 Analysis chain
Potential gap: pointerup outside element may be missed without pointer capture.
attachWhileTap listens on el for pointerup; if the pointer is released off‑element, some browsers won’t deliver pointerup to el. Recommend setPointerCapture on pointerdown and releasePointerCapture on up/cancel, and add a test covering “down on el, move out, up off el”.
Proposed change (src/lib/utils/interaction.ts):
- const handlePointerDown = () => {
+ const handlePointerDown = (e: PointerEvent) => {
+ // Ensure we receive the corresponding pointerup/cancel even if the pointer leaves the element
+ try {
+ el.setPointerCapture?.(e.pointerId)
+ } catch {}
callbacks?.onTapStart?.()
animate(el, whileTap as unknown as DOMKeyframesDefinition)
}
- const handlePointerUp = () => {
+ const handlePointerUp = (e: PointerEvent) => {
+ try {
+ el.releasePointerCapture?.(e.pointerId)
+ } catch {}
callbacks?.onTap?.()
if (!whileTap) return
if (reapplyHoverIfActive()) return
if (initial || animateDef) {
const resetRecord = buildTapResetRecord(initial ?? {}, animateDef ?? {}, whileTap ?? {})
if (Object.keys(resetRecord).length > 0) {
animate(el, resetRecord as unknown as DOMKeyframesDefinition)
}
}
}
- const handlePointerCancel = () => {
+ const handlePointerCancel = (e: PointerEvent) => {
+ try {
+ el.releasePointerCapture?.(e.pointerId)
+ } catch {}
callbacks?.onTapCancel?.()
// On cancel, also restore baseline if available
if (initial || animateDef) {
const resetRecord = buildTapResetRecord(initial ?? {}, animateDef ?? {}, whileTap ?? {})
if (Object.keys(resetRecord).length > 0) {
animate(el, resetRecord as unknown as DOMKeyframesDefinition)
}
}
}Add an e2e test:
- hover, pointerdown inside, mouse move outside, mouse up outside → expect hover reapplied if still hovered; otherwise baseline restored.
Also applies to: 74-87, 89-100, 134-183, 185-226, 228-275
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== rg: locate attachWhileTap =="
rg -n --hidden --no-ignore -S 'attachWhileTap' || true
echo
echo "== Print src/lib/utils/interaction.ts (if exists) =="
if [ -f src/lib/utils/interaction.ts ]; then
sed -n '1,400p' src/lib/utils/interaction.ts
else
echo "file not found: src/lib/utils/interaction.ts"
fi
echo
echo "== Print src/lib/utils/interaction.spec.ts (if exists) =="
if [ -f src/lib/utils/interaction.spec.ts ]; then
sed -n '1,400p' src/lib/utils/interaction.spec.ts
else
echo "file not found: src/lib/utils/interaction.spec.ts"
fi
echo
echo "== rg: pointer capture APIs (setPointerCapture / releasePointerCapture) =="
rg -n --hidden --no-ignore -S 'setPointerCapture|releasePointerCapture' || true
echo
echo "== rg: pointer event listeners in interaction.ts =="
rg -n --hidden --no-ignore -S --line-number "pointerdown|pointerup|pointercancel" src/lib/utils/interaction.ts || trueLength of output: 20977
Ensure pointer capture for attachWhileTap so pointerup/pointercancel aren't missed
attachWhileTap currently attaches pointer handlers only to the element; a pointer released off‑element can be missed — call setPointerCapture on pointerdown and releasePointerCapture on pointerup/pointercancel.
File: src/lib/utils/interaction.ts — update handlePointerDown / handlePointerUp / handlePointerCancel (≈ lines 65, 88, 99).
Proposed change (src/lib/utils/interaction.ts):
- const handlePointerDown = () => {
+ const handlePointerDown = (e: PointerEvent) => {
+ // Ensure we receive the corresponding pointerup/cancel even if the pointer leaves the element
+ try {
+ el.setPointerCapture?.(e.pointerId)
+ } catch {}
callbacks?.onTapStart?.()
animate(el, whileTap as unknown as DOMKeyframesDefinition)
}
- const handlePointerUp = () => {
+ const handlePointerUp = (e: PointerEvent) => {
+ try {
+ el.releasePointerCapture?.(e.pointerId)
+ } catch {}
callbacks?.onTap?.()
if (!whileTap) return
if (reapplyHoverIfActive()) return
if (initial || animateDef) {
const resetRecord = buildTapResetRecord(initial ?? {}, animateDef ?? {}, whileTap ?? {})
if (Object.keys(resetRecord).length > 0) {
animate(el, resetRecord as unknown as DOMKeyframesDefinition)
}
}
}
- const handlePointerCancel = () => {
+ const handlePointerCancel = (e: PointerEvent) => {
+ try {
+ el.releasePointerCapture?.(e.pointerId)
+ } catch {}
callbacks?.onTapCancel?.()
// On cancel, also restore baseline if available
if (initial || animateDef) {
const resetRecord = buildTapResetRecord(initial ?? {}, animateDef ?? {}, whileTap ?? {})
if (Object.keys(resetRecord).length > 0) {
animate(el, resetRecord as unknown as DOMKeyframesDefinition)
}
}
}Add/adjust test: simulate pointerdown with a pointerId on the element, move out, then dispatch pointerup with the same pointerId off‑element — assert the up path runs (hover reapply or baseline restore) and callbacks fire as expected.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/lib/utils/interaction.ts around lines 65, 88 and 99, attachWhileTap
currently only listens on the element and can miss pointerup/pointercancel when
the pointer is released off‑element; on pointerdown call
event.target.setPointerCapture(event.pointerId) (or the
element.setPointerCapture) and on pointerup and pointercancel call
element.releasePointerCapture(event.pointerId); ensure the handlers use
event.pointerId consistently so the captured pointer is released correctly;
update the test in src/lib/utils/interaction.spec.ts to create PointerEvent
instances with a pointerId, call el.dispatchEvent(new
PointerEvent('pointerdown', { pointerId: 1 })) then dispatch a pointerup on
document or another element with the same pointerId and assert the up path and
callbacks run (hover/baseline restore) as expected.
| callbacks?.onTapCancel?.() | ||
| // On cancel, also restore baseline if available | ||
| if (initial || animateDef) { | ||
| const resetRecord = buildTapResetRecord(initial ?? {}, animateDef ?? {}, whileTap ?? {}) | ||
| if (Object.keys(resetRecord).length > 0) { | ||
| animate(el, resetRecord as unknown as DOMKeyframesDefinition) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Also reapply hover on cancel; release capture; use transition on reset
Cancellations should return to hover if still hovered, else to baseline with the same fallback timing.
- const handlePointerCancel = () => {
+ const handlePointerCancel = (e: PointerEvent) => {
callbacks?.onTapCancel?.()
- // On cancel, also restore baseline if available
+ if (capturedPointerId != null) {
+ try { el.releasePointerCapture(capturedPointerId) } catch {}
+ capturedPointerId = null
+ }
+ if (reapplyHoverIfActive()) return
if (initial || animateDef) {
const resetRecord = buildTapResetRecord(initial ?? {}, animateDef ?? {}, whileTap ?? {})
if (Object.keys(resetRecord).length > 0) {
- animate(el, resetRecord as unknown as DOMKeyframesDefinition)
+ animate(
+ el,
+ resetRecord as unknown as DOMKeyframesDefinition,
+ callbacks?.hoverFallbackTransition as AnimationOptions
+ )
}
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| callbacks?.onTapCancel?.() | |
| // On cancel, also restore baseline if available | |
| if (initial || animateDef) { | |
| const resetRecord = buildTapResetRecord(initial ?? {}, animateDef ?? {}, whileTap ?? {}) | |
| if (Object.keys(resetRecord).length > 0) { | |
| animate(el, resetRecord as unknown as DOMKeyframesDefinition) | |
| } | |
| } | |
| } | |
| const handlePointerCancel = (e: PointerEvent) => { | |
| callbacks?.onTapCancel?.() | |
| if (capturedPointerId != null) { | |
| try { el.releasePointerCapture(capturedPointerId) } catch {} | |
| capturedPointerId = null | |
| } | |
| if (reapplyHoverIfActive()) return | |
| if (initial || animateDef) { | |
| const resetRecord = buildTapResetRecord(initial ?? {}, animateDef ?? {}, whileTap ?? {}) | |
| if (Object.keys(resetRecord).length > 0) { | |
| animate( | |
| el, | |
| resetRecord as unknown as DOMKeyframesDefinition, | |
| callbacks?.hoverFallbackTransition as AnimationOptions | |
| ) | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/lib/utils/interaction.ts around lines 100 to 108, the cancel path should
(1) release pointer capture, (2) reapply hover styles if the pointer is still
over the element, otherwise revert to baseline, and (3) use the same fallback
transition timing when resetting. Update the cancel block to: call
callbacks?.onTapCancel(), release any pointer capture on the element, determine
the target state (if the element is currently hovered reapply the hover
definition, else use initial/baseline), build the resetRecord from that target
state combined with animateDef and whileTap fallbacks, and call animate(el,
resetRecord) so the reset uses the same transition timing rather than an
immediate snap.
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/utils/a11y.spec.ts (1)
1-34: Add cases for contenteditable="false" and disabled controls.
Current utility treats any contenteditable as focusable and ignores disabled; add tests to lock behavior.describe('utils/a11y - isNativelyFocusable', () => { + it('contenteditable="false" is not focusable', () => { + expect(isNativelyFocusable('div', { contenteditable: 'false' })).toBe(false) + }) + it('disabled controls are not focusable', () => { + expect(isNativelyFocusable('button', { disabled: true })).toBe(false) + expect(isNativelyFocusable('input', { disabled: '' })).toBe(false) + }) })
♻️ Duplicate comments (13)
eslint.config.mjs (4)
33-44: Prettier conflicts resolved — good call turning formatting rules off.Disabling these rules eliminates churn with eslint-config-prettier and svelte/prettier.
53-60: Correct: rely on @typescript-eslint/no-unused-vars and disable core.This avoids duplicate reports and honors TS semantics.
30-31: Replace core no-duplicate-imports with import/no-duplicates (TS-aware) or keep it off.Core rule misfires with TypeScript’s separate type/value imports.
- 'no-duplicate-imports': ['error'], + // Use the TS-aware variant from eslint-plugin-import + 'no-duplicate-imports': 'off', + 'import/no-duplicates': ['error'],Also ensure the plugin is wired:
languageOptions: { globals: { ...globals.browser, ...globals.node } - }, + }, + plugins: { import: importPlugin },And add the import at the top of the file:
import importPlugin from 'eslint-plugin-import'Run this to verify the plugin is present in devDependencies:
#!/bin/bash fd -a package.json | xargs -I{} rg -n "eslint-plugin-import|eslint-import-resolver-typescript" {}
29-29: Turn off no-whitespace-before-property to fully defer formatting to Prettier.This is purely formatting and can reintroduce churn.
- 'no-whitespace-before-property': ['warn'], + 'no-whitespace-before-property': 'off',src/lib/utils/layout.spec.ts (2)
176-256: rAF throttle + cleanup looks correct.
Nice use of spies and global stubs; the size guard prevents false positives.
89-107: Also assert transformOrigin for scale-only path.
runFlipAnimation sets transformOrigin='0 0'; add an assertion to catch regressions.// Inline style should have only scale pre-applied expect(el.style.transform).toBe('scale(1.2, 1.1)') + expect(el.style.transformOrigin).toBe('0 0')src/lib/utils/interaction.spec.ts (1)
311-330: Double keydown guard simplified — nice.
Switched to direct .length assertion as previously suggested.src/lib/utils/interaction.ts (4)
66-85: Pointer capture + cleanup looks good.This addresses stuck pressed states and lost pointerup; release is handled on up/cancel.
61-85: whileTap.transition is ignored; split whileTap and reuse its transition on press and reset. Also reapply hover on cancel.Currently animate(...) on press/reset omits options, so any transition inside whileTap is dropped, and cancel never reapplies hover. Apply the following:
@@ - if (!whileTap) return () => {} + if (!whileTap) return () => {} + // Extract keyframes/transition from whileTap so we can respect author options + const { keyframes: tapKeyframes, transition: tapTransition } = splitHoverDefinition( + whileTap as Record<string, unknown> + ) @@ - callbacks?.onTapStart?.() - animate(el, whileTap as unknown as DOMKeyframesDefinition) + callbacks?.onTapStart?.() + animate( + el, + tapKeyframes as unknown as DOMKeyframesDefinition, + (tapTransition ?? callbacks?.hoverFallbackTransition) as AnimationOptions + ) @@ - if (initial || animateDef) { - const resetRecord = buildTapResetRecord(initial ?? {}, animateDef ?? {}, whileTap ?? {}) - if (Object.keys(resetRecord).length > 0) { - animate(el, resetRecord as unknown as DOMKeyframesDefinition) - } - } + if (initial || animateDef) { + const resetRecord = buildTapResetRecord( + initial ?? {}, + animateDef ?? {}, + (tapKeyframes ?? {}) as Record<string, unknown> + ) + if (Object.keys(resetRecord).length > 0) { + animate( + el, + resetRecord as unknown as DOMKeyframesDefinition, + (tapTransition ?? callbacks?.hoverFallbackTransition) as AnimationOptions + ) + } + } @@ - callbacks?.onTapCancel?.() - // On cancel, also restore baseline if available - if (initial || animateDef) { - const resetRecord = buildTapResetRecord(initial ?? {}, animateDef ?? {}, whileTap ?? {}) - if (Object.keys(resetRecord).length > 0) { - animate(el, resetRecord as unknown as DOMKeyframesDefinition) - } - } + callbacks?.onTapCancel?.() + if (reapplyHoverIfActive()) return + // On cancel, restore baseline if available + if (initial || animateDef) { + const resetRecord = buildTapResetRecord( + initial ?? {}, + animateDef ?? {}, + (tapKeyframes ?? {}) as Record<string, unknown> + ) + if (Object.keys(resetRecord).length > 0) { + animate( + el, + resetRecord as unknown as DOMKeyframesDefinition, + (tapTransition ?? callbacks?.hoverFallbackTransition) as AnimationOptions + ) + } + } @@ - callbacks?.onTapStart?.() - animate(el, whileTap as unknown as DOMKeyframesDefinition) + callbacks?.onTapStart?.() + animate( + el, + tapKeyframes as unknown as DOMKeyframesDefinition, + (tapTransition ?? callbacks?.hoverFallbackTransition) as AnimationOptions + ) @@ - if (initial || animateDef) { - const resetRecord = buildTapResetRecord(initial ?? {}, animateDef ?? {}, whileTap ?? {}) - if (Object.keys(resetRecord).length > 0) { - animate(el, resetRecord as unknown as DOMKeyframesDefinition) - } - } + if (initial || animateDef) { + const resetRecord = buildTapResetRecord( + initial ?? {}, + animateDef ?? {}, + (tapKeyframes ?? {}) as Record<string, unknown> + ) + if (Object.keys(resetRecord).length > 0) { + animate( + el, + resetRecord as unknown as DOMKeyframesDefinition, + (tapTransition ?? callbacks?.hoverFallbackTransition) as AnimationOptions + ) + } + } @@ - callbacks?.onTapCancel?.() + callbacks?.onTapCancel?.() if (initial || animateDef) { - const resetRecord = buildTapResetRecord(initial ?? {}, animateDef ?? {}, whileTap ?? {}) + const resetRecord = buildTapResetRecord( + initial ?? {}, + animateDef ?? {}, + (tapKeyframes ?? {}) as Record<string, unknown> + ) if (Object.keys(resetRecord).length > 0) { - animate(el, resetRecord as unknown as DOMKeyframesDefinition) + animate( + el, + resetRecord as unknown as DOMKeyframesDefinition, + (tapTransition ?? callbacks?.hoverFallbackTransition) as AnimationOptions + ) } }Also applies to: 105-128, 129-151, 153-161, 163-189
153-161: Broaden Space-key detection for cross-browser parity.Include legacy 'Spacebar' and e.code === 'Space'; preventDefault for all Space variants.
- const handleKeyDown = (e: KeyboardEvent) => { - if (!(e.key === 'Enter' || e.key === ' ' || e.key === 'Space')) return - // Prevent page scroll/activation for Space - if (e.key === ' ' || e.key === 'Space') e.preventDefault?.() + const handleKeyDown = (e: KeyboardEvent) => { + const isSpace = + e.key === ' ' || e.key === 'Space' || e.key === 'Spacebar' || e.code === 'Space' + const isActivateKey = e.key === 'Enter' || isSpace + if (!isActivateKey) return + if (isSpace) e.preventDefault?.()
163-177: Mirror Space handling on keyup.Match keydown parity and prevent default for Space variants.
- const handleKeyUp = (e: KeyboardEvent) => { - if (!(e.key === 'Enter' || e.key === ' ' || e.key === 'Space')) return - // Prevent page scroll/activation for Space - if (e.key === ' ' || e.key === 'Space') e.preventDefault?.() + const handleKeyUp = (e: KeyboardEvent) => { + const isSpace = + e.key === ' ' || e.key === 'Space' || e.key === 'Spacebar' || e.code === 'Space' + const isActivateKey = e.key === 'Enter' || isSpace + if (!isActivateKey) return + if (isSpace) e.preventDefault?.()src/lib/html/_MotionContainer.svelte (2)
65-89: Tabindex gating respects native focusability.Prevents redundant tabindex on inherently focusable controls. Nice.
92-100: transitionAmimate → transitionAnimate rename and typings look good.Corrects the typo and aligns types with AnimationOptions/DOMKeyframesDefinition.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
PRD.md(3 hunks)README.md(2 hunks)e2e/motion/hover-and-tap.test.ts(1 hunks)eslint.config.mjs(1 hunks)src/lib/html/_MotionContainer.spec.ts(1 hunks)src/lib/html/_MotionContainer.svelte(8 hunks)src/lib/utils/a11y.spec.ts(1 hunks)src/lib/utils/a11y.ts(1 hunks)src/lib/utils/interaction.spec.ts(2 hunks)src/lib/utils/interaction.ts(3 hunks)src/lib/utils/layout.spec.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.svelte
📄 CodeRabbit inference engine (.cursor/rules/svelte.mdc)
**/*.svelte: Use Svelte 5 syntax in all Svelte components
When adding comments to Svelte components, follow the guidance at https://svelte.dev/docs/svelte/faq#How-do-I-document-my-components
Files:
src/lib/html/_MotionContainer.svelte
🧬 Code graph analysis (4)
src/lib/utils/a11y.spec.ts (1)
src/lib/utils/a11y.ts (1)
isNativelyFocusable(3-22)
src/lib/utils/layout.spec.ts (1)
src/lib/utils/layout.ts (3)
computeFlipTransforms(31-51)runFlipAnimation(63-95)observeLayoutChanges(120-157)
src/lib/utils/interaction.ts (1)
src/lib/utils/hover.ts (2)
isHoverCapable(13-21)splitHoverDefinition(29-37)
src/lib/utils/interaction.spec.ts (1)
src/lib/utils/interaction.ts (2)
buildTapResetRecord(17-33)attachWhileTap(48-210)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e-tests (2/2, 2-2)
- GitHub Check: e2e-tests (1/2, 1-2)
🔇 Additional comments (20)
eslint.config.mjs (1)
45-52: no-unused-expressions config LGTM.Allows common short‑circuit/ternary patterns used in event handlers without suppressing real issues.
src/lib/utils/layout.spec.ts (2)
43-49: LGTM: threshold guard covers translate/scale noise.
Good assertions for sub-threshold dx/dy; no change needed.
258-317: Fallback path verified well.
setTimeout/clearTimeout tokens are asserted and globals restored; solid.src/lib/html/_MotionContainer.spec.ts (1)
41-53: Correct: no tabindex on natively focusable anchor.
Behavior matches a11y expectations.README.md (2)
82-82: Examples table: new Hover + Tap entry looks good.
Route and REPL linkage consistent with other rows.
115-121: Tap callbacks + keyboard a11y documented correctly.
Docs match implementation (Enter/Space handling, tabindex defaulting).e2e/motion/hover-and-tap.test.ts (2)
12-46: Robust readScale implementation.
Handles matrix and matrix3d with hypot; good resilience to rotation/skew.
49-64: End-to-end flow is stable and readable.
Hover before mouse.down ensures pointer is over target; expect.poll guards timing.PRD.md (2)
16-16: Tap callbacks + keyboard a11y clarified.
Enter/Space semantics and preventDefault note look correct.
74-103: Roadmap updates are clear and actionable.
Focus/variants/presence milestones are scoped well.src/lib/utils/interaction.spec.ts (7)
34-67: Happy path: down → whileTap, off-element up → baseline; callbacks asserted.
Good use of pointerId and document-level up to simulate capture/off‑element release.
81-95: Cancel path without baseline covered.
onTapCancel asserted; no extra animate — good.
96-108: Cancel path with baseline restore covered.
Reset to animate over initial verified.
109-139: Keyboard Enter lifecycle verified.
Start/up/cancel on blur covered; solid.
168-217: Hover re-apply path validated (transition respected).
Media query stub + matches(':hover') approach works.
219-260: Not-hovered reset path validated.
Baseline restoration asserted.
262-309: Safe catch path when matches throws.
Good guard coverage.src/lib/utils/interaction.ts (2)
17-33: Reset record logic is correct and minimal.Overlapping-key restoration matches the docstring and avoids unintended style changes.
86-104: Hover reapplication helper is solid.Good device gating and safe :hover check; splitHoverDefinition usage is correct.
src/lib/html/_MotionContainer.svelte (1)
153-164: attachWhileTap wiring is correct.Passing hoverDef and hoverFallbackTransition enables hover reapply and consistent timing.
| rules: { | ||
| 'dot-location': ['warn', 'property'], | ||
| 'guard-for-in': ['warn'], | ||
| camelcase: ['error'], |
There was a problem hiding this comment.
🧹 Nitpick
Prefer @typescript-eslint/naming-convention over core camelcase.
TS rule is more flexible and avoids false positives on external identifiers.
- camelcase: ['error'],
+ camelcase: 'off',
+ '@typescript-eslint/naming-convention': [
+ 'error',
+ { selector: 'variableLike', format: ['camelCase', 'UPPER_CASE', 'PascalCase'] },
+ { selector: 'typeLike', format: ['PascalCase'] }
+ ],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| camelcase: ['error'], | |
| camelcase: 'off', | |
| '@typescript-eslint/naming-convention': [ | |
| 'error', | |
| { selector: 'variableLike', format: ['camelCase', 'UPPER_CASE', 'PascalCase'] }, | |
| { selector: 'typeLike', format: ['PascalCase'] } | |
| ], |
🤖 Prompt for AI Agents
In eslint.config.mjs around line 27, the config uses the core "camelcase" rule
which is less flexible for TypeScript; replace it with an equivalent
@typescript-eslint/naming-convention rule. Remove or disable "camelcase:
['error']" and add a @typescript-eslint/naming-convention entry that enforces
camelCase for variables, functions, and parameters (allowing leading underscore
for unused vars), permits PascalCase for types/interfaces, and does not flag
quoted or external identifiers; ensure @typescript-eslint/eslint-plugin is
installed and referenced so the rule is applied.
| 'no-whitespace-before-property': ['warn'], | ||
| 'no-duplicate-imports': ['error'], | ||
| 'no-var': ['error'], | ||
| 'prefer-const': ['off'], |
There was a problem hiding this comment.
🧹 Nitpick
Reconsider prefer-const = off.
If not intentional, set to warn to catch accidental lets.
- 'prefer-const': 'off',
+ 'prefer-const': ['warn', { destructuring: 'all' }],Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In eslint.config.mjs around line 32, the rule 'prefer-const' is currently turned
off which allows accidental use of let; change its value to 'warn' (i.e.,
'prefer-const': ['warn']) unless you intentionally want it disabled, then
run/verify ESLint to ensure existing violations are surfaced and fix or suppress
them as needed.
| it('adds tabindex=0 for non-focusable elements with whileTap', async () => { | ||
| /* trunk-ignore(eslint/@typescript-eslint/no-explicit-any) */ | ||
| const { container } = render(MotionContainer as unknown as any, { | ||
| props: { tag: 'div', whileTap: { scale: 0.95 } } | ||
| }) | ||
| await flushTimers() | ||
| const el = container.firstElementChild as HTMLElement | ||
| expect(el.getAttribute('tabindex')).toBe('0') | ||
| }) |
There was a problem hiding this comment.
🧹 Nitpick
Add afterEach to restore timers/globals to avoid test leakage.
You stub RAF and use fake timers in beforeEach; ensure cleanup per test.
beforeEach(() => {
animateMock.mockClear()
vi.useFakeTimers()
vi.stubGlobal('requestAnimationFrame', (cb: FrameRequestCallback) => {
setTimeout(() => cb(0), 0)
return 0 as unknown as number
})
})
+import { afterEach } from 'vitest'
+afterEach(() => {
+ vi.useRealTimers()
+ vi.unstubAllGlobals()
+})Also consider a test to confirm pre‑existing tabindex is preserved (not overwritten) when whileTap is set.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/lib/html/_MotionContainer.spec.ts around lines 55 to 63, the tests stub
requestAnimationFrame and use fake timers in beforeEach but lack an afterEach to
restore timers and any global stubs; add an afterEach that restores real timers
(jest.useRealTimers or equivalent) and restores any RAF/global stubs to avoid
leakage between tests. Also add a small additional test ensuring that when an
element already has a tabindex, rendering with whileTap does not overwrite that
existing tabindex (i.e., pre-existing tabindex is preserved).
| if ((attrs as Record<string, unknown>).tabindex != null) return true | ||
| if ((attrs as Record<string, unknown>).tabIndex != null) return true | ||
| if ((attrs as Record<string, unknown>).contenteditable != null) return true | ||
| switch (tag) { | ||
| case 'a': | ||
| return Boolean((attrs as unknown as { href?: string }).href) | ||
| case 'button': |
There was a problem hiding this comment.
Fix contenteditable handling (respect "false").
Treat only truthy/true contenteditable as focusable.
- if ((attrs as Record<string, unknown>).contenteditable != null) return true
+ {
+ const ce = (attrs as Record<string, unknown>).contenteditable
+ if (ce != null) {
+ const v = String(ce).toLowerCase()
+ if (v !== 'false') return true
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ((attrs as Record<string, unknown>).tabindex != null) return true | |
| if ((attrs as Record<string, unknown>).tabIndex != null) return true | |
| if ((attrs as Record<string, unknown>).contenteditable != null) return true | |
| switch (tag) { | |
| case 'a': | |
| return Boolean((attrs as unknown as { href?: string }).href) | |
| case 'button': | |
| if ((attrs as Record<string, unknown>).tabindex != null) return true | |
| if ((attrs as Record<string, unknown>).tabIndex != null) return true | |
| { | |
| const ce = (attrs as Record<string, unknown>).contenteditable | |
| if (ce != null) { | |
| const v = String(ce).toLowerCase() | |
| if (v !== 'false') return true | |
| } | |
| } | |
| switch (tag) { | |
| case 'a': | |
| return Boolean((attrs as unknown as { href?: string }).href) | |
| case 'button': |
🤖 Prompt for AI Agents
In src/lib/utils/a11y.ts around lines 7 to 13, the current check returns true
whenever contenteditable is present, but it should treat only truthy
contenteditable as focusable; change the contenteditable check so it reads the
attribute value, and return true only if the value is not null/undefined and is
not the literal false (i.e., handle boolean false and string "false"
case-insensitively); otherwise treat it as not focusable and continue to the
tag-specific checks.
| case 'button': | ||
| case 'input': | ||
| case 'select': | ||
| case 'textarea': | ||
| case 'summary': | ||
| return true | ||
| default: | ||
| return false | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Account for disabled controls not being focusable.
Native disabled form controls aren’t focusable; reflect that.
switch (tag) {
case 'a':
return Boolean((attrs as unknown as { href?: string }).href)
- case 'button':
- case 'input':
+ case 'button':
+ case 'input':
case 'select':
case 'textarea':
case 'summary':
- return true
+ return !(('disabled' in (attrs as Record<string, unknown>)) && (attrs as Record<string, unknown>).disabled !== false)
default:
return false
}Add corresponding tests (see a11y.spec.ts suggestion) and confirm MotionContainer tabindex logic still behaves as intended.
🏁 Script executed:
#!/bin/bash
# Locate the a11y utility file and print its first 200 lines to verify focusability logic
file=$(fd a11y.ts -t f)
echo "=== File path: $file ==="
sed -n '1,200p' "$file"Length of output: 754
Disabled native controls must be treated as not focusable — disabled must override tabindex.
The function returns true early for tabindex/tabIndex/contenteditable, so patching only the switch won’t stop disabled controls (with tabindex) from being reported focusable.
File: src/lib/utils/a11y.ts (isNativelyFocusable)
- Add an early check before the tabindex/tabIndex/contenteditable returns: if ('disabled' in attrs) && (attrs.disabled !== false) then return false for native form controls (button, input, select, textarea).
- Add tests in a11y.spec.ts covering: disabled controls (with and without tabindex), tabindex on non-disabled elements, and verify MotionContainer tabindex behavior remains correct.
🤖 Prompt for AI Agents
In src/lib/utils/a11y.ts around lines 13 to 21, isNativelyFocusable currently
returns true early for elements with tabindex/tabIndex/contenteditable which
allows disabled native form controls to be reported focusable; add an early
guard before those checks: if the attrs object has a 'disabled' property and
attrs.disabled !== false, then for native form control tag names
('button','input','select','textarea','summary' as applicable) return false
(disabled must override tabindex), and keep the existing logic for non-disabled
elements; also add unit tests in src/lib/utils/a11y.spec.ts to cover disabled
controls with and without tabindex, tabindex on non-disabled elements, and
ensure MotionContainer tabindex behavior remains unchanged.
| it('attachWhileTap: keyboard Space behaves like Enter (prevents scroll)', async () => { | ||
| const el = document.createElement('div') | ||
| animateMock.mockClear() | ||
| const onTapStart = vi.fn() | ||
| const onTap = vi.fn() | ||
| const preventDefault = vi.fn() | ||
| const cleanup = attachWhileTap( | ||
| el, | ||
| { scale: 0.9 }, | ||
| { scale: 1 }, | ||
| { scale: 1.1 }, | ||
| { onTapStart, onTap } | ||
| ) | ||
|
|
||
| el.dispatchEvent(new KeyboardEvent('keydown', { key: ' ', cancelable: true })) | ||
| // Manually call preventDefault as KeyboardEvent in jsdom may not call through | ||
| preventDefault() | ||
| await Promise.resolve() | ||
| expect(onTapStart).toHaveBeenCalledTimes(1) | ||
|
|
||
| el.dispatchEvent(new KeyboardEvent('keyup', { key: ' ', cancelable: true })) | ||
| preventDefault() | ||
| await Promise.resolve() | ||
| expect(onTap).toHaveBeenCalledTimes(1) | ||
| cleanup() | ||
| }) |
There was a problem hiding this comment.
🧹 Nitpick
Assert Space defaultPrevented to prove scroll suppression.
Current test calls a standalone preventDefault; assert on the dispatched event instead.
- const preventDefault = vi.fn()
- const cleanup = attachWhileTap(
+ const cleanup = attachWhileTap(
el,
{ scale: 0.9 },
{ scale: 1 },
{ scale: 1.1 },
{ onTapStart, onTap }
)
-
- el.dispatchEvent(new KeyboardEvent('keydown', { key: ' ', cancelable: true }))
- // Manually call preventDefault as KeyboardEvent in jsdom may not call through
- preventDefault()
+ const kd = new KeyboardEvent('keydown', { key: ' ', cancelable: true, bubbles: true })
+ el.dispatchEvent(kd)
await Promise.resolve()
expect(onTapStart).toHaveBeenCalledTimes(1)
+ expect(kd.defaultPrevented).toBe(true)
- el.dispatchEvent(new KeyboardEvent('keyup', { key: ' ', cancelable: true }))
- preventDefault()
+ const ku = new KeyboardEvent('keyup', { key: ' ', cancelable: true, bubbles: true })
+ el.dispatchEvent(ku)
await Promise.resolve()
expect(onTap).toHaveBeenCalledTimes(1)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('attachWhileTap: keyboard Space behaves like Enter (prevents scroll)', async () => { | |
| const el = document.createElement('div') | |
| animateMock.mockClear() | |
| const onTapStart = vi.fn() | |
| const onTap = vi.fn() | |
| const preventDefault = vi.fn() | |
| const cleanup = attachWhileTap( | |
| el, | |
| { scale: 0.9 }, | |
| { scale: 1 }, | |
| { scale: 1.1 }, | |
| { onTapStart, onTap } | |
| ) | |
| el.dispatchEvent(new KeyboardEvent('keydown', { key: ' ', cancelable: true })) | |
| // Manually call preventDefault as KeyboardEvent in jsdom may not call through | |
| preventDefault() | |
| await Promise.resolve() | |
| expect(onTapStart).toHaveBeenCalledTimes(1) | |
| el.dispatchEvent(new KeyboardEvent('keyup', { key: ' ', cancelable: true })) | |
| preventDefault() | |
| await Promise.resolve() | |
| expect(onTap).toHaveBeenCalledTimes(1) | |
| cleanup() | |
| }) | |
| it('attachWhileTap: keyboard Space behaves like Enter (prevents scroll)', async () => { | |
| const el = document.createElement('div') | |
| animateMock.mockClear() | |
| const onTapStart = vi.fn() | |
| const onTap = vi.fn() | |
| const cleanup = attachWhileTap( | |
| el, | |
| { scale: 0.9 }, | |
| { scale: 1 }, | |
| { scale: 1.1 }, | |
| { onTapStart, onTap } | |
| ) | |
| const kd = new KeyboardEvent('keydown', { key: ' ', cancelable: true, bubbles: true }) | |
| el.dispatchEvent(kd) | |
| await Promise.resolve() | |
| expect(onTapStart).toHaveBeenCalledTimes(1) | |
| expect(kd.defaultPrevented).toBe(true) | |
| const ku = new KeyboardEvent('keyup', { key: ' ', cancelable: true, bubbles: true }) | |
| el.dispatchEvent(ku) | |
| await Promise.resolve() | |
| expect(onTap).toHaveBeenCalledTimes(1) | |
| cleanup() | |
| }) |
🤖 Prompt for AI Agents
In src/lib/utils/interaction.spec.ts around lines 141 to 166, the test manually
calls a standalone preventDefault() function instead of asserting that the
dispatched KeyboardEvent had defaultPrevented set (to prove scrolling was
suppressed). Change the test to create and store the KeyboardEvent objects (e.g.
const keydown = new KeyboardEvent(...); el.dispatchEvent(keydown);) and assert
expect(keydown.defaultPrevented).toBe(true) (and similarly for the keyup event)
instead of calling the standalone preventDefault(), then remove the manual
preventDefault() calls.
| const handlePointerDown = (event: PointerEvent) => { | ||
| // Capture pointer so we receive up/cancel even if pointer leaves the element | ||
| if (typeof event.pointerId === 'number') { | ||
| try { | ||
| if ('setPointerCapture' in el) { | ||
| el.setPointerCapture(event.pointerId) | ||
| } | ||
| } catch { | ||
| // noop if not supported | ||
| } | ||
| activePointerId = event.pointerId |
There was a problem hiding this comment.
🧹 Nitpick
Guard against multi-touch reentry.
If a second finger taps while activePointerId is set, state can flap. Ignore additional pointerdowns until release.
const handlePointerDown = (event: PointerEvent) => {
+ if (activePointerId !== null) return📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handlePointerDown = (event: PointerEvent) => { | |
| // Capture pointer so we receive up/cancel even if pointer leaves the element | |
| if (typeof event.pointerId === 'number') { | |
| try { | |
| if ('setPointerCapture' in el) { | |
| el.setPointerCapture(event.pointerId) | |
| } | |
| } catch { | |
| // noop if not supported | |
| } | |
| activePointerId = event.pointerId | |
| const handlePointerDown = (event: PointerEvent) => { | |
| if (activePointerId !== null) return | |
| // Capture pointer so we receive up/cancel even if pointer leaves the element | |
| if (typeof event.pointerId === 'number') { | |
| try { | |
| if ('setPointerCapture' in el) { | |
| el.setPointerCapture(event.pointerId) | |
| } | |
| } catch { | |
| // noop if not supported | |
| } | |
| activePointerId = event.pointerId |
🤖 Prompt for AI Agents
In src/lib/utils/interaction.ts around lines 66 to 76, the pointerdown handler
does not guard against multi-touch reentry which allows a second pointerdown to
flip state; update handlePointerDown to ignore additional pointerdown events
when activePointerId is already set (return early if activePointerId != null),
only proceed to setPointerCapture and assign activePointerId when no active
pointer is present (or alternatively only accept event.isPrimary), and keep
existing try/catch around setPointerCapture; this prevents state flap until the
active pointer is released.
| window.addEventListener('pointerup', handlePointerUp as EventListener) | ||
| window.addEventListener('pointercancel', handlePointerCancel as EventListener) | ||
| document.addEventListener('pointerup', handlePointerUp as EventListener) | ||
| document.addEventListener('pointercancel', handlePointerCancel as EventListener) |
There was a problem hiding this comment.
🧹 Nitpick
Window + document listeners are redundant; prefer one (or only when capture fails).
Pointer capture already routes events; keep either window or document, and add them only if setPointerCapture threw.
- window.addEventListener('pointerup', handlePointerUp as EventListener)
- window.addEventListener('pointercancel', handlePointerCancel as EventListener)
- document.addEventListener('pointerup', handlePointerUp as EventListener)
- document.addEventListener('pointercancel', handlePointerCancel as EventListener)
+ // Fallback for browsers without capture support
+ window.addEventListener('pointerup', handlePointerUp as EventListener)
+ window.addEventListener('pointercancel', handlePointerCancel as EventListener)
@@
- window.removeEventListener('pointerup', handlePointerUp as EventListener)
- window.removeEventListener('pointercancel', handlePointerCancel as EventListener)
- document.removeEventListener('pointerup', handlePointerUp as EventListener)
- document.removeEventListener('pointercancel', handlePointerCancel as EventListener)
+ window.removeEventListener('pointerup', handlePointerUp as EventListener)
+ window.removeEventListener('pointercancel', handlePointerCancel as EventListener)
@@
- window.removeEventListener('pointerup', handlePointerUp as EventListener)
- window.removeEventListener('pointercancel', handlePointerCancel as EventListener)
- document.removeEventListener('pointerup', handlePointerUp as EventListener)
- document.removeEventListener('pointercancel', handlePointerCancel as EventListener)
+ window.removeEventListener('pointerup', handlePointerUp as EventListener)
+ window.removeEventListener('pointercancel', handlePointerCancel as EventListener)Also applies to: 114-117, 138-141, 202-205
🤖 Prompt for AI Agents
In src/lib/utils/interaction.ts around lines 78-81 (and similarly at 114-117,
138-141, 202-205), add/remove logic so you don't always register both window and
document pointer listeners; instead rely on pointer capture routing and only
fall back to adding the extra document/window listeners when setPointerCapture
throws or capture isn't supported. Modify the attach logic to try
setPointerCapture on the target and, if that fails (catch the error or detect
unsupported), then add the secondary document/window listeners; ensure you
remove those fallback listeners in the corresponding cleanup path so you don't
keep redundant handlers.
Bug: Tapping on an item that has onhover did not go back to the hover state after tap
Summary by CodeRabbit
New Features
Documentation
Tests
Chores