Video embed can overflow over other parts of the UI
-
Observe this embed on iPad (w/Safari)
-
Images does this too. On narrower screens 50% of the YouTube onebox Is just plain missing. It's outside the visible area of the screen.
Test
https://www.youtube.com/watch?v=qPPciVYVLIQ
-
What the hell, I've never seen this happen before.
-
Ah, now it failed as it as usually.
-
@Arantor said in Video embed can overflow over other parts of the UI:
Observe this embed on iPad (w/Safari)
I've seen that on Chrome too.
-
display:block
OMG WHAT A REVOLUTIONARY WEB IDEA!!!!!
-
@Arantor said in Video embed can overflow over other parts of the UI:
bserve this embed on iPad (w/Safari)
Observed.
-
@Lorne-Kates said in Video embed can overflow over other parts of the UI:
display:block
OMG WHAT A REVOLUTIONARY WEB IDEA!!!!!Yeah, wouldn't it be amazing if you checked right now and it already used
display:block
without anyone changing anything
-
@Lorne-Kates said in Video embed can overflow over other parts of the UI:
display:block
OMG WHAT A REVOLUTIONARY WEB IDEA!!!!!The problem is us. The "reply" and "quote" tools aren't buttons in stock NodeBB, they're just links. Locally, we're adding some padding to make them look like buttons. If we did something like
.clearfix { margin-top: 10px; }
that would move our buttons below the embedded video.
-
@NedFodder said in Video embed can overflow over other parts of the UI:
The problem is us.
Maybe we're adding the space in the wrong part of the box model?
-
@NedFodder said in Video embed can overflow over other parts of the UI:
.clearfix
Word of warning: that class is used on a LOT of elements.
-
@dkf said in Video embed can overflow over other parts of the UI:
@NedFodder said in Video embed can overflow over other parts of the UI:
Maybe we're adding the space in the wrong part of the box model?It appears so. The contents of the
small.pull-right
is overflowing it:edit: and all of the elements inside it are overflowing their boxes, too:
The padding isn't being counted in the size of the elements.
-
@ben_lubar said in Video embed can overflow over other parts of the UI:
@NedFodder said in Video embed can overflow over other parts of the UI:
.clearfix
Word of warning: that class is used on a LOT of elements.
That's why I said
@NedFodder said in Video embed can overflow over other parts of the UI:
something like
You have all the information you need to fix this if you wanted to.
-
@anotherusername said in Video embed can overflow over other parts of the UI:
@dkf said in Video embed can overflow over other parts of the UI:
@NedFodder said in Video embed can overflow over other parts of the UI:
Maybe we're adding the space in the wrong part of the box model?It appears so. The contents of the
small.pull-right
is overflowing it:edit: and all of the elements inside it are overflowing their boxes, too:
The padding isn't being counted in the size of the elements.
Adding
padding-top: 10px;
to thesmall.pull-right
fixes it.I believe I've also pointed out in other threads that the NodeBB's dev (mis)use of padding is a travesty all on it's own.
-
@ben_lubar said in Video embed can overflow over other parts of the UI:
@Lorne-Kates said in Video embed can overflow over other parts of the UI:
display:block
OMG WHAT A REVOLUTIONARY WEB IDEA!!!!!Yeah, wouldn't it be amazing if you checked right now and it already used
display:block
without anyone changing anythingWouldn't it be amazing if things that were
display:block
didn't overflow or get overflowed due to other shitty css being shitty?
-
@Lorne-Kates While the open source community was flailing their arms that Microsoft interpreted the CSS 1.0 spec wrong, none of them stopped to think that Microsoft's version made a hell of a lot more sense.
-
@blakeyrat said in Video embed can overflow over other parts of the UI:
@Lorne-Kates While the open source community was flailing their arms that Microsoft interpreted the CSS 1.0 spec wrong, none of them stopped to think that Microsoft's version made a hell of a lot more sense.
And two(ish) versions of CSS later, and UI devs still can't figure out what "padding" is. Or why it's a bad idea. Or how to make something look like something consistently.
-
@anotherusername ...okay, that may have been a red herring. Paragraph elements already don't overlap the post tools, because this rule gives them enough of a margin:
p { margin: 0 0 10px; }
So basically, anything inside
.content
needs a 10px margin under it, and currently, only<p>
elements have that. The elements that overlap the interface are<div>
elements. There should probably be another rule like this:.content > * { margin: 0 0 10px; }
@ben_lubar can you confirm this and add it?
-
@anotherusername well it looks like the video embed was fixed, but the rule is
.js-lazyYT { margin-bottom: 10px; }
If you're going that route, you also need to apply it to
.iframely-link
(example) and possibly other things too. That's why I thought it'd be simpler to just apply it to all of the direct children of.content
.
-
@anotherusername said in Video embed can overflow over other parts of the UI:
That's why I thought it'd be simpler to just apply it to all of the direct children of .content.
What about putting it on
.content
itself? Obviously, putting margins affects the things inside a post, but it seems like if the problem you're trying to solve is how the stuff in content is overlapping stuff outside of content...
-
@boomzilla that'd work, but the
p
elements already have a margin, so you'd double up on space if there was a.content > p:last-child
. You could use:.content { margin-bottom: 10px; } .content > *:last-child { margin-bottom: 0 !important; }
-
This bug was reported in the first week of nodebb.
-
Which topic is that? I want to see the Talkie and the comments about Talkie and his compulsion to discuss bread.
-
@anotherusername The CSS is in our custom stuff, and I've also added it to the less file in the youtube plugin. That's a work in progress. Hopefully I'll have something that can be shared next week (might get something out tomorrow, but with the holiday weekend, I don't expect to get anything done until Tuesday after that).
-
@anotherusername said in Video embed can overflow over other parts of the UI:
@boomzilla that'd work, but the
p
elements already have a margin, so you'd double up on space if there was a.content > p:last-child
. You could use:.content { margin-bottom: 10px; } .content > *:last-child { margin-bottom: 0 !important; }
What strange version of CSS are you using?
-
@ben_lubar huh? I didn't mean instead of the
p { margin-bottom: 10px; }
rule, I meant in addition to it.
-
@boomzilla said in Video embed can overflow over other parts of the UI:
and I've also added it to the less file
Wait, we're using
less
? I assumed we were usingscss
. Thecustom.css
file clearly wasn't css, and it passed through the scss compiler just fine. Apparently it's a polyglot.I'll see about fixing that in my latest PR...
-
@error Our site custom styling is CSS, but NodeBB and its plugins use less.
-
@boomzilla It certainly is not CSS. I found that out writing the unit tests. It was Less using .css extension.
It parses correctly as either Less or Scss (thus my confusion), but it does not parse correctly as CSS. CSS does not allow nesting of selectors.