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

feat!: switch sizes default to responsive-first #977

Merged
merged 4 commits into from
Sep 13, 2023

Conversation

danielroe
Copy link
Member

@danielroe danielroe commented Sep 11, 2023

resolves #645

This aims to enable responsive sizing by default. It has two breaking changes:

  1. sizes without an explicit screen width (e.g. sizes="100vw") are interpreted as a responsive minimum size, e.g. 1px:100vw.
  2. Sizes are output from that screen-size up rather than that screen-size down, so sizes="md:100px lg:200px" means that the image is displayed at 100px by default. And at lg and above, at 200px. (Previously, this meant that it was displayed at 100px only up to md, and then displayed at 200px for all sizes above that.)

As these are both significant changes from previous behaviour, special care is requested in evaluating the API and DX.

@danielroe danielroe added the enhancement New feature or request label Sep 11, 2023
@danielroe danielroe requested a review from pi0 September 11, 2023 15:45
@danielroe danielroe self-assigned this Sep 11, 2023
@cloudflare-pages
Copy link

cloudflare-pages bot commented Sep 12, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 56087cf
Status: ✅  Deploy successful!
Preview URL: https://075ac7a8.nuxt-image.pages.dev
Branch Preview URL: https://feat-responsive-defaults.nuxt-image.pages.dev

View logs

@codecov-commenter
Copy link

codecov-commenter commented Sep 12, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.06% 🎉

Comparison is base (bd00755) 89.15% compared to head (56087cf) 89.22%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #977      +/-   ##
==========================================
+ Coverage   89.15%   89.22%   +0.06%     
==========================================
  Files          44       44              
  Lines        2914     2913       -1     
  Branches      323      325       +2     
==========================================
+ Hits         2598     2599       +1     
+ Misses        315      313       -2     
  Partials        1        1              
Files Changed Coverage Δ
src/runtime/image.ts 86.71% <100.00%> (+0.68%) ⬆️
src/runtime/utils/index.ts 76.86% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Atinux
Copy link
Member

Atinux commented Sep 12, 2023

It looks good to me.

What changes are required in the docs?

Copy link
Member Author

Will update 👍

@nuxt-studio
Copy link

nuxt-studio bot commented Sep 13, 2023

Live Preview ready!

Name Edit Preview Latest Commit
Image Edit on Studio ↗︎ View Live Preview 56087cf

Copy link
Member

@pi0 pi0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall looks a good idea to base on smaller sizes 👍🏼

@danielroe danielroe changed the title feat!: switch default to responsive-first sizes feat!: switch sizes default to responsive-first Sep 13, 2023
@danielroe danielroe merged commit fadb6ee into main Sep 13, 2023
3 checks passed
@danielroe danielroe deleted the feat/responsive-defaults branch September 13, 2023 15:32
@danielroe danielroe mentioned this pull request Sep 13, 2023
riddla pushed a commit to tricks-gmbh/nuxt-image that referenced this pull request Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow setting simple view-width based "sizes" values
4 participants