Noissue - Adjustments to video component for blog

Noissue - Adjustments to video component for blog

[juliandoucette, 2018-04-24 15:56:05]

Ready for review.

See
https://adblockplus.org/blog/what-you-can-block-with-adblock-plus-part-2-video
for an example blog post with a video.

https://codereview.adblockplus.org/29760667/diff/29760668/static/css/main.css
File static/css/main.css (right):

https://codereview.adblockplus.org/29760667/diff/29760668/static/css/main.css...
static/css/main.css:751: display: block;
Images are displayed inline by default. This can result in an unintended space
underneath them. See
https://adblockplus.org/blog/what-you-can-block-with-adblock-plus-part-2-video.

https://codereview.adblockplus.org/29760667/diff/29760668/static/css/main.css...
static/css/main.css:778: margin: 0;
This prevents an unintended space above and below the disclaimer (which is
visible on the home page... It's strange we haven't addressed this before :/).

https://codereview.adblockplus.org/29760667/diff/29760668/static/css/textpatt...
File static/css/textpattern.css (right):

https://codereview.adblockplus.org/29760667/diff/29760668/static/css/textpatt...
static/css/textpattern.css:163: .video-parent
This prevents videos from stretching full-width on blog pages.

[juliandoucette, 2018-04-24 15:56:44]

https://codereview.adblockplus.org/29760667/diff/29760668/static/css/textpatt...
File static/css/textpattern.css (right):

https://codereview.adblockplus.org/29760667/diff/29760668/static/css/textpatt...
static/css/textpattern.css:163: .video-parent
On 2018/04/24 15:56:05, juliandoucette wrote:
> This prevents videos from stretching full-width on blog pages.

Detail: I based this width on the default that YouTube recommends in their embed
code.

[ire, 2018-04-25 08:34:33]

Thanks Julian! Just one question

https://codereview.adblockplus.org/29760667/diff/29760668/static/css/main.css
File static/css/main.css (right):

https://codereview.adblockplus.org/29760667/diff/29760668/static/css/main.css...
static/css/main.css:751: display: block;
On 2018/04/24 15:56:05, juliandoucette wrote:
> Images are displayed inline by default. This can result in an unintended space
> underneath them. See
>
https://adblockplus.org/blog/what-you-can-block-with-adblock-plus-part-2-video.

Acknowledged.

https://codereview.adblockplus.org/29760667/diff/29760668/static/css/main.css...
static/css/main.css:778: margin: 0;
On 2018/04/24 15:56:05, juliandoucette wrote:
> This prevents an unintended space above and below the disclaimer (which is
> visible on the home page... It's strange we haven't addressed this before :/).

Acknowledged. Yes it is :/

https://codereview.adblockplus.org/29760667/diff/29760668/static/css/textpatt...
File static/css/textpattern.css (right):

https://codereview.adblockplus.org/29760667/diff/29760668/static/css/textpatt...
static/css/textpattern.css:163: .video-parent
On 2018/04/24 15:56:05, juliandoucette wrote:
> This prevents videos from stretching full-width on blog pages.

Ack. But why don't we want the videos stretching full-width?

[juliandoucette, 2018-04-25 09:56:03]

Question answered :)

https://codereview.adblockplus.org/29760667/diff/29760668/static/css/textpatt...
File static/css/textpattern.css (right):

https://codereview.adblockplus.org/29760667/diff/29760668/static/css/textpatt...
static/css/textpattern.css:163: .video-parent
On 2018/04/25 08:34:33, ire wrote:
> On 2018/04/24 15:56:05, juliandoucette wrote:
> > This prevents videos from stretching full-width on blog pages.
> 
> Ack. But why don't we want the videos stretching full-width?

1. They weren't full-width before
2. I think full-width is a little too wide in the blog posts as they are today

Example:
https://adblockplus.org/blog/what-you-can-block-with-adblock-plus-part-2-video

[ire, 2018-04-25 13:45:33]

LGTM (+ comment)

https://codereview.adblockplus.org/29760667/diff/29760668/static/css/textpatt...
File static/css/textpattern.css (right):

https://codereview.adblockplus.org/29760667/diff/29760668/static/css/textpatt...
static/css/textpattern.css:163: .video-parent
On 2018/04/25 09:56:03, juliandoucette wrote:
> On 2018/04/25 08:34:33, ire wrote:
> > On 2018/04/24 15:56:05, juliandoucette wrote:
> > > This prevents videos from stretching full-width on blog pages.
> > 
> > Ack. But why don't we want the videos stretching full-width?
> 
> 1. They weren't full-width before
> 2. I think full-width is a little too wide in the blog posts as they are today
> 
> Example:
> https://adblockplus.org/blog/what-you-can-block-with-adblock-plus-part-2-video

Thanks! I actually prefer them full-width, I generally like bigger videos and I
think it looks better in this case since most pictures are also full width. 

But if they weren't full-width before (I assume you mean before we changed the
header/footer styles) then you should go ahead with this.

[juliandoucette, 2018-04-25 16:53:26]

Thanks!

Done https://hg.adblockplus.org/web.adblockplus.org/rev/20c681b19a36.