Four Card Feature

Community feedback

• 

  Akiz-Ivanov•100

  @Akiz-IvanovPosted about 1 hour ago

  Hello @TheRoboRobin, great job on the challenge. 💪

  If you want to make your code more organized, consider splitting your SCSS into separate files (e.g. _variables.scss or _layout.scss) and importing them. While this isn't essential for small projects, it can help with better organization.  

  The responsiveness is great! Great idea for the tablet screens layout. On laptop screens (1024-1200px) the cards feel a bit too close to the edges, it's not a big deal, but adding a little bit of padding on the sides would be great.

  There’s a brief moment between 880px and 894px when the first card shrinks, causing the layout to appear slightly unbalanced. This can probably be fixed quickly by adding "align-items: stretch" to this media query:

  &__cards {
  	@media only screen and (max-width: $screen-medium) {
        grid-template-columns: repeat(2, minmax(20rem, 1fr));
        align-items: stretch;
      }
  }
  

  There's also a little accessibility issue here in the header:

  <h2 class="heading-secondary">Reliable, efficient delivery</h2>
  <h1 class="heading-primary">Powered by Technology</h1>
  

  Having <h2> before <h1> can confuse the screen readers. Heading levels should follow a logical hierarchy. A <h1> should always be the top-level heading.

  In this design it looks like both lines are part of the same header, so it would be better practice to put both inside h1 and style them with css.

  <h1>
    Reliable, efficient delivery
    <span>Powered by Technology</span>
  </h1>
  

  To place them on separate lines, you can use either the <br> tag or apply display: flex with flex-direction: column to the <h1>.

  Overall, it's a great solution. 🥳 Wishing you the best with your coding journey!

  0