Featured Video -- can you apply a class instead of inline css?

[alexgreen]

@MrBryan I'm seeing you've reduced the gap below featured videos by applying an inline style (maybe with javascript?): <section class="featured-media" data-dev-object-descrip="02-organisms/article/featured-media" style="margin-bottom: -83px;">

Instead of doing this can you apply a class to the section (i.e. .featured-media--video) and then style that class in the stylesheet? I ask because having a class there would allow me to make necessary CSS adjustments to this style and other related elements. (and in general applying classes rather than inline stylestyles makes it much easier for others to understand and work with.)

[MrBryan]

I would have but the issue there was that the JavaScript was needed to calculate the difference between the height of the fixed offset you made for the ad /feature and the actual offset needed by various wider aspect ratio video / images ( so no class could cover the variability ) . It's not ideal though for sure. Maybe there is a better css solution for making space for the ad/feature that will remove the need

Sent from my iPhone

On Jun 29, 2016, at 1:44 PM, alexgreen notifications@github.com wrote:

@MrBryan I'm seeing you've reduced the gap below featured videos by applying an inline style (maybe with javascript?):

Instead of doing this can you apply a class to the section (i.e. .featured-media--video) and then style that class in the stylesheet? I ask because having a class there would allow me to make necessary CSS adjustments to this style and other related elements. (and in general applying classes rather than inline stylestyles makes it much easier for others to understand and work with.) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[alexgreen]

Can't you just get the javascript to apply a class (under the currently-specified conditions) rather than applying inline styling?

[MrBryan]

Yes but the class wouldn't help anything because the class doesn't know how big the gap will be in advance. I can apply a class in addition to an in-line style that knows the height if that helps, so that you know when the margin adjustment has taken place

[alexgreen]

Can't you set javascript to only apply the class when margin adjustment is required? ...and then the necessary styling can be applied to the class, not inline.

(We might be hitting the limits text-based communication here, but I believe what I'm suggesting here is essentially the same thing you're proposing in #202 .)

[MrBryan]

You would have to have a whole bunch of classes defined in advance of varying heights and then match them up with some weird rules . As it is it only does the margin when it is needed, and I could also add a class if you like at the same point. Redefining class style rules would not simplify it either and is unusual. When you do that kind of targeting by class it ultimately applies it in line anyways with Jquery . Maybe if you tell me what problem you're trying to solve I can understand better where your head is at

[alexgreen]

Aha, you're saying that it calculates different margin offsets depending on the conditions? (I thought it was always applying the same offset, because videos usually all have the same aspect ratio, so I wouldn't expect the margin to vary from one video to the next.)

[MrBryan]

Just saw your edit. So you mean detect the size of the image in Bric?. Even then the image could see have infinite aspect ratios that need to be covered by a class - So the closest thing to what you're suggesting would be to inject a text style block that defines a class and the amount of margin that comes with it - it's too weird I think especially since that should be in the head

[MrBryan]

Exactly it does both videos and short images

[alexgreen]

Ok, in that case a class wouldn't work.

Does it also affect photos? Because I think photos might be better dealt with via CSS cropping. (To trim the image to an appropriate aspect ratio.)

[alexgreen]

?

[phillipadsmith]

Does it also affect photos? Because I think photos might be better dealt with via CSS cropping. (To trim the image to an appropriate aspect ratio.)

We have a different tool for re-sizing/cropping photos "on the fly;" it's really best not to crop in CSS because then the client is receiving too much image (unnecessary bandwidth) which impacts performance.

[alexgreen]

We have a different tool for re-sizing/cropping photos "on the fly;"

That makes the css much simpler, but the images really only need by cropped in certain viewports, whereas in most views we could actually display the full uncropped image.

Cropping server-side is definitely simpler so let's go with that for the sake of launch expediency, but it risks losing important peripheral image elements and composition in the case that someone has a good reason for uploading a wide aspect photo. Admittedly this will likely be very rare (other than video) so probably it's pretty low priority.

For server-side cropping please set all featured images to be cropped to 3 x 2, so that the width is equal to no more than 1.5 times the height. For example an image that is 400px tall should be no wider than 600px. Only the width needs to be cropped, tall images should be ok.

[alexgreen]

I'll open a new issue for the cropping.

[phillipadsmith]

For server-side cropping please set all featured images to be cropped to 3 x 2, so that the width is equal to no more than 1.5 times the height. For example an image that is 400px tall should be no wider than 600px. Only the width needs to be cropped, tall images should be ok.

That is the plan, indeed.