Mobile first Four Card Feature Section, pure HTML & CSS

Community feedback

• 

  T

  ApplePieGiraffe•30,545

  @ApplePieGiraffePosted about 4 years ago

  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)!

  2
• 

  T

  Matt Studdert•13,611

  @mattstuddertPosted about 4 years ago

  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!

  1

  ---

  

  P

  Robert McGovern•1,075

  @tarasisPosted about 4 years ago

  @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.

  1

  

  T

  Matt Studdert•13,611

  @mattstuddertPosted about 4 years ago

  @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!

  1
• 

  P

  Robert McGovern•1,075

  @tarasisPosted about 4 years ago

  Cheers ApplePieGiraffe 👍, sadly taking a new screenshot didn’t help.

  0