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.

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

Bug: onResize media event is missing #21972

Closed
rileyjshaw opened this issue Jul 27, 2021 · 0 comments
Closed

Bug: onResize media event is missing #21972

rileyjshaw opened this issue Jul 27, 2021 · 0 comments
Labels
Component: DOM Type: Feature Request

Comments

@rileyjshaw
Copy link
Contributor

Note: I’m happy to make a pull request to fix this, I just wanted to log it first to ensure there’s interest.

React’s synthetic media events contain several existing media events, for instance onLoadedMetadata and onVolumeChange. But there is no onResize handler.

resize is a standard media event that triggers when one or both of the videoWidth and videoHeight attributes have just been updated. It’s useful for responding to resolution changes in video players.

React version: 17.0.2 (latest release as of initial issue report)

Steps To Reproduce

  1. Create a <video> element with an onResize prop.
  2. Check the console for the following warning:
 Warning: Unknown event handler property `onResize`. It will be ignored.

Link to code example: https://codesandbox.io/s/musing-snowflake-zb0qh?file=/src/App.js

The current behavior

onResize handlers are ignored on <video> elements.

The expected behavior

onResize handlers are valid on <video> elements.
@rileyjshaw rileyjshaw added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Jul 27, 2021
rileyjshaw added a commit to rileyjshaw/react that referenced this issue Jul 27, 2021
@rileyjshaw
Fix facebook#21972: Add onResize event to video elements
809a54d 
This is a simple fix for facebook#21972 that adds support for the `onResize` media event.

I created a separate `videoEventTypes` array since I doubt anyone will want to add `onResize` to an audio event. It would simplify the code a bit to just add `resize` to the `mediaEventTypes` array, if that’s preferred.
rileyjshaw added a commit to rileyjshaw/react that referenced this issue Jul 27, 2021
@rileyjshaw
Fix facebook#21972: Add onResize event to video elements
5ec46b2 
This is a simple fix for facebook#21972 that adds support for the `onResize` media event.

I created a separate `videoEventTypes` array since I doubt anyone will want to add `onResize` to an audio event. It would simplify the code a bit to just add `resize` to the `mediaEventTypes` array, if that’s preferred.

Pre-commit checklist ([source](https://reactjs.org/docs/how-to-contribute.html#sending-a-pull-request))

✅ Fork the repository and create your branch from `main`.
✅ Run `yarn` in the repository root.
✅ If you’ve fixed a bug or added code that should be tested, add tests!
✅ Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch TestName` is helpful in development.
✅ Run `yarn test --prod` to test in the production environment.
✅ If you need a debugger, run `yarn debug-test --watch TestName`, open chrome://inspect, and press “Inspect”.
✅ Format your code with prettier (`yarn prettier`).
✅ Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only check changed files.
✅ Run the Flow typechecks (`yarn flow`).
✅ If you haven’t already, complete the CLA.
@rileyjshaw rileyjshaw mentioned this issue Jul 27, 2021
Merged
@gaearon gaearon added Component: DOM Type: Feature Request and removed Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug labels Sep 6, 2021
@gaearon gaearon closed this as completed in fbce2d5 Sep 7, 2021
zhengjitf pushed a commit to zhengjitf/react that referenced this issue Apr 15, 2022
@rileyjshaw @zhengjitf
Fix facebook#21972: Add onResize event to video elements (facebook#…
98e294d 
…21973)

* Fix facebook#21972: Add `onResize` event to video elements

This is a simple fix for facebook#21972 that adds support for the `onResize` media event.

I created a separate `videoEventTypes` array since I doubt anyone will want to add `onResize` to an audio event. It would simplify the code a bit to just add `resize` to the `mediaEventTypes` array, if that’s preferred.

Pre-commit checklist ([source](https://reactjs.org/docs/how-to-contribute.html#sending-a-pull-request))

✅ Fork the repository and create your branch from `main`.
✅ Run `yarn` in the repository root.
✅ If you’ve fixed a bug or added code that should be tested, add tests!
✅ Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch TestName` is helpful in development.
✅ Run `yarn test --prod` to test in the production environment.
✅ If you need a debugger, run `yarn debug-test --watch TestName`, open chrome://inspect, and press “Inspect”.
✅ Format your code with prettier (`yarn prettier`).
✅ Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only check changed files.
✅ Run the Flow typechecks (`yarn flow`).
✅ If you haven’t already, complete the CLA.

* Consolidate `videoEventTypes` array into `mediaEventTypes`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Component: DOM Type: Feature Request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@gaearon @rileyjshaw