Skip to content

Add update trigger to scroll-reveal action for dynamic elements#55

Merged
Sparkier merged 2 commits into
mainfrom
jules/feat/scroll-reveal-manual-update-js1-aad187b8-d56c-427e-a9cc-b763f1ac38a9
Jul 16, 2026
Merged

Add update trigger to scroll-reveal action for dynamic elements#55
Sparkier merged 2 commits into
mainfrom
jules/feat/scroll-reveal-manual-update-js1-aad187b8-d56c-427e-a9cc-b763f1ac38a9

Conversation

@google-labs-jules

Copy link
Copy Markdown
Contributor

Why This Change is Necessary

Previously, dynamic elements rendered inside conditional template blocks or added post-mount remained permanently hidden. This occurred because our custom scroll-reveal Svelte action only queried and observed child elements during its initial instantiation (mount).

To work around this limitation and ensure complete content visibility when exporting or printing the CV page as a PDF, we had a fragile, manual DOM-query workaround in src/routes/cv/+page.svelte. This workaround directly manipulated CSS visibility classes, bypassing Svelte's reactive paradigm.

This PR introduces an explicit update hook to the scroll-reveal action, allowing templates to trigger a re-query of the DOM whenever specific page states change. This resolves the hidden-content bug cleanly, retains high performance, and allows us to remove the brittle CV page workaround.


Key Decisions & Rationale

  • Explicit State Triggers over Continuous Polling: To avoid performance overhead and layout thrashing, we rejected automatic polling or heavy MutationObserver implementations. Instead, the reveal action accepts an explicit parameter object. It only re-evaluates children when Svelte signals a state change via the action's update method.
  • Idempotent Updates: The updated query logic specifically registers newly discovered .reveal elements with the IntersectionObserver while ignoring elements that have already been revealed, preventing double-animation or layout disruptions.
  • Scope Preservation: Changes are localized entirely within reveal.ts and the pages that utilize dynamic rendering (like the CV page), avoiding the need to refactor every single page binding in the codebase.

What Was Changed

1. Scroll-Reveal Action (src/lib/actions/reveal.ts)

  • Updated the Svelte action signature to accept a configuration parameter (e.g., use:reveal={{ trigger }}).
  • Implemented the update lifecycle method. When the trigger dependency changes, the action re-queries the DOM for target .reveal elements.
  • Registered new elements with the active IntersectionObserver while ensuring already-visible elements remain unaffected.
  • Ensured clean unmounting logic via observer.disconnect() to prevent memory leaks.

2. CV Page Refactoring (src/routes/cv/+page.svelte)

  • Removed the legacy, fragile DOM-query workaround (container.parentElement?.querySelectorAll('.reveal') ...) used during PDF exports.
  • Bound the showLinks state dependency directly to the reveal wrapper:
    <div use:reveal={{ trigger: showLinks }}>
    Now, whenever showLinks toggles (e.g., when returning from the CV PDF export), Svelte automatically signals the reveal action to re-observe newly rendered elements.

Verification & Testing

  • Dynamic Rendering: Verified that conditional portfolio elements successfully animate (slide/fade) into view upon scrolling.
  • No Regression on Unmount: Verified that leaving/unmounting pages successfully disconnects the Intersection Observer without throwing console errors.
  • Code Quality: Successfully ran vitest and svelte-check to confirm no type regressions or test failures were introduced.

@google-labs-jules

Copy link
Copy Markdown
Contributor Author

[CI/CD Fix Attempt 1]

Fixed the ESLint failure in the test_and_build workflow.

The reveal Svelte action in src/lib/actions/reveal.ts was previously using any types for the options parameter and the update method's newOptions argument, which caused @typescript-eslint/no-explicit-any errors during the bun run lint step. Additionally, newOptions was declared but never used, resulting in a @typescript-eslint/no-unused-vars error.

To resolve this, I:

  1. Created an explicit RevealOptions interface for the action options to remove the any types.
  2. Removed the unused newOptions parameter from the update method.

Verified locally that bun run lint and the other CI checks pass successfully.

@Sparkier
Sparkier merged commit 0e85aa6 into main Jul 16, 2026
1 check passed
@Sparkier
Sparkier deleted the jules/feat/scroll-reveal-manual-update-js1-aad187b8-d56c-427e-a9cc-b763f1ac38a9 branch July 16, 2026 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant