Fix support for reusing responsive video src#75
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where multiple media queries that reference the same video URL would fail because the old implementation used URLs as object keys, causing collisions when the same URL was used for different media queries. The fix restructures the data to use media queries as keys and URLs as values in the server component, and introduces an indexed intermediary structure in the client component to avoid similar collisions.
Changes:
- Modified
LazyVideoServer.tsxto buildmediaSrcswith media queries as keys instead of URLs, preventing key collisions when the same URL is used for different media queries - Updated
LazyVideoClient.tsxto use an indexed query structure to avoid issues with duplicate URLs, and refactoredgetFirstMatchto work with the new structure - Added comprehensive tests to verify the fix works correctly for both the base case and the Contentful integration
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| packages/react/src/LazyVideo/LazyVideoServer.tsx | Refactored media source object construction to use media queries as keys and URLs as values, eliminating the key collision issue |
| packages/react/src/LazyVideo/LazyVideoClient.tsx | Added indexed query transformation to handle duplicate URLs correctly and updated the matching logic to work with the new structure |
| packages/react/cypress/component/LazyVideo.cy.tsx | Added test case to verify that the same video asset can be used with different media queries |
| packages/contentful/cypress/component/ContentfulVisual.cy.tsx | Added test to verify fallback behavior when portrait video is not set |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
….com:BKWLD/next-visual into fix-support-for-reusing-responsive-video-src
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No description provided.