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

Responsive Images #124

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

Responsive Images #124

wants to merge 6 commits into from

Conversation

micmro
Copy link
Contributor

@micmro micmro commented Jul 19, 2023

Largest resolution

Screenshot 2023-07-19 at 4 47 28 PM

Smaller resolution

Screenshot 2023-07-19 at 4 48 05 PM

Also added batching of asset download to work around rate-limiting issues.

@codesandbox
Copy link

codesandbox bot commented Jul 19, 2023

This branch is running in CodeSandbox. Use the links below to review this PR faster.


CodeSandbox logoCodeSandbox logo  Open in CodeSandbox Web Editor | VS Code | VS Code Insiders | Preview

...metadata,
}))
assets.map(({ value, ...rest }, i) =>
batchDelay(i)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

batch asset requests to avoid upstream (likely rate-limiting) timeouts.
Screenshot 2023-07-19 at 10 26 58 AM

@github-actions
Copy link

Demo App Preview: https://d11mwnosflssyy.cloudfront.net/

@github-actions
Copy link

github-actions bot commented Jul 19, 2023

Code Coverage Report

Coverage after merging batch-asset-download into main

87.07%
Coverage Report
FileBranchesFuncsLinesUncovered Lines
../core-components/dist
   index.js66.97%73.33%68.42%..., 90, 91, 92, 93
../foundations
   generator-config.ts100%100%100%
../foundations/generated
   design-tokens.constants.ts100%100%100%
../foundations/generated/icons
   account-circle.tsx100%100%100%
   arrow-left.tsx100%0%75%4
   arrow-right.tsx100%100%100%
   chevron-right.tsx100%0%75%4
   close.tsx100%0%75%4
   dark-mode.tsx100%0%75%4
   event-note.tsx100%0%75%4
   figma.tsx100%100%100%
   github.tsx100%100%100%
   index.ts100%41.18%100%
   instagram.tsx100%100%100%
   light-mode.tsx100%0%75%4
   linked-in.tsx100%100%100%
   menu.tsx100%100%100%
   schedule.tsx100%0%75%4
   settings.tsx100%0%75%4
   twitter.tsx100%0%75%4
   youtube.tsx100%0%75%4
../foundations/src/helpers
   src-to-src-set.ts100%50%55.56%11, 12, 13, 16
../shared/constants
   index.ts100%100%100%
   media-queries.ts100%100%100%
../shared/utils
   design-tokens.utils.ts100%100%100%
   flatten.utils.ts0%0%23.08%..., 32, 9, 9, 9
   index.ts100%100%100%
   styles.utils.ts100%100%100%
components/button
   button.styles.ts0%100%100%13
   button.tsx100%100%100%
   index.ts100%100%100%
components/footer
   footer.styles.ts100%100%100%
   footer.tsx100%100%100%
   index.ts100%100%100%
components/hero
   hero.styles.ts50%100%100%10
   hero.tsx100%100%100%
   index.ts100%100%100%
components/image-text-item
   image-text-item.styles.ts100%100%100%
   image-text-item.tsx100%100%100%
   index.ts100%100%100%
components/image-text-list
   image-text-list.tsx100%100%100%
   index.ts100%100%100%
components/inline-link
   index.ts100%100%100%
   inline-link.styles.ts50%100%100%13, 19, 9
   inline-link.tsx66.67%100%100%44, 51
components/link-button
   index.ts100%100%100%
   link-button.styles.ts50%100%100%14, 19, 8, 9
   link-button.tsx83.33%100%100%45
components/link-icon
   index.ts100%100%100%
   link-icon.styles.ts50%100%100%14, 18, 24
   link-icon.tsx66.67%100%100%43, 52
components/nav
   index.ts100%100%100%
   nav.styles.ts50%100%100%14, 38, 55
   nav.tsx66.67%80%95%87, 89, 91
components/nav-item
   index.ts100%100%100%
   nav-item.styles.ts100%100%100%
   nav-item.tsx100%100%100%

@github-actions
Copy link

github-actions bot commented Jul 19, 2023

🎨 Chromatic Results 🎨
Chromatic URL https://www.chromatic.com/build?appId=63cabc3ddcbcfa97ebf8e624&number=222
Storybook Preview https://63cabc3ddcbcfa97ebf8e624-oegsnhzxhu.chromatic.com/
Component Count 16

@nx-cloud
Copy link

nx-cloud bot commented Jul 19, 2023

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 2aa6bfc. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 2 targets

Sent with 💌 from NxCloud.

@@ -0,0 +1,5 @@
/** Customizable Configuration for the Generator */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This config can be reused to configure the generator based on project needs, e.g. as an alternative to special tokens

Comment on lines +9 to +10
"src/",
"./generator-config.ts"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

publish config

Comment on lines +48 to +59
.resize(
width === 'full' // full res version
? undefined
: {
// resize to max-size, this might sometimes create duplicates,
// if the source image is small, but this makes the
width: width,
height: width,
fit: 'inside',
withoutEnlargement: true,
}
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

resize based on config and combine with full-res version

Comment on lines -78 to +79
src={src}
{...srcToSrcSet(src)}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use helper to auto-generate sourceset

@micmro micmro changed the title Batch asset download Responsive Images Jul 19, 2023
brenzy
brenzy previously approved these changes Jul 20, 2023
@brenzy brenzy dismissed their stale review May 21, 2024 14:36

Michael was unhappy with something in this commit. Unfortunately I have no recollection of what the issue was.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants