padding in the "video" column on the leaderboard

[spMohanty]

The padding int the video column on the leaderboard is a bit weird ?

https://www.crowdai.org/challenges/learning-how-to-walk/leaderboards

[scarroll32]

The video wasn't designed for ... I haven't yet discussed it with Jason but let's discuss it with him next week. The modal isn't yet implemented https://github.com/crowdAI/crowdai/issues/135

[jsnrynlds]

@seanfcarroll @spMohanty will videos always be a 4:3 aspect ratio? If so I'll provide a new placeholder image as these are currently square. I think we can change the padding on the the video wrapper to:

table td {
padding: 0 0;
}

[scarroll32]

I suspect so, but @spMohanty can you confirm?

[spMohanty]

At least in context of this challenge it will always be 4:3 aspect ratio.

In future challenges, we can impose the rule and stick to videos with 4:3 aspect ration.

[jsnrynlds]

@seanfcarroll Coming back to this, I don't think we should include a placeholder image for entries that don't include a video. Could we just replace this with an em dash?

[scarroll32]

Hi Jason

There will be a mix of some rows with video and some without

Eg https://www.crowdai.org/challenges/learning-how-to-walk/leaderboards

But happy to go with your suggestion.

Cheers, Sean

On 21 Jun 2017, at 14:43, Jason Reynolds notifications@github.com wrote:

@seanfcarroll Coming back to this, I don't think we should include a placeholder image for entries that don't include a video. Could we just replace this with an em dash?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[scarroll32]

@jsnrynlds What is the final decision on this?

[scarroll32]

Hi @jsnrynlds, not sure if I did this correctly, but the result is as below, so I have not added the code at this point.

[jsnrynlds]

@seanfcarroll Ah, okay, I think that's due to the placeholders being square previously. Think 20px padding above/below will work.

I'm really up against it today but will be in touch tomorrow.

[scarroll32]

OK no problem, cheers.

I'm on vacation from next week.

[jsnrynlds]

@seanfcarroll When I inspect this in Chrome it's showing padding: 23px 0; on the td. Can we try changing this topadding: 16px 0;? Also, I don't think we should include placeholders if there's no image... maybe just an em dash.

[scarroll32]

Hi @jsnrynlds

I added the styling but it didn't seem to make a difference. Not sure if I am adding it in the wrong place, but you can see the change here: https://github.com/crowdAI/crowdai/pull/234/commits/ae4913a2124493d3bff463acab7f790c02d1e1fc

I also replaced the placeholder image with a tag https://github.com/crowdAI/crowdai/commit/df3bf0e7a7a6089a5a5b6de4e2b1bbd0205eddf5

I think the placeholder works better.

I'll leave this issue open as the padding issues are not resolved.

[scarroll32]

Added some padding. Closed.