jensimmons/cssremedy

Fix the tiny gap under replaced elements that shows by default

jensimmons opened this issue · 29 comments

I was just reading Tantek's original Reset stylesheet (likely the first ever).

He wrote

/* whoever thought blue linked image borders were a good idea? */
a img,:link img,:visited img { border:none }

OMG! That's right. There were ugly blue boxes around images that have a link. Does this still happen anyplace? I don't think so. But if it does.... we should fix it. Ha.

Meanwhile, I do think images still have that little gap below them, because they are display: inline by default (mistake, imho), and the line height for whatever text size is added to the bottom of the box. I've spent many hours of my life trying to figure out where that gap is coming from. Until I finally remembered to always include this in my CSS:

img { display: block; }

Shall we add that to Remedy? Does anyone else always do this?

There was someone talking on Twitter a while ago about display: block on images as "treating a symptom", and offered an intentional solution to removing the gap... but I can't find it right now, will get back when I do!

Update: This is the tweet, and the solution is to use:

img { vertical-align: middle; }

(That being said, I do always display: block images.)

I just made a demo: https://codepen.io/jensimmons/pen/QYmmJK?editors=1100
You can see it on a page by itself (link might expire): https://s.codepen.io/jensimmons/debug/QYmmJK

I feel like vertical-align: middle changes the default too much — gets into a world where images could end up in unexpected places.

@fantasai concurs:

Honestly if you're not using an image as part of an inline layout, display: block makes perfect sense. Also 'middle' is a weird value, I'd pick top or bottom, neither of which invokes alignment to font metrics.

All of this does depend on the image. If it's a little icon, like the demo on the MDN page for vertical align, then vertical-align: middle is closer to what you want (although not it either, exactly).

After looking at this again for a while, I believe we have two choices.

img {
    display: block;
}

or

img {
   vertical-align: bottom;
}

Neither gives people their likely desired result for little inline icon images. Developers are going to need to write code for those usecases. There's simply too much variation in such moments to be able to guess what people want.

To me, this is a choice between

  1. Set people up for the thing they are doing most often — using images as blocks.
    or
  2. Stay as close as possible to the default CSS, keeping images inline, adjusting vertical-align to bottom in order to remove the mystery line spacing. That's Benjamin's argument in the tweet he wrote (referenced above, thanks @danburzo for bringing it up) — don't change CSS more than you mean to, just fix the bug you have.

To me, this isn't about correcting a bug, however. This is about finding a new best default. I expect that someone long ago debated whether to make images behave as (what we now call) inline or block. Maybe there was a public discussion. Maybe Marc Andreessen just decided. But if we were inventing the img element and its default CSS today, for the first time, I believe it would make more sense to set the default to block. My sense is that most of the time, images are in fact presented as a block-type object. The usecase where an image is set within a line of type — as part of that line of type — maybe we do that 10% of the time? This makes me lean towards:

img {
   display: block;
}
tigt commented

Twitter and WordPress use inline <img> for emoji — devices without the right fonts, new emoji that keep getting released, etc. Then there's the older phpBB/other forum software smileys, Twitch.tv emotes, and so on.

Firefox's reader mode has problems with them as a result of it trying to blockify images, so I would heavily recommend keeping images inline. Blockifying images works pretty naturally like this already:

<p>Blah blah blah…</p>

<img>

<p>Bloo bloo bloo…</p>

…which seems to be the simplest thing that could possibly work, markup-wise.


EDIT: I went digging and found other use-cases:

  • Icons next to usernames on Twitch, other forum/chat platforms like IRCCloud, MediaWiki usernames
  • Characters probably never will be supported by Unicode, like fictional/constructed languages such as Klingon, Elvish, etc.
  • Other new character polyfilling such as the new Russian currency symbol
  • Video game wikis love using them to mark which games something belongs to, prices in in-game currency, reusing in-game-text symbols like button prompts/elemental affinity symbols/character faces/item icons/etc.
  • GitHub itself uses inline images/icons for its “Styling with Markdown is supported” reminder when adding a comment, the “jensimmons changed the title Two things about image styles Fix the tiny gap under images that shows by default 2 hours ago” bit after @danburzo's comment, the tabs at the top of the page, the Participants avatars on the righthand rail, the Watch/Star/Fork buttons at the top, and so on. It may use flexbox or other layout methods for them under the hood, but they could just as easily be vertical-align for many of them.

So what?

While it's true that these use-cases are vastly outweighed by block images, my argument is that blockifying inline-by-default images is easy and just works by inserting them between two block-level elements — which happens 99% of the time naturally — but inlining block-by-default images is much more annoying and requires a class or fragile selectors.

mor10 commented

As I understand it, based on the original spec, the img element was meant to display inline icons and figures, much like how emoji are displayed inline today. However, this is not the "normal" use case for the img element today, nor has it been for a very long time. Imo the img element should be block level, and the developer can choose to set display:inline in the rare cases where inline images are needed.

@tight: inlining block-by-default images is much more annoying and requires a class or fragile selectors

I'd like to suggest using display: inline-block (e.g. like <button> or <select>) instead of just inline. This way you also gain the ability of using vertical margins and other block-like benefits.

As @tigt pointed out correctly, this still needs the vertical-align: bottom in order to fix the spacing issue.

What do you think? 🤔

And thats what a working draft of the spec mentions as "typical default"

img { display: inline-block; }
tigt commented

@Dangoo I’d be fine with that too, but I think inline-block still has the gap in question unless vertical-align is also set.

Just to be clear, this is not a discussion about changing the CSS specification or how browsers work. It is impossible to do so.

This is about CSS Remedy. If you don’t know what that is, please read the project page for this repo.

tigt commented

@jensimmons I’m sorry I was unclear. I’d like to keep display:inline/inline-block for ease of development with the aforementioned use-cases, even with Remedy applied.


EDIT: I am very much in favor of adding vertical-align:bottom/middle to solve the gap problem.

Thanks everyone for the input here. It's really fun to take a deep dive into such details, and try to figure out what the 'best' way, the best 'new default' for CSS could be.

I also posted this question to Twitter, where a lot of people responded. https://twitter.com/jensimmons/status/1094731679040712707

I'd been leaning towards img { display: block; } for a while... but the outcry on Twitter asking for block decided it for me. There are definitely good reasons to go with the other options. But blockifying images is the most common use case.

It's not surprising we don't all agree. That's the beautiful thing about having a starter-file, each of us can fork it and make it perfect for our particular way of doing things. Or override the default, setting things back to inline.

I'm going to close this issue now, I think we've uncovered all the important ideas in this conversation.

@jensimmons Put img, canvas, object, etc. { display: block; vertical-align: middle; }. That way it'll be block by default, which makes the most sense, but when someone turns an image inline, it'll default to middle alignment, which is most likely the one that's wanted for that use case.

@Dangoo Images are replaced elements, so inline and inline-block mean the same thing. (The block part of inline-block refers to how the element's insides are laid out, and for images CSS can't control that anyway.)

I agree on display: block, but you kept the vertical-align: bottom, is that in case you want to set the image back to inline? Seems like belt and braces now.

I did put vertical-align: bottom; in there for the folks who were saying (on Twitter) they struggle when resetting block back to inline. Of course a savvy developer would understand they should adjust the vertical-align, but well, most of us aren't that savvy.

Fantasai makes a good point, that it's likely people will want vertical-align: middle in that case. I'll change it to that. And yes, we should do this for all (?) replaced elements that are inline by default.

@fantasai are all replaced elements inline by default??

So what should we include here?

img, video, canvas, audio, iframe, embed, object  { display: block; vertical-align: middle; }

SVG?

Ok... lemme go look this up:

Phrasing content: 
 <abbr>, <audio>, <b>, <bdo>, <br>, <button>, <canvas>, <cite>, 
<code>, <command>, <data>, <datalist>, <dfn>, <em>, <embed>, 
<i>, <iframe>, <img>, <input>, <kbd>, <keygen>, <label>, <mark>, 
<math>, <meter>, <noscript>, <object>, <output>, <progress>, <q>, 
<ruby>, <samp>, <script>, <select>, <small>, <span>, <strong>, 
<sub>, <sup>, <svg>, <textarea>, <time>, <var>, <video>, <wbr> 

Embedded content: 
<audio>, <canvas>, <embed>, <iframe>, <img>, <math>, <object>, <svg>, <video>

I would still choose a camp and go display: block only, then let the ones that want to reset it back to inline deal with the alignment themselves, depending on their need.

@uandco @TalbotG

Hey, both of you, stop this. This project is not going to be a place for adversarial bickering. You’ve each advocated for your position. Now stop. No insulting each other.

I am teaching Web Intro this semester and last week I had a class on inline vs block... I literally thought that images were already block for a moment there so I'm voting for that to be the remedy. In my mind it's what they are anyway.

For the curious out there (like me!) do you mind explaining why it makes sense to add this rule just for img and video but not for the other replaced elements?

img, video {
  max-width: 100%; /* Make images and video flexible by default. */
  height: auto; /* Ensure images and video maintain their aspect ratio when max-width comes into play. */
}

Working on improving our base styles for Tailwind and learning a lot following the meticulous work in this project 👍

After some testing, I propose:

/*
* 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%;
}

/* 
* Maintain intrinsic aspect ratios when `max-width` is applied 
* (audio, iframe, embed, and object have no intrinsic ratios, set height explicitly)
*/
img, svg, video, canvas {
  height: auto;
}
tigt commented

I ran into an issue recently with img { vertical-align: middle } at work.

If an image breaks and falls back to its alt text, a vertical-align other than baseline produces different behavior across browsers. Here’s a CodePen to demonstrate.

  • Older (current?) versions of Firefox have the alt text dip below the baseline
  • As of Firefox Nightly 71.0a1, though, the changing of vertical-align appears to be ignored and the text flows on the same baseline, which seems ideal to me.
  • Chrome has the same problem as older Firefox, along with a broken image icon

Is it worth trying to fix this problem with selectors specifically for alt text? (Like img:is(:-moz-broken, :-moz-loading, :-moz-user-disabled))

@tigt Maybe open that as a new issue for discussion? I'm not sure how high priority that should be, but I'm interested in hearing people's thoughts.

img,
svg,
video,
canvas,
audio,
iframe,
embed,
object {
  display: block;
  vertical-align: middle;
}

VS Code prompts this warning:
Property is ignored due to the display. With 'display: block', vertical-align should not be used.

@web2033 Read the earlier comments - the vertical-align is there so that when people change their images to inline with an override style, the tiny gap under them is still gone by default.

The rule is basically "Block by default, vertical-align middle if not block".

Just add a comment above vertical align stating it's a fallback value for inlines.

 max-width: 100%; 

Hey @adamwathan , I just wanted to let you know that max-width: 100%; as default on an image element, breaks a development that relies on transform and translate3d, to move the focus point of the specific image into the wrapper.
When I remove max-width: 100%; from the img element, it works as expected.

It took me several hours to figure out that there is no bug in my library, just the default max-width: 100% comes from tailwind causes my layout to break. My project has no conflict with tw preflight.css so far. This img{max-width: 100%;} is the first one.

I believe that setting the layout of the img elements by the parent width is a bit too opinionated. It is perfect if there is an option to disable this.

I believe that setting the layout of the img elements by the parent width is a bit too opinionated. It is perfect if there is an option to disable this.

Using <img> outside of wrapper element IS the opinionated approach though, since replaceable elements behave inconsistently between browsers, especially when they are the part of flexbox/grid.

Not sure if anyone is also facing tons of warnings from using Next.js with TailwindCSS (tailwind uses preflight), but with these settings on images,

img, video {
    max-width: 100%;
    height: auto;
}

it causes images to be "responsive" based on the image's height, which can be ideal for some developers.

However, Next.js has some optimisations for images built in-place and it needs images to be explicitly set a width and a height, so I'm unsure which option to follow.
I'm slightly leaning onto Next.js's side, because these settings tend to produce unexpected results if your picture is in portrait mode and you have already set a desired height for it. (The image will overflow the height that you have set)
Even after reading this thread, I can't really find a compelling reason to use this CSS rule as a form of CSS reset.

img, video {
  max-width: none;
  height: revert-layer;
}

Here's what I did to revert the changes for this CSS rule. (By also explicitly defining width and height on the tag) You might also want to add object-fit: contain to prevent the stretching of images.

Relevant thread where people are also facing a similar issue: tailwindlabs/tailwindcss#506
Would recommend to reopen this issue again and discuss it