Skip to content
New issue

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

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

Already on GitHub? # to your account

[@astrojs/image] Ability to add a class to <picture> element #5476

Closed
1 task done
jkjustjoshing opened this issue Nov 25, 2022 · 5 comments
Closed
1 task done

[@astrojs/image] Ability to add a class to <picture> element #5476

jkjustjoshing opened this issue Nov 25, 2022 · 5 comments
Assignees
Labels
- P2: nice to have Not breaking anything but nice to have (priority) pkg: image Related to the `@astrojs/image` package (scope)

Comments

@jkjustjoshing
Copy link
Contributor

What version of astro are you using?

1.6.11

Are you using an SSR adapter? If so, which one?

None

What package manager are you using?

npm

What operating system are you using?

Mac

Describe the Bug

Problem

Can't lay out <Picture> inside a CSS grid container

Reason

Before @astrojs/image 0.11.0, adding a class prop to <Picture>, the class would be put on the internal HTML <picture> element. If I wanted to style the <picture> element I could refer to the class, and if I wanted to style the underlying <img> element I could refer to .myClass img. This allowed me to put most of the styles on the <img> element, but add grid-area/grid-row/grid-column CSS properties on the <picture> element. Putting those CSS properties on the <img> element don't do anything.

Proposed solution

Keep the class prop on the <img>, but add a pictureClass prop that puts the class name on the <picture> element.

Possible workaround

Set picture { display: contents; } as a global style in my project. I can't find this suggested anywhere on the internet, and I hesitate to do this without exploring if there are any downsides to this - does it break how <picture> natively works or change the accessibility semantics in a negative way?

Link to Minimal Reproducible Example

N/A

Participation

  • I am willing to submit a pull request for this issue.
@bluwy
Copy link
Member

bluwy commented Nov 28, 2022

Another workaround would be something similar to #5481 (comment)

@matthewp matthewp added - P2: nice to have Not breaking anything but nice to have (priority) pkg: image Related to the `@astrojs/image` package (scope) labels Dec 6, 2022
@Princesseuh Princesseuh self-assigned this Jan 25, 2023
@eostrom
Copy link

eostrom commented Jan 31, 2023

I ran into this too. In my case, I'm using a Flexbox to lay out a stack of text elements, and in the Next app I'm porting from, I had used the picture's class to position it absolutely as a background for the text. When Astro applied the class to the image instead, the absolute positioning worked, but my text was shifted down, because the picture itself took up a cell of the Flexbox.

Using display: contents, as suggested, didn't really solve my problem, as I now had three source elements taking up cells instead of the containing picture. I was able to work around it by hiding them:

picture {
  display: contents;

  source {
    display: none;
  }
}

... but I'd rather add a class to the element in question.

If pictureClass is the desired approach, I'd be happy to work on a PR for it.

@xav-ie
Copy link

xav-ie commented Feb 11, 2023

My workaround was to wrap the <Picture/>/<Image/> special elements in a wrapper <div/> and set that to have {display:contents;} but as mentioned in #5481 (comment) , you will have to make sure <style is:global>... :(. Then, I just styled the image inside the wrapper div with this selector: div img {}. Oddly enough, I had to set some properties to be !important... :/. Worth it for the nice blurred effect hehe. Unfortunately, that effect is still pretty jank ughhhhhdkasjdfhaslkdfjaldkfj

@Princesseuh
Copy link
Member

Hello! @astrojs/image is heading towards maintenance mode in favour of the new astro:assets feature. As such, we won't be adding new features to it. We are open to contributors sending PRs for this however. Sorry for the inconvenience.

@isc30
Copy link

isc30 commented Feb 20, 2025

The best workaround i could find:

picture { display: contents; }
source { display: none; }

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
- P2: nice to have Not breaking anything but nice to have (priority) pkg: image Related to the `@astrojs/image` package (scope)
Projects
None yet
Development

No branches or pull requests

7 participants