Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the tiny gap under replaced elements that shows by default #14

Closed
jensimmons opened this issue Feb 5, 2019 · 29 comments · Fixed by #51
Closed

Fix the tiny gap under replaced elements that shows by default #14

jensimmons opened this issue Feb 5, 2019 · 29 comments · Fixed by #51

Comments

@jensimmons
Copy link
Owner

jensimmons commented Feb 5, 2019

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?

@jensimmons jensimmons self-assigned this Feb 5, 2019
@jensimmons jensimmons changed the title Two bits for images Two things about image styles Feb 5, 2019
@jensimmons jensimmons removed their assignment Feb 5, 2019
@danburzo
Copy link

danburzo commented Feb 5, 2019

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.)

@jensimmons jensimmons changed the title Two things about image styles Fix the tiny gap under images that shows by default Feb 10, 2019
@jensimmons
Copy link
Owner Author

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

@jensimmons
Copy link
Owner Author

jensimmons commented Feb 10, 2019

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
Copy link

tigt commented Feb 10, 2019

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
Copy link

mor10 commented Feb 11, 2019

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.

@Dangoo
Copy link

Dangoo commented Feb 11, 2019

@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
Copy link

tigt commented Feb 11, 2019

@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.

@jensimmons
Copy link
Owner Author

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
Copy link

tigt commented Feb 11, 2019

@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.

@jensimmons
Copy link
Owner Author

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.

@fantasai
Copy link
Contributor

@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.

@fantasai
Copy link
Contributor

@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.)

@uandco
Copy link

uandco commented Feb 12, 2019

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.

@jensimmons
Copy link
Owner Author

jensimmons commented Feb 12, 2019

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>

@jensimmons jensimmons reopened this Feb 12, 2019
@jensimmons jensimmons changed the title Fix the tiny gap under images that shows by default Fix the tiny gap under replaced elements that shows by default Feb 12, 2019
@fantasai
Copy link
Contributor

@uandco
Copy link

uandco commented Feb 13, 2019

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.

@jensimmons
Copy link
Owner Author

@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.

Repository owner deleted a comment from uandco Feb 14, 2019
Repository owner deleted a comment from uandco Feb 14, 2019
Repository owner deleted a comment from TalbotG Feb 14, 2019
Repository owner deleted a comment from uandco Feb 14, 2019
@ghost
Copy link

ghost commented May 27, 2020

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.

@Sora2455
Copy link

@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".

@BassOfBass
Copy link

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

thien-do pushed a commit to thien-do/cssremedy that referenced this issue Apr 30, 2021
The vertical-align for replaced elements is incorrectly warned by some editors built-in linting (afaik VSCode), saying it's useless when using with block display. However, this is completely expected: jensimmons#14

I just think that the comment is a little bit confusing at first. Mentioning the property name would help!
@lautapercuspain
Copy link

lautapercuspain commented May 31, 2021

 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.

@tranvansang
Copy link

tranvansang commented Jul 11, 2021

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.

@BassOfBass
Copy link

BassOfBass commented Jul 11, 2021

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.

LeoDog896 added a commit to deep-sea-tactics/robot that referenced this issue Nov 28, 2022
Add `vertical-align: middle` to align replaced elements more sensibly by default. (jensimmons/cssremedy#14 (comment))

   This can trigger a poorly considered lint error in some tools but is included by design.
Cata-hm referenced this issue in FarmaStack-Grupo05/Front-FarmaStack Apr 25, 2023
@MorningLit
Copy link

MorningLit commented Feb 7, 2024

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

luoxiaozero pushed a commit to thaw-ui/thaw that referenced this issue May 17, 2024
* Hi @luoxiaozero,

I've just noticed the same issue as described here (#190) and tracked it down to tailwindcss.  In the output.css generated by it we have:
```
/*
1. Make replaced elements `display: block` by default. (jensimmons/cssremedy#14)
2. Add `vertical-align: middle` to align replaced elements more sensibly by default. (jensimmons/cssremedy#14 (comment))
   This can trigger a poorly considered lint error in some tools but is included by design.
*/

img,
svg,
video,
canvas,
audio,
iframe,
embed,
object {
  display: block;
  /* 1 */
  vertical-align: middle;
  /* 2 */
}
```

Your recent change sets the display to inline-block (983e857) and this collides with the 'vertical-align' set by tailwindcss.

Changing the vertical-align property to 'top' fixes the misalignment so I think the simplest fix is to add 'vertical-align: top' to icon.css. I don't see it affecting anything else so it should be safe.

* icon missaligment fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.