pages/test.html - Issue 29703633: Noissue - Abstracted embedded video for use in blog

Side by Side Diff

Use n/p to move between diff chunks; N/P to move between comments.

Keyboard Shortcuts

	File
u :	up to issue
m :	publish + mail comments
M :	edit review message
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line
<Enter> :	respond to / edit current comment
d :	mark current comment as done

	Issue
u :	up to list of issues
m :	publish + mail comments
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue
# :	close issue

	Comment/message editing
<Ctrl> + s or <Ctrl> + Enter :	save comment
<Esc> :	cancel edit

Side by Side Diff: pages/test.html

Issue 29703633: Noissue - Abstracted embedded video for use in blog (Closed) Base URL: https://hg.adblockplus.org/web.adblockplus.org

Patch Set: Created Feb. 20, 2018, 10:13 p.m.

Left:
Right:

Use n/p to move between diff chunks; N/P to move between comments.

Jump to:

View unified diff | Download patch

OLD	NEW
(Empty)
	1 title=Video test
	juliandoucette 2018/02/20 22:28:10 Detail: I'm not planning to push this page. I just Detail: I'm not planning to push this page. I just created it to demonstrate how this abstraction works.
	2

	3 <head>

	4 <link rel="stylesheet" href="/css/video.css" />
	juliandoucette 2018/02/20 22:28:10 NIT: The "/" is unnecessary here. NIT: The "/" is unnecessary here.
	5 <script src="/js/video.js"></script>
	juliandoucette 2018/02/20 22:28:11 Detail: I wasn't sure about including this js/css Detail: I wasn't sure about including this js/css separately or as part of main js/css. So far, videos only appear on the index page and soon-to-be blog posts. Therefore we could only include it there. However, I'm not necessarily against including it in main js/css to avoid a separate HTTP request. I don't know which option is how much worse for performance. But I would like to separate this code via build process when we implement one for adblockplus.org.
	6 <script>

	7 document.documentElement.className = "js";
	juliandoucette 2018/02/20 22:28:11 NIT/Detail: This was only done inside index.js pre NIT/Detail: This was only done inside index.js previously :/ (that is why I inlined it here). (I'll move it to main.js in the next patchset) juliandoucette 2018/02/20 22:36:07 Done. Show quoted text On 2018/02/20 22:28:11, juliandoucette wrote: > NIT/Detail: This was only done inside index.js previously :/ (that is why I > inlined it here). > > (I'll move it to main.js in the next patchset) Done. ire 2018/02/21 09:11:29 My bad! Do you want to push this change separately Show quoted text On 2018/02/20 22:28:11, juliandoucette wrote: > NIT/Detail: This was only done inside index.js previously :/ (that is why I > inlined it here). > > (I'll move it to main.js in the next patchset) My bad! Do you want to push this change separately?
	8 </script>

	9 </head>

	10

	11 <div class="video-parent">

	12 <a

	13 class="video-link"

	14 target="_blank"

	15 href="https://www.youtube-nocookie.com/embed/pVYtzF5SemU?html5=1&autohid e=1&enablejsapi=1&controls=2&fs=1&modestbranding=1&rel=0&amp ;showinfo=0&theme=light&autoplay=1">

	16 <img

	17 class="video-thumbnail"

	18 src="/img/video-thumbnail.jpg"

	19 alt="{{ video-alt-text Introduction to Adblock Plus video thumbnail }}">

	20 <img

	21 class="video-play"

	22 src="/img/video-external.png"

	23 alt="{{ video-play-alt-text Play Video }}">

	24 </a>

	25 <p class="video-disclaimer">

	26 {{ video-disclaimer Click to play this video. Please note that this video is hosted by YouTube. When this video is played, some personal data is transferred to YouTube. For more information, please review YouTube's <a href='https://www. google.com/intl/en/policies/privacy/' target='_blank'>privacy policy</a>. }}

	27 </p>

	28 </div>

	29

	30 <div class="video-parent" style="max-width: 520px; margin-top: 1em;">
	juliandoucette 2018/02/20 22:28:11 Detail: The first video demonstrates that this com Detail: The first video demonstrates that this component is responsive (full width) by default. The second demo demonstrates how to shrink the video (by limiting the width of the parent or container).
	31 <a

	32 class="video-link"

	33 target="_blank"

	34 href="https://www.youtube-nocookie.com/embed/pVYtzF5SemU?html5=1&autohid e=1&enablejsapi=1&controls=2&fs=1&modestbranding=1&rel=0&amp ;showinfo=0&theme=light&autoplay=1">

	35 <img

	36 class="video-thumbnail"

	37 src="/img/video-thumbnail.jpg"

	38 alt="{{ video-alt-text Introduction to Adblock Plus video thumbnail }}">

	39 <img

	40 class="video-play"

	41 src="/img/video-external.png"

	42 alt="{{ video-play-alt-text Play Video }}">

	43 </a>

	44 <p class="video-disclaimer">

	45 {{ video-disclaimer Click to play this video. Please note that this video is hosted by YouTube. When this video is played, some personal data is transferred to YouTube. For more information, please review YouTube's <a href='https://www. google.com/intl/en/policies/privacy/' target='_blank'>privacy policy</a>. }}

	46 </p>

	47 </div>

	48

	49 <? include scripts/initialize-videos ?>

OLD	NEW

« no previous file with comments | « includes/scripts/initialize-videos.html ('k') | static/css/index.css » ('j') | static/css/index.css » ('J')