Looking to hire developers?
Final diff https://github.com/tarasis/tarasis.github.io/blob/master/FrontendMentor/four-card-feature-section/notes/desktop-final-diff.png
I am aware I shouldn’t have used width to scale the images in the boxes. Forgot at the time what I should do. Leads to them scaling up which isn’t ideal.
Edit to remove no longer applicable comment
@ApplePieGiraffe
Posted
Hey, Robert McGovern!
Your actual solution looks pretty good and is responsive!
Perhaps try taking a new screenshot to see if that fixes the way your solution is displayed on FrontendMentor.
Good work!
Keep coding (and happy coding, too)!
@mattstuddert
Posted
Great work on this challenge, Robert! You've matched the design really well. That is odd about the screenshot. My best guess is that the images are stretching in the screenshot due to the display: flex; declaration inside the cards. As the flow of content inside the cards is a column and the heading and paragraph you're using are block elements, I'd just get rid of flexbox inside the cards. You could then just put display: block; margin-left: auto; on the images to push them over to the right. My guess is that once those changes are pushed the screenshot will look OK.
We're actually exploring other screenshot options beyond Puppeteer, which we're using at the moment. As you've noticed, it's a bit too inconsistent, so we're looking for better options and will look to improve the reliability of the screenshots soon.
Just a quick note on your code. You've currently got multiple h1 elements. Although this is valid HTML it's generally still considered a bad practice, as it can cause accessibility issues with the content hierarchy. Instead, I'd recommend sticking to a single h1.
Keep up the great work!
@tarasis
Posted
@mattstuddert Thanks Matt. Those are exactly the sort of comments I need!
Yes changing the cards from using flexbox, and then putting display: block; margin-left: auto; did the trick. The new screenshot is more inline with what I see. (Bottom of Karma box slightly chopped off, but I am happy to live with that 😄)
As I said on Slack, I think I had Flexbox on the brain, and didn't even think of the easier solution.
Great point about the h1 elements. I hadn't considered that. I've changed the cards to use h2 now for the titles. I couldn't decide about the right approach for the "Reliable, efficient delivery" & "Powered by Technology ", neither seemed the right one to be an h1 alone. I'd maybe argue the second one as its bolded. So I changed both to p's
I think next time I need to maybe use a CSS reset too. I've seen slight positional differences between Edge, Firefox Dev Editon and the Responsively App.
Again, thank you for your time.
@mattstuddert
Posted
@tarasis that's great to hear my suggestion worked and the screenshot issue is now resolved. Using a CSS reset is definitely recommended. I use reset.css on every new project as a default.
I'd still say that the "Reliable, efficient delivery" & "Powered by Technology" is a heading. It's the main heading on this page, so I'd go with a single h1 wrapped around both lines. I'd the use span elements on the inside to separate the lines to style them differently. So it would look something like this:
<h1>
<span>Reliable, efficient delivery</span>
<span>Powered by Technology<span>
</h1>
It's good to see you using the Responsively App as well. It's an amazing tool to help to build responsive sites!
@tarasis
Posted
Cheers ApplePieGiraffe 👍, sadly taking a new screenshot didn’t help.
Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!
Join our Discord
