QwikDev / qwik

Instant-loading web apps, without effort
https://qwik.dev
MIT License
20.67k stars 1.29k forks source link

[🐞] video tags trigger resize event #6738

Open arensade opened 1 month ago

arensade commented 1 month ago

Which component is affected?

Qwik Runtime

Describe the bug

Description

When adding a simple <video> tag to my Qwik app, the Qwik framework triggers a resize event unexpectedly. I can listen to this event using the following code block:

useOnDocument(
  "resize",
  $(() => {
    console.log("RESIZE----");
  })
);

However, when I try to listen for the resize event using pure JavaScript in a simple index.html file, I do not observe the same behavior (as expected).

Steps to Reproduce

  1. Clone the repository: qwik-video-tag-demo
  2. Navigate to the qwik-app directory and install the dependencies:
    cd qwik-app
    npm install
  3. Start the development server:
    npm start
  4. Open the application in your browser.
  5. Observe the console logs. You will see RESIZE---- being logged repeatedly.

Note: just one file changed after installation with cli: https://github.com/arensade/qwik-video-tag-demo/blob/main/qwik-app/src/routes/index.tsx

Expected Behavior

The resize event should not be triggered when a <video> tag is added to the Qwik app unless an actual resize occurs.

Actual Behavior

The resize event is being triggered continuously without any actual resize action.

Reproduction Repository

A minimal reproducible example is available in the following repository: qwik-video-tag-demo

Additional Context

Reproduction

https://github.com/arensade/qwik-video-tag-demo/tree/main/qwik-app

Steps to reproduce

No response

System Info

Operating System**: (MacOS & Windows)

Additional Information

No response

wmertens commented 1 month ago

A reproduction using only qwik doesn't do anything but perhaps because the videos aren't included https://qwik.dev/playground/#f=Q0o0JuaWNYiwRcS%2FDmqi1YEnIyX0KFIiKhkB7UbNBWBHKQHLOWDGUdIB85ATHQggV%2BVKQa7BnlGuukCgBPQESLpWE%2BIhnOEKbLtAGiq21Qmg%2FJMfDyyIgOV6Qi1SgIElFBKBZXwBqGTLyc8vUMgF5VsFEL%2FYMw9Yp8PDHqwDklkViouSgebq6SdlpusmlSZnA4m8vMp4YHWaCSyF9MpTk3ITasFp3VYJbIk%2BSEgJETuYhkH1xptYGBTo5eaXoekHiqBot9EHiyNiG9RYQ4vs0Rw6JHMoAA

arensade commented 1 month ago

Hi @wmertens, Exactly, please change webm source to: https://dl6.webmfiles.org/big-buck-bunny_trailer.webm

arensade commented 1 month ago

Hi @wmertens,

Is there any way to prevent dispatch any event on the added tags? Unfortunately video tags with resize event cause crash on devices such as iPhone and Android (maybe because it going to re-render the elements and load the videos over and over).

arensade commented 1 month ago

I guess below merge caused this issue:

https://github.com/QwikDev/qwik/pull/6547

arensade commented 1 month ago

I've identified a concern with our current approach for handling window events, specifically click and resize events. Currently, we're using the useOnWindow("resize", ...) method to manage these events.

The useOnWindow function is expected to listen for specified events (like "resize") and execute the associated callback when the event occurs. However, it appears that this function is also dispatching the "resize" event, which is not the intended behavior.

As an alternative, I am planning to implement the standard addEventListener method directly to listen for the "resize" event. This approach should provide the necessary control over the page's behavior when it is resized without inadvertently dispatching events.

Example:

window.addEventListener("resize", (event) => {
    // Control page size when resized
});

This solution will ensure that we are only listening for and responding to the "resize" event as expected at this moment.

shairez commented 1 month ago

thanks for the update @arensade !

I just looked into the source code and I don't see anywhere that it fires the actual event it listens to, but maybe I missed it

Can you please try moving your repro code to stackblitz (using https://qwik.new) that shows how it fires off the event ?

github-actions[bot] commented 1 month ago

Hello @arensade. Please provide the missing information requested above. Issues marked with STATUS-2: missing info will be automatically closed if they have no activity within 14 days. Thanks 🙏

arensade commented 1 month ago

Hi @shairez,

I simulate the issue on Stackblitz, you can find the below link:

https://stackblitz.com/edit/qwik-starter-uj6heo?file=src%2Froutes%2Fhome%2Findex.tsx

Just open Console in Google Dev Tools, it going to write "RESIZE" when the app loaded.

shairez commented 1 month ago

Thank you @arensade ! We'll look into it