Skip to content

docs: adding recipe for dynamically importing Images#5391

Merged
sarah11918 merged 15 commits into
withastro:mainfrom
jdwilkin4:feat/dynamic-image-imports
Nov 25, 2023
Merged

docs: adding recipe for dynamically importing Images#5391
sarah11918 merged 15 commits into
withastro:mainfrom
jdwilkin4:feat/dynamic-image-imports

Conversation

@jdwilkin4
Copy link
Copy Markdown
Contributor

@jdwilkin4 jdwilkin4 commented Nov 13, 2023

Deploy preview link

https://docs-git-fork-jdwilkin4-feat-dynamic-image-d06fcf-astrodotbuild.vercel.app/en/recipes/dynamically-importing-images/

Description (required)

This PR is responsible for adding a new recipe for dynamically importing images using Vite's import.meta.glob function.

Related issues & labels (optional)

@netlify
Copy link
Copy Markdown

netlify Bot commented Nov 13, 2023

Deploy Preview for astro-docs-2 ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 5ee689a
🔍 Latest deploy log https://app.netlify.com/sites/astro-docs-2/deploys/6553bedfad671f00086aadff
😎 Deploy Preview https://deploy-preview-5391--astro-docs-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@sarah11918 sarah11918 added add new content Document something that is not in docs. May require testing, confirmation, or affect other pages. recipe Best solved by creating a short how-to/example labels Nov 14, 2023
Comment thread src/content/docs/en/recipes/dynamically-importing-images.mdx
Comment thread src/content/docs/en/recipes/dynamically-importing-images.mdx Outdated
@sarah11918 sarah11918 added the help - leave feedback Let's crowd source this one! Looking for comments, suggestions, LGTMs! label Nov 17, 2023
@sarah11918
Copy link
Copy Markdown
Member

Thanks so much @jdwilkin4 for jumping on another issue! 💪

I'm going to ask @Princesseuh to take a look over this, as it's been a hot topic in our Discord lately and we know we really do need better guidance around this topic.

There are a few directions a contribution like this could go in: a recipe, more explicit guidance on the images page itself, and I'd like Erika to see to comment about how close this was to what she had in mind. Also, to see whether there's anything additional (could be a separate PR!) she'd like covered, so that I know how this content fits into her idea of which new content needs to be added to docs.

Comment thread src/content/docs/en/recipes/dynamically-importing-images.mdx Outdated
Comment thread src/content/docs/en/recipes/dynamically-importing-images.mdx
Comment thread src/content/docs/en/recipes/dynamically-importing-images.mdx Outdated
Comment thread src/content/docs/en/recipes/dynamically-importing-images.mdx
@vercel
Copy link
Copy Markdown

vercel Bot commented Nov 18, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview Nov 25, 2023 0:16am

Comment thread src/content/docs/en/guides/images.mdx Outdated
Comment thread src/content/docs/en/recipes/dynamically-importing-images.mdx Outdated
}

const { imagePath, altText, name, age } = Astro.props;
const images = import.meta.glob<Record<string, () => Promise<{ default: ImageMetadata }>>('/src/**/*.{jpeg,jpg,png,gif}')

This comment was marked as resolved.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Agreed with @florian-lefebvre here: I'd love to see that original import function so as not to startle our non-TypeScript users.

Similar to my comment above about defining the prop types as optional, if it's helpful for TS users to know the shape, then after this code snippet there could be a mention separately of it.

Co-authored-by: Florian Lefebvre <contact@florian-lefebvre.dev>
Copy link
Copy Markdown
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Hey @jdwilkin4 this is fabulous, and I can't tell you how thrilled I am to have this for docs! I so appreciate your conciseness and directness when providing instructions. That is very comforting to a reader and sets them up for success, and it's so important to us in Astro Docs here!

I've left you some suggestions to consider with my thinking behind them, so see what you think!

Comment thread src/content/docs/en/recipes/dynamically-importing-images.mdx Outdated
Comment thread src/content/docs/en/recipes/dynamically-importing-images.mdx Outdated
Comment thread src/content/docs/en/recipes/dynamically-importing-images.mdx Outdated
- example-image-3.jpg
</FileTree>

:::note
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I adore this note. Perfectly written and perfectly placed! 💜

Comment thread src/content/docs/en/recipes/dynamically-importing-images.mdx Outdated
Comment thread src/content/docs/en/recipes/dynamically-importing-images.mdx Outdated
Comment thread src/content/docs/en/recipes/dynamically-importing-images.mdx
Comment thread src/content/docs/en/recipes/dynamically-importing-images.mdx
Comment thread src/content/docs/en/recipes/dynamically-importing-images.mdx Outdated
Comment thread src/content/docs/en/recipes/dynamically-importing-images.mdx Outdated
@sarah11918 sarah11918 removed the help - leave feedback Let's crowd source this one! Looking for comments, suggestions, LGTMs! label Nov 24, 2023
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Comment thread src/content/docs/en/recipes/dynamically-importing-images.mdx
Comment thread src/content/docs/en/recipes/dynamically-importing-images.mdx Outdated
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Copy link
Copy Markdown
Member

@kevinzunigacuellar kevinzunigacuellar left a comment

Choose a reason for hiding this comment

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

Awesome job, team! 🚀 Building on some feedback, I included a comment to enhance readability for TypeScript users by adding a type. Along the way, I encountered a couple of minor details while creating a reproduction. I've left comments that should fix them.

Thanks a lot @jdwilkin4, amazing recipe 🧑‍🍳

Comment thread src/content/docs/en/recipes/dynamically-importing-images.mdx Outdated
Comment thread src/content/docs/en/recipes/dynamically-importing-images.mdx Outdated
Comment thread src/content/docs/en/recipes/dynamically-importing-images.mdx Outdated
Comment thread src/content/docs/en/recipes/dynamically-importing-images.mdx Outdated
Co-authored-by: Kevin Zuniga Cuellar <46791833+kevinzunigacuellar@users.noreply.github.com>
Copy link
Copy Markdown
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Alright, it's still Friday for me, and that means FriYay Merge Day! 🥳

Thank you @jdwilkin4 and everyone for helping get this recipe over the finish line! Once the checks pass, I'll merge this one so it can be helping people right away! 🙌

@jdwilkin4
Copy link
Copy Markdown
Contributor Author

jdwilkin4 commented Nov 25, 2023

Hey @sarah11918 !

I pushed up one commit to remove that last of the incorrect developer props and changed it to the correct name prop which was one of Kevin's code review comments.

Thanks 👍

Copy link
Copy Markdown
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Alright, let's do this peeps! 🥳

@sarah11918 sarah11918 merged commit 28e4a0a into withastro:main Nov 25, 2023
@jdwilkin4 jdwilkin4 deleted the feat/dynamic-image-imports branch November 25, 2023 00:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

add new content Document something that is not in docs. May require testing, confirmation, or affect other pages. recipe Best solved by creating a short how-to/example

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Adding docs about images dynamic imports

5 participants