Closed britneywwc closed 2 months ago
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 69.64%. Comparing base (
94cc8cf
) to head (68dac1c
). Report is 30 commits behind head on case-study-templates.
small
element, so it's sized properly, but also for better semantics. It should probably be both image and copyright in a figure element for better semantics (I quite like this approach), but I leave that to you.I also replied to your comment on Figma.
One comment on the implementation in code: will we be able to add some formatting within the text with the current templates (for instance, links, italics...)?
@britneywwc I left some comments in the figma
- [ ] Breadcrumbs at the top are missing.
Is there a template for content writers to control what the breadcrumb title would be? I also had a question about the breadcrumbs heading, left a comment for you in the Figma file.
The contact us link should be aligned to the fourth column as per the design.
In the template, Contact Us is aligned to the first column but on the example it's 4th column. Could you please confirm which to follow?
One comment on the implementation in code: will we be able to add some formatting within the text with the current templates (for instance, links, italics...)?
Yes there will be, there was a comment left by the Vanilla team to tweak the macros to address this. I will be changing the macros to accommodate more text formatting.
Per the image sizes, I'm waiting for Mattea's help to export the images, will try to fix the image widths then.
Is there a template for content writers to control what the breadcrumb title would be?
Just added it to the template metadata, it should be the company name 👌🏻 As for the styling, use this component, same implementation as here.
The contact us link should be aligned to the fourth column as per the design.
Aligned to the fourth, I just tweaked it.
This is ready for you guys, please have another look!
@juanruitina Per your comment for the figures & captions, I applied your suggestion for using <small>
but wrapping it in a <figure>
element ruins the image container styles so I will leave it out for now.
One minor thing: logo alt shouldn't say "logo", as per the content guidelines. You could also use the company name you used for the breadcrumbs, so "European Space Agency" instead 🤷🏻♂️ +1ing this already as it's minor.
Spacing between sections seems huge on desktop, but I defer to visual (@mattea-turic) on the decision there. It's looking cute 💫
Thanks for all the reviews! I made a minor tweak to the image section by adding p-image-container--cinematic
after seeing's Lyubo's comment and that is causing the bottom two images to have some big spacing between the image and the caption. I think this is because the original image is not in the cinematic ratio, so maybe this is something to be fixed on assets? (@mattea-turic)
Done
QA
Issue / Card
Fixes WD-14051
Screenshots
[If relevant, please include a screenshot.]
Help
QA steps - Commit guidelines