gatsbyjs / gatsby

The best React-based framework with performance, scalability and security built in.
https://www.gatsbyjs.com
MIT License
55.27k stars 10.31k forks source link

gatsby-image: Add the ability to have className on actual img element #24159

Closed hbish closed 3 years ago

hbish commented 4 years ago

Summary

I have a requirement to add className to the actual <img> element to support microformats.

Currently GatsbyJs only has

There is no support for the actual <img> element.

Basic example

<Img
  fluid={data['profilePic'].childImageSharp.fluid}
  alt={`Ben Shi`}
  className={'avatar'}
  imgClassName={'u-photo'}
/>

Motivation

In microformats u-photo is only valid on (<img src>) elements. gatsby-image both fixed and fluid does not support this. http://microformats.org/wiki/microformats-2-implied-properties

hbish commented 4 years ago

@LekoArts apologies for jumping the gun and raising a PR on this.

Happy to wait and see what the Gatsby team thinks of this small enhancement and will resubmit the PR once it has been approved. Cheers!

wardpeet commented 4 years ago

Hey @hbish,

I get the issue, adding another prop is painful as it's already so bloated. Wouldn't it be possible to do css nesting?

With sass:

.imageWrapper {
  img {
    @apply .u-photo
  }
}

Love to hear your response, thank you!

hbish commented 4 years ago

@wardpeet Thanks for the suggestion. I agree that the component is already bloated but its probably the cleanest/simplest solution.

While I think I could get it working by introducing postcss but it feels like overkill and also a hack as I would have to write a pretty nested rule like below. This is because the <img> immediately follows the wrapper is the placeholder and not the actual <img>.

.avatar {
    picture {
      img {
        @apply .u-photo
      }
    }
  }

Another consideration with @apply, is that the noscript block would not have the class applied so a separate rule would be needed.

Finally there is also an imgStyle prop that gets passed down to the actual image already so it feels like this is may have been an oversight initially as applying style directly is not as repeatable as using the className.

Thoughts?

amingilani commented 4 years ago

I just want to add that Bulma images also require an is-rounded applied directly to the image to function correctly. I'm certain other frameworks require similar things as well.

alexdkaminski commented 4 years ago

Also need this to get a Tailwind UI component to work properly.

isaac-martin commented 4 years ago

We would like to be able to pass down the css generated by emotion to the img element and it is currently not possible with the current gatsby image component. Any update - to me it looks like the change suggested in #24161 would solve this?

MarkBennett commented 4 years ago

PR #24161 is currently closed pending discussion in an issue. What further dialog is needed to moved this issue forward?

As it stands the acceptance criteria is clearly layed out by the initial author of this issue. Are there technical considerations that need discussion? Docs or tests needing updating? I'd be happy to contribute as well.

rustyconover commented 4 years ago

I'd like to see PR #24161 merged as well, like @amingilani stated Bulma images need this for using is-rounded.

steffik02 commented 4 years ago

Just thought I'd add that I need this functionality as well, or the ability to pass data attributes to the tag. I'm trying to add an attribute to prevent pinning to Pinterest with certain images and I can't get the attribute on the actual img tag. If I could at least get a class name on there, then I could add the attribute with Javascript.

robinske commented 3 years ago

+1 to @steffik02's comment - many features of the Pinterest save button widget won't work without the ability to set custom attributes on the image itself. I'd love to be able to update the Pinterest plugin to support those attributes and the ability to set data attributes on an image would be the simplest way.

juanmartin commented 3 years ago

Any news on this discussion? I also need it to inject Tailwind classes directly on the img tag. In my case I want to change the object-fit property which works only applied to the img tag not the parent div that is returned by <Img /> . It would be great to have imgClassName available!

hbish commented 3 years ago

Given there havent been much response from the gatsby team, its probably worthwhile checking out the new gatsby-plugin-image #27950 and leave your feedback there to see if the new plugin meet your needs.

LekoArts commented 3 years ago

Hi, sorry for the silence here. As our API on gatsby-image is a bit convoluted and it wasn't the top-top request this one slipped through a bit. However, the new gatsby-plugin-image which is currently in beta added this feature a week ago: https://github.com/gatsbyjs/gatsby/pull/28520

So please give it a try and leave feedback in https://github.com/gatsbyjs/gatsby/discussions/27950 - for gatsby-image we won't backport this. Thanks!