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.

Sign up for GitHub

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

Jump to bottom

feat: add useBackgroundImage composable #1248

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

feat: add useBackgroundImage composable #1248

wants to merge 8 commits into from
+466 −4

Conversation

shunnNet
Copy link

@shunnNet shunnNet commented Feb 14, 2024 •
edited
Loading

Add a useBackgroundImage composable to enable the background to use the same functionality as <nuxt-img>.

Usage

<script setup lang="ts">

  const cls = useBackgroundImage('/images/nuxt.png', {
    provider: 'ipx',
    sizes: '200,500:500,900:900',
    modifiers: {
      width: 300,
      height: 300,
      // others...
    },
    densities: "1x 2x",
    preload: true,
    nonce: 'any nonce',
  })
// insert a <style> in <head>
/*
  <style id="nuxt-bg-Btzvq1">
    .nuxt-bg-Btzvq1 {
      background-image: url(/_ipx/s_900x900/images/nuxt.png);
      background-image: image-set(
         url('/_ipx/s_900x900/images/nuxt.png') 1x, 
         url('/_ipx/s_1800x1800/images/nuxt.png') 2x
      );
    }

    @media (max-width: 900px) {
      .nuxt-bg-Btzvq1 {
        background-image: url(/_ipx/s_500x500/images/nuxt.png);
        background-image: image-set(
           url('/_ipx/s_500x500/images/nuxt.png') 1x,
           url('/_ipx/s_1000x1000/images/nuxt.png') 2x
        );
      }
    }

    @media (max-width: 500px) {
      .nuxt-bg-Btzvq1 {
        background-image: url(/_ipx/s_200x200/images/nuxt.png);
        background-image: image-set(
           url('/_ipx/s_200x200/images/nuxt.png') 1x, 
           url('/_ipx/s_400x400/images/nuxt.png') 2x
         );
      }
    }
  </style>

*/

</script>
<template>
  <div
    :class="cls"
    :style="{
      width: '300px',
      height: '300px',
      backgroundSize: 'cover',
      backgroundRepeat: 'no-repeat',
      backgroundPosition: 'center'
    }"
  />
</template>

@shunnNet
Copy link
Author

Hello, I have added 3 new commits:

  • During SSR, because class names are randomly generated, it leads to duplicated insertion of style tags on both the server and client sides. Therefore, I attempted to fix this issue. Commit: bf2665d0
  • I initially thought there might be a similar duplication issue with preload, but it seems to occur only in dev mode. Therefore, I added a condition to check for this and removed that condition later.

@shunnNet
Copy link
Author

shunnNet commented Mar 6, 2024

@danielroe
Hello, I am a newcomer participating in an open-source project. I would like to understand why this Pull Request has not received any response. Did I make any mistakes somewhere?

@danielroe
Copy link
Member

My apologies for the slow review on this PR! I'm personally not sure this would be the right way to add this functionality. (Adding CSS generated in JS.)

Instead I think generating the image URL with useImage() and passing it in to a <style> block with a binding might be worth trying.

@shunnNet
Copy link
Author

First of all, thank you for your response.

I believe I should first explain the rationale behind this PR:

nuxt-img encapsulates the functionalities of html <img> and <picture> into a simple interface, so intuitively, I thought it should have a similar solution for background-image as well. However, the fact is that the documentation only suggests users manually generate src and place them either in v-bind:style or in a v-bind within a style block. If users want functionality similar to the <NuxtImg> component, they have to write scripts themselves to generate different versions of the image and set CSS accordingly.

Therefore, the purpose of this PR is to make setting background-image as convenient as NuxtImg. I believe it would make the package more complete. I think v-bind cannot address this situation.

However, you mentioned uncertainty about whether "Adding CSS generated in JS." is a good approach. I would like to understand more about your concerns regarding this idea. Could you please elaborate on why this might not be the correct approach and what your concerns are?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@shunnNet @danielroe