Add with-cloudinary example #43250
Add with-cloudinary example #43250
Conversation
Nutlope
requested review from
timneutkens,
ijjk,
shuding,
styfle,
leerob,
steven-tey and
balazsorban44
as code owners
| // This effect keeps track of the last viewed photo in tshe modal to keep the index page in sync when the user navigates back | ||
| if (lastViewedPhoto && !photoId) { | ||
| document | ||
| .querySelector(`#photo-${lastViewedPhoto}`) |
There was a problem hiding this comment.
Is a querySelector the way we want to do this? Or could / should it be done using a ref?
There was a problem hiding this comment.
I paired with Sam/Ryan on this featured and I actually suggested using a ref but they recommended I do it with querySelector instead. Can't remember the reason why – perhaps speed? Cause otherwise, I'd have to attach a ref to each of the 300+ images on a page.
There was a problem hiding this comment.
You think I should explore using ref instead?
There was a problem hiding this comment.
I agree – I'll add this to my list of improvements for early next week. Sound good?
So far I have:
- Add infinite scrolling
- Fix bottom photo navbar animations on very wide screens
- Convert querySelector to
ref
For now, I'd love to ship this since we're planning to send the email tomorrow morning.
There was a problem hiding this comment.
querySelector seems really odd with react
Lets compare the perf with `ref
Co-authored-by: Michael Novotny <manovotny@gmail.com>
| const results = await cloudinary.v2.search | ||
| .expression(`folder:${process.env.CLOUDINARY_FOLDER}/*`) | ||
| .sort_by('public_id', 'desc') | ||
| .max_results(400) |
There was a problem hiding this comment.
Weren't we gonna add the infinite scrolling handling to reduce the initial document size?
There was a problem hiding this comment.
Planning on adding infinite scrolling + smoothing out some animations next week, but for now, I'd love to ship it since we're sending out an email tomorrow morning to all Next.js Conf attendees with the photo gallery. Lemme know if that sounds alright to you.
For context, we've kept folks waiting for too long since I was originally planning to send this out early last week and I've already told a couple folks that we were sending it tomorrow.
|
Looks good to land, we can address mentioned tweaks in follow-ups! |
| <Image | ||
| src={`https://res.cloudinary.com/${ | ||
| process.env.NEXT_PUBLIC_CLOUDINARY_CLOUD_NAME | ||
| }/image/upload/c_scale,${navigation ? 'w_1280' : 'w_1920'}/${ |
There was a problem hiding this comment.
This feels off because the 1280/1920 distinction is made twice. Once here in src and once for width.
I'm still thinking that with-cloudinary example should use the loaderFile to configure cloudinary and then you just pass src=${currentImage.public_id}.${currentImage.format} to each <Image>
Added an image gallery example using Next.js and Cloudinary.
Edit: This is now ready to ship!