sveltejs / kit

web development, streamlined
https://svelte.dev/docs/kit
MIT License
18.73k stars 1.95k forks source link

`@sveltejs/enhanced-img`: cleaner support for light and dark mode image src's #11096

Open jasongitmail opened 11 months ago

jasongitmail commented 11 months ago

Describe the problem

When showing different images for light and dark mode, using CSS like below cause multiple HTTP requests, one per image, even for the image that is not displayed. This is expected behavior and not a problem.

<enhanced:img
  class="dark:hidden"
  alt="App screenshot"
  src="$lib/assets/images/screenshot.png"
/>

<enhanced:img
  class="hidden dark:block"
  src="$lib/assets/images/screenshot-dark.png"
  alt="App screenshot"
/>

The solution is to use a reactive variable like this:

<script lang="ts">
  import screenshotDark from '$lib/assets/images/screenshot-dark.png?enhanced';
  import screenshotLight from '$lib/assets/images/screenshot.png?enhanced';
  let isDark = false; // just for the demo
  $: screenshot = isDark ? screenshotDark : screenshotLight;
</script>

<button on:click={() => (isDark = !isDark)}>Toggle image</button>

<enhanced:img
  alt="App screenshot"
  src={screenshot}
/>

This works and changes the screenshot when the button is clicked, but is a fair amount of code per image.

Describe the proposed solution

It would be nice if enhanced:img supported a syntax like this src attribute too:

<script lang="ts">
  $: isDark = false; // just for the demo
</script>

<button on:click={() => (isDark = !isDark)}>Toggle image</button>

<enhanced:img
  alt="App screenshot"
  src={isDark ? '$lib/assets/images/screenshot-dark.png' : '$lib/assets/images/screenshot.png'}
/>

Alternatives considered

No response

Importance

nice to have

Additional Information

No response

benmccann commented 11 months ago

One option would be a new rune like:

<enhanced:img
  alt="App screenshot"
  src={isDark ? $import('$lib/assets/images/screenshot-dark.png') : $import('$lib/assets/images/screenshot.png')}
/>
jasongitmail commented 11 months ago

A syntax that doesn't introduce a new concept, like $import(), would seem preferable imo. For example:

<enhanced:img
  alt="App screenshot"
  src={isDark ? '$lib/assets/images/screenshot-dark.png' : '$lib/assets/images/screenshot.png'}
/>

At least for the two most common use cases:

  // single source
  src={'$lib/assets/images/screenshot.png' }
OR
  // two sources using a ternary
  src={isDark ? '$lib/assets/images/screenshot-dark.png' : '$lib/assets/images/screenshot.png'}

These could still be transformed by the compiler into an implementation that is backed by $import(), if needed. But hidden from the dev in the common cases. $import() could exist to support more complex logic beyond ternaries (but that'd probably be so rare as to maybe be worth punting on until there is demand shown for it).

benmccann commented 11 months ago

I think it'd get hard to understand which syntaxes we support and which we don't

jarrodldavis commented 8 months ago

I think a better solution would be to use the existing media query support that <source> already has. Could enhanced-img support passing in additional sources with associated media queries? Something like:

<enhanced:img
  alt="Logo"
  src="./logo-normal.png"
  media={{
    "(prefers-color-scheme: dark)": "./logo-dark.png"
  }}
/>

could become this:

<picture>
  <source
    srcset="/@imagetools/047cecc46e1d3a215cc189025122dae3db2c6f88 1x, /@imagetools/ecb7cafcef20bcf657a06e756eb5174b9b887530 2x"
    type="image/avif"
    media="(prefers-color-scheme: dark)"
  >
  <source
    srcset="/@imagetools/cdb38850b7f2728f9efa3ffec5acf8449f766200 1x, /@imagetools/87656fe026ecf3c39ffceb1b65b67af0ca1bcde5 2x"
    type="image/webp"
    media="(prefers-color-scheme: dark)"
  >
  <source
    srcset="/@imagetools/5b1992f592b1465628e5fbd393dc4ff215252afa 1x, /@imagetools/58bc1dcd5300bac26d21039fb3f56b9adf87dc82 2x"
    type="image/png"
    media="(prefers-color-scheme: dark)"
  >
  <source
    srcset="/@imagetools/d63c5f34def1365fac10888f3d598ba15970d695 1x, /@imagetools/715ee330fa944bd8b9008e26b1fe9eb29cf44956 2x"
    type="image/avif"
  >
  <source
    srcset="/@imagetools/3ca27dc388a8ded1b827b1b6d08161c55ab28d9a 1x, /@imagetools/3ae5331aa43d226fffdafc43701d6dc28d3170bd 2x"
    type="image/webp"
  >
  <source
    srcset="/@imagetools/414e774cb2f77bda747041e30501e23526ae6332 1x, /@imagetools/507af6054c0a3baa3d096142a2a6b4598e9380d4 2x"
    type="image/png"
  >
  <img alt="Logo" src="/@imagetools/507af6054c0a3baa3d096142a2a6b4598e9380d4" width="256" height="256">
</picture>
Skaldebane commented 8 months ago

@jarrodldavis in that case, wouldn't it be better to have a <enhanced:picture> element to keep things consistent with vanilla HTML 🤔

SapiensAnatis commented 2 months ago

This would be really nice to have. I also think media queries are best for this - as far as I know there are difficulties getting an isDarkMode variable during server rendering (or at least mode-watcher will return undefined).