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

[gatsby-plugin-image] GatsbyImage component doesn't respect 'draggable' attribute #30666

Closed
smashercosmo opened this issue Apr 3, 2021 · 9 comments
Labels
topic: media Related to gatsby-plugin-image, or general image/media processing topics

Comments

@smashercosmo
Copy link

Description

GatsbyImage component doesn't respect 'draggable' attribute (this has been reported before and has been fixed for the previous gatsby image component #4631)

Steps to reproduce

  1. Try to add 'draggable' attribute to GatsbyImage component
<GatsbyImage image={...} draggable={false} />
  1. Check the html source. You'll see that 'draggable' attribute hasn't been added to any tag.

Expected result

'draggable' attribute is added to <picture> tag (not <img> tag!)

Actual result

'draggable' attribute is not added at all

Environment

System:
OS: macOS 10.15.7
CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
Shell: 5.7.1 - /bin/zsh
Binaries:
Node: 12.21.0 - /usr/local/opt/node@12/bin/node
Yarn: 1.22.10 - /usr/local/bin/yarn
npm: 6.14.11 - /usr/local/opt/node@12/bin/npm
Languages:
Python: 2.7.16 - /usr/bin/python
Browsers:
Chrome: 89.0.4389.114
Firefox: 78.8.0
Safari: 14.0.3
npmPackages:
gatsby: 3.2.1 => 3.2.1
gatsby-plugin-image: 1.3.0-next.1 => 1.3.0-next.1
gatsby-plugin-manifest: 3.2.0 => 3.2.0
gatsby-plugin-netlify-cms: 5.2.0 => 5.2.0
gatsby-plugin-postcss: 4.2.0 => 4.2.0
gatsby-plugin-react-helmet: 4.2.0 => 4.2.0
gatsby-plugin-sharp: 3.2.0 => 3.2.0
gatsby-plugin-sitemap: 3.2.0 => 3.2.0
gatsby-plugin-svgr: 3.0.0-beta.0 => 3.0.0-beta.0
gatsby-plugin-typescript: 3.2.0 => 3.2.0
gatsby-source-filesystem: 3.2.0 => 3.2.0
gatsby-transformer-json: 3.2.0 => 3.2.0
gatsby-transformer-remark: 3.2.0 => 3.2.0
gatsby-transformer-sharp: 3.2.0 => 3.2.0
gatsby-transformer-yaml: 3.2.0 => 3.2.0

@smashercosmo smashercosmo added the type: bug An issue or pull request relating to a bug in Gatsby label Apr 3, 2021
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Apr 3, 2021
@LekoArts LekoArts added topic: media Related to gatsby-plugin-image, or general image/media processing topics and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Apr 6, 2021
@ascorbic ascorbic added help wanted Issue with a clear description that the community can help with. good first issue Issue that doesn't require previous experience with Gatsby labels Apr 6, 2021
@ascorbic
Copy link
Contributor

ascorbic commented Apr 6, 2021

This would be a thing worth adding. It would just need a change in components/picture.tsx to destructure draggable and apply it to the <picture> element.

@ascorbic ascorbic added type: feature or enhancement Issue that is not a bug and requests the addition of a new feature or enhancement. and removed type: bug An issue or pull request relating to a bug in Gatsby labels Apr 6, 2021
@ascorbic
Copy link
Contributor

ascorbic commented Apr 6, 2021

OK, I thought I should test this and it already works. I was surprised by your assertion that it needed to be applied to the <picture> element, because nothing else should be attached to it. As I thought, attaching it to <img> works fine. Tested in Chrome and FF. Because we already pass all props through to the img tag, adding draggable={false} to the StaticImage or GatsbyImage component works as expected.

@ascorbic ascorbic added status: needs reproduction This issue needs a simplified reproduction of the bug for further troubleshooting. and removed good first issue Issue that doesn't require previous experience with Gatsby help wanted Issue with a clear description that the community can help with. type: feature or enhancement Issue that is not a bug and requests the addition of a new feature or enhancement. labels Apr 6, 2021
@smashercosmo
Copy link
Author

@ascorbic that's reaaaaaaallyyyy strange. Let me recheck.

@smashercosmo
Copy link
Author

Alright, @ascorbic, I was wrong about draggable not being added at all. It's indeed being added to the img tag. But unfortunately it doesn't have any effect (you're still able to drag images), unless it's added to the picture tag.

@stldo
Copy link

stldo commented Apr 6, 2021

I confirm that draggable is being added to <img> element and, at least on Chrome, it only works if added to the <picture> element. According to MDN, the picture element accepts only global attributes, and draggable is a global attribute. On the other hand, the previous gatsby-image plugin attaches the draggable attribute to the <img> element, and it works as expected.

@ascorbic
Copy link
Contributor

ascorbic commented Apr 7, 2021

I'm convinced this is a browser bug, because every other attribute is attached to the img tag instead. However I can reproduce it, and attaching it to the picture tag does work, so I've opened a PR to attach it to both.

@stldo
Copy link

stldo commented Apr 7, 2021

It should be a browser bug, but most likely there's a CSS property causing this issue. It is present in gatsby-plugin-image, while gatsby-image works fine. Also, I've never seen this issue before, neither in React nor plain JavaScript.

@stldo
Copy link

stldo commented Apr 7, 2021

The .gatsby-image-wrapper img selector has the property all: inherit set. Disabling it makes dragabble="false" set on the <img> element to work as expected. It doesn't make much sense setting the property all to inherit on an <img> element, if it wasn't set for other purposes, removing it should be a better approach than setting draggable to false in the <picture> element.

@ascorbic
Copy link
Contributor

ascorbic commented Apr 8, 2021

Ah, good find! You're right that doesn't make much sense anymore, though I'll need to see if this causes regressions anywhere

@ascorbic ascorbic removed the status: needs reproduction This issue needs a simplified reproduction of the bug for further troubleshooting. label Apr 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: media Related to gatsby-plugin-image, or general image/media processing topics
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants