#320 Refactor images #396
#320 Refactor images #396
Conversation
| @@ -4,6 +4,7 @@ | |||
| "allCategories": "All categories", | |||
| "answer.no": "No", | |||
| "answer.yes": "Yes", | |||
| "blogPostPage.guestName": "Guest name", | |||
There was a problem hiding this comment.
I found the missing translation key - it was just incorrectly named before. It is eventDetails.eventGuests. Please use this one and remove "blogPostPage.guestName", also in the slovak translation. Sorry for additional misdirection.
There was a problem hiding this comment.
I see now. Noted :)
There was a problem hiding this comment.
I am sorry, now I see that the advice i gave you was wrong. the eventDetails.eventGuests just says "Účinkujú" and then there's a list of guests, for which we need an alt in their profile picture. We can discuss this in person :)
There was a problem hiding this comment.
That's part of the "remove object-center when it's not necessary" task. There was a div with className object-center and the entire div was deleted - it was there (only) so that object-center could be applied, I assume
There was a problem hiding this comment.
The original comment was meant for that specific line of code. This change may be ok, but it slows the review process.
katarinazaprazna
force-pushed
the
320/techdebt-refactor-nextjs-images
branch
2 times, most recently
from
8c18a1c to
52dcf6e
Compare
katarinazaprazna
force-pushed
the
320/techdebt-refactor-nextjs-images
branch
from
52dcf6e to
1290b0b
Compare
| <div className="relative w-full shrink-0 md:h-75 lg:h-[400px]"> | ||
| <StrapiImage | ||
| image={bannerImage || getImagePlaceholder(EventDetailPlaceholder)} | ||
| alt={bannerImage?.alternativeText} |
There was a problem hiding this comment.
| alt={bannerImage?.alternativeText} | |
| alt={bannerImage?.alternativeText ?? ""} |
based on the recent image alt clarification, this seems to be the right solution (it follows our approach for richtext images)
| <div className="relative h-40.5 w-full shrink-0"> | ||
| <StrapiImage | ||
| image={listingImage || coverImage || getImagePlaceholder()} | ||
| alt={listingImage?.alternativeText || coverImage?.alternativeText} |
There was a problem hiding this comment.
Now I realize that this is a decorational image as per our docs. It had an empty alt before, so it should remain. Sorry about that confusion when we discussed this
| <div className="relative mb-4 h-40.5 w-full shrink-0"> | ||
| <StrapiImage | ||
| image={coverMedia?.data?.attributes ?? getImagePlaceholder()} | ||
| fill |
There was a problem hiding this comment.
| fill | |
| fill | |
| alt="" |
Now I realize that this is a decorational image as per our docs. It had an empty alt before, so it should remain. Sorry about that confusion when we discussed this
| <div className="relative mb-4 h-40.5 w-full shrink-0"> | ||
| <StrapiImage | ||
| image={image?.attributes ?? getImagePlaceholder()} | ||
| fill |
There was a problem hiding this comment.
| fill | |
| fill | |
| alt="" |
Now I realize that this is a decorational image as per our docs. It had an empty alt before, so it should remain. Sorry about that confusion when we discussed this
github-actions
bot
added
pr: fix & ship 🚢
|
Test build pipeline info 🚀 🔜 next |
Description
MImageandimgtags withStrapiImageandImage(from Next)fillproperty ofStrapiImageand wrapping the image intodivwith defined dimensions)Testing
Browse the site and ensure that images are displayed correctly, such as no image is missing, is cropped, or strangely stretched
Changes were made at:
/(to seeNoticeCard)/zazite/podujatia/zazite/podujatia/konverzujeme-po-anglicky-3-2-25/error(stránka sa nenašla)/zazite/prenajmite-si-priestor(to seeBranchCard)/sluzby/vzdelavanie/clanky(to seeBlogPostCard)/sluzby/vzdelavanie/clanky/efektivne-sluzby-kniznice-pre-sucasnu-spolocnost-priklady-z-praxe/o-nas/partneri-a-spoluprace(to seePartnerCardRow)/o-nas/z-historie/navstivte/nase-lokality/klariska(to seeImageGalleryandImageLightBox)