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

Image component with 'inline-block' adds text padding #18915

Closed
nicholaschiang opened this issue Nov 7, 2020 · 10 comments
Closed

Image component with 'inline-block' adds text padding #18915

nicholaschiang opened this issue Nov 7, 2020 · 10 comments
Labels
bug Issue was opened via the bug report template.

Comments

@nicholaschiang
Copy link
Contributor

Bug report

Describe the bug

The new <Image /> component in Next.js 10.0.1 has display set to inline-block which leads to what seems like extra margin on the bottom of the <div /> like so (see the red part):

image

This is what that component (pictured above) looks like:

export default function Avatar({
  src = '',
  loading = false,
  size = 500,
}: AvatarProps): JSX.Element {
  const { t } = useTranslation();

  return (
    <div className={cn(styles.wrapper, { [styles.loading]: loading })}>
      {!loading && !!src && !validPhoto(src) && (
        <div className={styles.photoWrapper}>
          <img className={styles.photo} data-cy='avatar' src={src} alt='' />
        </div>
      )}
      {!loading && !!src && validPhoto(src) && (
        <Image
          data-cy='avatar'
          layout='fixed'
          height={size}
          width={size}
          src={src}
          alt=''
        />
      )}
      {!src && !loading && (
        <div className={styles.noPhotoWrapper}>
          <div className={styles.noPhoto}>{t('common:no-photo')}</div>
        </div>
      )}
    </div>
  );
}

Pretty basic, but the <Image /> component adds that bit of red margin. To fix this, I've followed some StackOverflow advice and added the following styles to the wrapper <div />:

.wrapper {
  letter-spacing: 0;
  word-spacing: 0;
  font-size: 0;
}

To Reproduce

Steps to reproduce the behavior, please provide code snippets or a repository:

  1. Use the new <Image /> component in any <div /> wrapper.
  2. Set a background color on that <div /> wrapper (so you can see the "margin").
  3. Notice how there's a tiny "margin" between the bottom of the image and container.

Expected behavior

The Next.js image component is very opinionated when it comes to styling IMO (unlike other image optimization libraries I've used in the past). This is the first time (I think) that Next.js actually adds opinionated styling to an app which can get really annoying when those styles update and you've got to update all of your app's styling to work with the new Next.js <Image /> component styles.

IMO, the <Image /> component should work out-of-box as a drop-in replacement for the HTML <img /> tag (i.e. the styling should behave in exactly the same way so existing styles work fine with it). When I started replacing img tags with the new <Image /> component, I had to update a bunch of my styling to work with the way Next.js styles that image (related to #18871). And then, I updated to Next.js 10.0.1 and all those styles broke again!

I'm not exactly sure the best way to go about fixing this, but you definitely need to address those styling issues.

System information

  • OS: PopOS 20.10
  • Browser (if applies) Firefox
  • Version of Next.js: 10.0.1
  • Version of Node.js: 12.18.3
@nicholaschiang nicholaschiang added the bug Issue was opened via the bug report template. label Nov 7, 2020
nicholaschiang added a commit to tutorbookapp/tutorbook that referenced this issue Nov 7, 2020
@Timer
Copy link
Member

Timer commented Nov 9, 2020

This mimics the default behavior of the <img /> component (which also has the text padding), so it's expected.

Closing as a duplicate of #18637, please voice any new layout requests there!

@NeryC
Copy link

NeryC commented Jul 14, 2021

that is not resolving that issue

@jaspersorrio
Copy link

jaspersorrio commented Jul 14, 2021

Due to this behaviour, we are forced to add extra html tags to wrap around next/image causing an excessive DOM size.

If possible, please look into this as it has become antithesis to what next.js is trying to achieve.

Images are always rendered in such a way as to avoid Cumulative Layout Shift, a Core Web Vital that Google is going to use in search ranking.

Issue #21914 describes this behaviour well.

@mxmzb
Copy link

mxmzb commented Oct 12, 2021

This mimics the default behavior of the <img /> component (which also has the text padding), so it's expected.

Closing as a duplicate of #18637, please voice any new layout requests there!

While this is true, we can add a display: block; to normal <img /> tags though, which does not work with next/image

@robclancy
Copy link

@Timer how is this a duplicate of that other issue? And how is this anything like default when you can put any style on it you want.
This issue should be reopened.

michaelperrin added a commit to michaelperrin/randonavigo that referenced this issue Dec 16, 2021
@michaelperrin
Copy link

Only way I found to fix this is to force the height on the parent div element, which is not ideal:

<div style={{ height: `${height}px` }}>
  <Image
    height={height}
    width={width}
    src="/picture.png"
    alt=""
  />
</div>

@vnoitkumar
Copy link

As mentioned in the Next.js Doc (https://nextjs.org/docs/basic-features/image-optimization) "When using layout='responsive', the parent element must have display: block".

This solved the problem for me.

@leerob
Copy link
Member

leerob commented Jan 4, 2022

Feedback we've received here:

I've tried all of the solutions outlined and refactored my implementation so I could better target the element, and I still get this painful inline-block that prevents me from styling the rest of my page accordingly. Thinking about rotating the inner div by 180 and then the outer component by another 180 just to remove the 1px margin-bottom, reverted to native img for now.

@robclancy
Copy link

It got changed to a span wrapper instead at some point. But here is the code we use to "fix" this issue (that should not be closed).

.image-block-hack > span {
  display: block !important;
}

.image-h-full-hack > span {
  height: 100% !important;
}

And it means for any image we need to be block or 100% height we have to wrap them with a div that uses one of these classes. And if we want other styles we will just have to keep adding these hacks.

@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Feb 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template.
Projects
None yet
Development

No branches or pull requests

10 participants