Skip to content

🚀 [Story video] Use the inlined video response instead of issuing an XHR request, for the 1st video of the 1st web story page #37499

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

Merged
coreymasanto merged 25 commits into from
Feb 14, 2022
Merged

🚀 [Story video] Use the inlined video response instead of issuing an XHR request, for the 1st video of the 1st web story page #37499

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
163b8f7
For the first video of the first web story page, use the inlined vide…
coreymasanto Jan 26, 2022
0819baf
Fix merge conflict
coreymasanto Jan 26, 2022
cf79773
Refactor to prevent unnecessary XHR-related function calls
coreymasanto Jan 26, 2022
16ba4e8
Fix promise
coreymasanto Jan 26, 2022
ad8a9a0
Syntax fix
coreymasanto Jan 27, 2022
455d45e
Allow the XHR request to be sent if the inline video is unexpectedly …
coreymasanto Jan 27, 2022
4b3a02f
Lint
coreymasanto Jan 27, 2022
73c67c1
Remove unnecessary variable from shouldUseInlineVideoResponse and upd…
coreymasanto Jan 28, 2022
c92c2e7
Wrap JSON.parse() in a try/catch
coreymasanto Jan 28, 2022
6164988
Pull out a new requestCachedVideoSources() method from the fetchCache…
coreymasanto Jan 28, 2022
7c760e0
Clean up preexisting lengthy if-statement
coreymasanto Jan 28, 2022
b4093ea
Merge branch 'main' into inlinedVideo
coreymasanto Feb 8, 2022
5640b66
Simplify JSON parsing logic in requestCachedVideoSources
coreymasanto Feb 8, 2022
8787143
Add a check for the 'sources' key
coreymasanto Feb 8, 2022
d2465cc
Lint fix
coreymasanto Feb 8, 2022
bc73828
Add initial, untested version of tests for inline video response logic
coreymasanto Feb 11, 2022
697dd6b
Merge branch 'main' into inlinedVideo
coreymasanto Feb 11, 2022
b477b97
Add a describe grouping for the set of inline video response tests
coreymasanto Feb 11, 2022
2d108a6
Update logic for determining whether the inline response should be used
coreymasanto Feb 11, 2022
a5487d2
Get tests passing
coreymasanto Feb 11, 2022
c328d21
Lint fixes
coreymasanto Feb 11, 2022
e0e6de7
Merge branch 'main' into inlinedVideo
coreymasanto Feb 11, 2022
e0e8b85
Merge branch 'main' into inlinedVideo
coreymasanto Feb 11, 2022
d4ed4ec
Merge branch 'main' into inlinedVideo
coreymasanto Feb 14, 2022
db458ea
Merge branch 'main' into inlinedVideo
coreymasanto Feb 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Lint fixes
  • Loading branch information
@coreymasanto
coreymasanto committed Feb 11, 2022
commit c328d214301a138f0065842f86adc71cfc4e9602
8 changes: 4 additions & 4 deletions extensions/amp-video/0.1/test/test-video-cache.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,8 +496,6 @@ describes.realWin('amp-video cached-sources', {amp: true}, (env) => {

describe('web stories: inlined video', async () => {
it('should use the inlined source for the first video in the story instead of sending an XHR request', async () => {
console.log('test #1');

// Set up an inlined source response for the first video in the story
const storyEl = createStoryForInlineVideoTesting();
env.win.document.body.appendChild(storyEl);
Expand All @@ -510,7 +508,9 @@ describes.realWin('amp-video cached-sources', {amp: true}, (env) => {
await fetchCachedSources(videoEl, env.ampdoc);

expect(xhrSpy).to.have.not.been.called;
const inlinedSources = videoEl.querySelectorAll('source[src="inlined_video_response.mp4"]');
const inlinedSources = videoEl.querySelectorAll(
'source[src="inlined_video_response.mp4"]'
);
expect(inlinedSources).to.have.lengthOf(1);
});

Expand Down Expand Up @@ -599,7 +599,7 @@ describes.realWin('amp-video cached-sources', {amp: true}, (env) => {
const storyPageEl2 = env.win.document.createElement('amp-story-page');
storyEl.appendChild(storyPageEl1);
storyEl.appendChild(storyPageEl2);

// Place two videos on the first page. video #1 is nested more deeply than
// video #2, but it should still be considered the first video on the page.
const gridLayerEl = env.win.document.createElement('amp-story-grid-layer');
Expand Down
4 changes: 3 additions & 1 deletion extensions/amp-video/0.1/video-cache.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,8 @@ function requestCachedVideoSources(videoEl, ampdoc) {
*/
function shouldUseInlineVideoResponse(videoEl, win) {
// Google video cache inlines the first video of the first web story page.
const firstVid = win.document.querySelector('amp-story-page:first-of-type amp-video');
const firstVid = win.document.querySelector(
'amp-story-page:first-of-type amp-video'
);
return videoEl === firstVid;
}