jensimmons / cssremedy

Start your project with a remedy for the technical debt of CSS.
Mozilla Public License 2.0
2.2k stars 113 forks source link

`display:block` negates `vertical-align` #76

Closed thierryk closed 3 years ago

thierryk commented 3 years ago

I believe vertical-align applies to (see Formal Definition on MDN):

inline-level and table-cell elements. It also applies to ::first-letter and ::first-line.

So I doubt it does anything in the rule below as it is associated with display:block:

/*
* 1. Block display is usually what we want
* 2. Remove strange space-below when inline
* 3. Responsive by default
*/
img, svg, video, canvas, audio, iframe, embed, object {
  display: block;
  vertical-align: middle;
  max-width: 100%;
}

display: block alone should take care of #2 ("Remove strange space-below when inline")

Malvoz commented 3 years ago

I believe vertical-align: middle is used here to have the gap removed irrespectively of the display value, in case the author changes it.

mirisuzanne commented 3 years ago

That's right. We decided to provide a fallback vertical-alignment as a fallback for inline use-cases, when authors override the display value.

thierryk commented 3 years ago

What about adding that piece of info to the comment then?

* 2. Remove strange space-below in case authors overwrite the display value

FWIW: I didn't know this project and looking at those 2 lines made me wonder...

mirisuzanne commented 3 years ago

@thierryk I like that as a clarification. Do you want to make a PR?

thierryk commented 3 years ago

@mirisuzanne sure!

thierryk commented 3 years ago

PR https://github.com/jensimmons/cssremedy/pull/77 will close this