Skip to content

Clean up CldVideoPlayer and CldUploadWidget on astro:before-swap #126

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Nov 13, 2025
Merged

Clean up CldVideoPlayer and CldUploadWidget on astro:before-swap #126

merged 1 commit into from
Nov 13, 2025
+90 −8

Conversation

@Juneezee
Copy link
Contributor

@Juneezee Juneezee commented Oct 1, 2025 •
edited
Loading

Description

This PR adds cleanup handling for the <CldUploadWidget /> and <CldVideoPlayer /> components on the astro:before-swap event.

Reference: https://cloudinary.com/documentation/upload_widget_reference#destroy
Reference: https://cloudinary.com/documentation/video_player_api_reference#dispose
Reference: https://docs.astro.build/en/guides/view-transitions/#astrobefore-swap

I created 3 minimal files locally to verify the changes:

<!-- docs/src/pages/TestLayout.astro -->
---
import { ClientRouter } from "astro:transitions";
const { title } = Astro.props;
---
<html lang="en">
  <head>
    <meta charset="utf-8">
    <meta name="viewport" content="width=device-width, initial-scale=1">
    <ClientRouter/>
  </head>
  <body>
    <nav>
      <a href="/upload">CldUploadWidget</a>
      <a href="/video">CldVideoPlayer</a>
    </nav>
    <h1>{title}</h1>
    <article>
      <slot /> <!-- your content is injected here -->
    </article>
  </body>
</html>
<!-- docs/src/pages/upload.astro -->
---
import { CldUploadWidget } from "../../../astro-cloudinary";
import TestLayout from "./TestLayout.astro";
---

<TestLayout title="Cleanup CldUploadWidget">
  <CldUploadWidget uploadPreset="astro-cloudinary-signed" signatureEndpoint="/api/sign-cloudinary-params">
    <button>Upload</button>
  </CldUploadWidget>

  <CldUploadWidget options={{ sources: ['local', 'url', 'unsplash'] }} uploadPreset="astro-cloudinary-unsigned">
    <button>Upload</button>
  </CldUploadWidget>
</TestLayout>
<!-- docs/src/pages/video.astro -->
---
import { CldVideoPlayer } from "../../../astro-cloudinary";
import TestLayout from "./TestLayout.astro";
---

<TestLayout title="Cleanup CldVideoPlayer">
  <CldVideoPlayer src="samples/sea-turtle" width="1920" height="1080" />
  <CldVideoPlayer src="samples/elephants" width="1920" height="1080" />
</TestLayout>

Here is a screen recording of the testing performed with debugging console.log statements:

2025-10-01.10-58-07.mp4

Issue Ticket Number

Closes #10.

This commit also fixes #90.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Fix or improve the documentation
  • This change requires a documentation update

Checklist

  • I have followed the contributing guidelines of this project as mentioned in CONTRIBUTING.md
  • I have created an issue ticket for this PR
  • I have checked to ensure there aren't other open Pull Requests for the same update/change?
  • I have performed a self-review of my own code
  • I have run tests locally to ensure they all pass
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes needed to the documentation

@Juneezee
Clean up CldVideoPlayer and CldUploadWidget on astro:before-swap
5f5d689 
Closes #10.

This commit also fixes #90.

Signed-off-by: Eng Zer Jun <[email protected]>
@vercel
Copy link

vercel bot commented Oct 1, 2025

@Juneezee is attempting to deploy a commit to the Cloudinary DevX Team on Vercel.

A member of the Team first needs to authorize it.

Juneezee
Juneezee commented Oct 1, 2025
View reviewed changes
Copy link
Contributor Author

@Juneezee Juneezee left a comment

Choose a reason for hiding this comment

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

I have performed a self-review of my code changes.

/cc @jlooper-cloudinary @devpatocld @eportis-cloudinary Please take a look. Thanks 😊

astro-cloudinary/src/components/CldVideoPlayer.astro
Comment on lines +206 to +211
window.addEventListener('load', state.initialise);

// Lifecycle events, requires <ClientRouter />
// https://docs.astro.build/en/guides/view-transitions/#lifecycle-events
document.addEventListener('astro:after-swap', state.reset);
document.addEventListener('astro:page-load', state.initialise);
Copy link
Contributor Author

@Juneezee Juneezee Oct 1, 2025

Choose a reason for hiding this comment

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

document.addEventListener('astro:page-load', state.initialise);

This is the line that fixes #90.

I am aware of another PR attempt at #99. However, that PR is insufficient because both the load event and the astro:page-load event are fired on the first page load (e.g., when you click the refresh button). Therefore, it is necessary to use a function like the useState() function above, which relies on closures
to define initialization variables and ensure that initialization runs only once.

@devpatocld devpatocld requested review from eportis-cloudinary and removed request for eportis-cloudinary October 2, 2025 16:52
@Juneezee
Copy link
Contributor Author

Juneezee commented Oct 27, 2025

@devpatocld @jlooper-cloudinary @eportis-cloudinary Will this be reviewed before October ends? 🥹

@jlooper-cloudinary
Copy link
Collaborator

jlooper-cloudinary commented Oct 27, 2025

@devpatocld @jlooper-cloudinary @eportis-cloudinary Will this be reviewed before October ends? 🥹

hi, it's under review now, please be patient.

@Juneezee
Copy link
Contributor Author

Juneezee commented Nov 6, 2025

@devpatocld @jlooper-cloudinary Just a quick follow-up. This PR didn't get merged in time before the end of October. Does that mean I'm no longer eligible for the swag kit? Thanks for clarifying!

@eportis-cloudinary
Copy link
Contributor

eportis-cloudinary commented Nov 13, 2025

Hi @Juneezee! Thank you for this thoughtful and well-authored PR, and apologies for the tardy delay. I've just put it through the paces and it does indeed solve both issues, in a clean way. Merging.

I'm adding the necessary tags so that you receive swag for your Hacktoberfest contribution. Thank you!!

@eportis-cloudinary eportis-cloudinary merged commit 57d7c88 into cloudinary-community:main Nov 13, 2025
1 check failed
@eportis-cloudinary
Copy link
Contributor

eportis-cloudinary commented Nov 13, 2025

@all-contributors please add @Juneezee for code

@allcontributors allcontributors bot mentioned this pull request Nov 13, 2025
Merged
@allcontributors
Copy link
Contributor

allcontributors bot commented Nov 13, 2025

@eportis-cloudinary

I've put up a pull request to add @Juneezee! 🎉

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

Reviewers

@eportis-cloudinary eportis-cloudinary Awaiting requested review from eportis-cloudinary

Assignees

No one assigned

Labels

hacktoberfest hacktoberfest2025 hacktoberfest-accepted

Projects

None yet

Milestone

No milestone

4 participants

@Juneezee @jlooper-cloudinary @eportis-cloudinary @devpatocld