Skip to content
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.

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

Fix the destroy method #125

Closed
wants to merge 1 commit into from

Conversation

tadaskarpavicius
Copy link

This PR contains a:

  • bugfix
  • new feature
  • code refactor
  • test update
  • typo fix
  • metadata update

Motivation / Use-Case

destroy() method should remove all traces of the plugin and restore the media element to its original state.
At the moment it just removes the container element (with video element itself) which is not an expected behavior

Breaking Changes

Potentially breaking in those cases where users expected such destroy() behavior and wrote code which interacts with outerContainer after destroy method was called

Additional Info

Copy link

vercel bot commented Mar 18, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
vlite ✅ Ready (Inspect) Visit Preview Jul 26, 2024 2:36pm

@yoriiis
Copy link
Member

yoriiis commented Apr 2, 2024

Hello @tadaskarpavicius, thanks for your feedback!

Interesting, indeed, the changes are potentially breaking and I don't want to make a major release with just that for the moment.

I can add a parameter (disabled by default) to the "destroy" function that does this behaviour. Tell me what you think

destroy() method should remove all traces of the plugin and restore the media element to its original state. At the moment it just removes the container element (with video element itself) which is not an expected behavior
@yoriiis
Copy link
Member

yoriiis commented Jul 26, 2024

@tadaskarpavicius Still relevant today?

@yoriiis
Copy link
Member

yoriiis commented Aug 5, 2024

Closing due to inactivity. Thanks! Feel free to reopen if relevant.

@yoriiis yoriiis closed this Aug 5, 2024
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.

2 participants