Skip to content

Follow-ups from PR #45 review: motion + marketing widget nits #58

@prakashUXtech

Description

@prakashUXtech

Five non-blocking items the #45 review surfaced. None blocked the merge; grouping them here so they don't get lost. Ordered roughly by impact.

1. Marketing widgets render href straight from spec props (no scheme check)
Navbar.svelte:34,37, Footer.svelte:25, Cta.svelte:20, LogoCloud.svelte:23 all emit <a href={link.href}> from spec-driven values, so a javascript: string would pass through. The authoring pipeline is trusted today, so this is low-risk, but the schema for these fields should constrain to http/https/#-anchors and the manifest entries should say so.

2. with-motion.ts uses cssText += where direct property writes are safer
with-motion.ts:223 (enter initial frame) and :249 (inView arming) do node.style.cssText += .... If anything else writes cssText between those points, the concatenation serializes the whole computed style and can stomp other properties. applyScrollChannel already does the right thing with direct node.style.transform = ... writes; these two paths should match it.

3. Unused loadInView import
with-motion.ts:319 keeps void loadInView; as a placeholder for a future inView-spring path. It trips a lint warning. Either drop it or add an eslint-disable note before 0.5.0 ships as a consumer dependency.

4. wireScroll IntersectionObserver fallback binds a global scroll listener
with-motion.ts:406-410: with no IO, the scroll listener is bound on window and never unobserved while the element is off-screen. destroy() does remove it, so it's not a persistent leak, just a hot path per scrolling node in old browsers. IO is universal in our prerender targets, so this is minor.

5. event-dispatcher.ts:477 getElementById fallback escapes instance scope
The primary lookup uses root.querySelector scoped to the Ripple instance subtree; the document.getElementById fallback can match a node in a sibling Ripple instance on the same page, undermining the isolation note in Ripple.svelte:117-123. Dropping the fallback and relying on scoped querySelector with CSS.escape (the catch already nulls on failure) is cleaner.

Context: surfaced while reviewing #45 against the new SSR/prerender path (ripple is now prerendered by paw-sites).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions