3-column preview card component

Community feedback

• 

  Alexander Morse•610

  @SakeranPosted over 2 years ago

  Hey there! Nice job finishing up this project. Looking through your source code, I noticed a few areas where you might be able to improve.

  Before getting into anything else, I notice you committed a few unnecessary folders to your git repository: namely node_modules and .sass-cache. In particular, you should try to avoid committing node_modules, since it tends to make the project's file size unreasonably big. To make git ignore these, just add the following lines to the project's .gitignore file:

  node_modules
  .sass-cache
  

  ---

  With respect to the layout's responsiveness, it took me a moment to realize that there was any, since the breakpoint wasn't where I expected it to be. You have the right idea using min-width to achieve a mobile-first layout, but let's consider the breakpoint you've chosen to use:

  @media screen and (min-width: 375px) {
    /* Styles for the desktop view */
  }
  

  Here, you're saying "if the screen is wider than 375 pixels, then it's wide enough to fit the entire desktop layout". Which, if you resize the viewport to something small-ish, clearly isn't the case - you can't even see any content at some sizes.

  Again, you've got the right idea using min-width, but you're switching over to the desktop view too soon - 375px is definitely phone-sized, and on the narrower end of it at that. Personally, the smallest width I'd consider "desktop-sized" would be something like 768px, and even that might too small for some designs. If you weren't aware, most browsers have tools to let you inspect a page at different screen sizes, which is useful for figuring out which sizes to use. For example, here's an overview of Chrome's Device Mode

  By the way, there's no need to separate media queries out into different .scss files. In Sass, you've got support for nested media queries, so you can simply write something like:

  .container {
      /* Mobile-first styles for .container */
      
      @media screen and (min-width: ...) {
          /* Desktop styles for .container */
      }
  }
  

  ---

  I also noticed you're positioning the .container element using this CSS: margin: 4rem 1.5rem;. While this gives a decent-looking result at medium-sized screens, there are a couple issues:

  • As we saw above, the explicit margins "squish" the content, causing it to vanish on smaller screens.
  • At very large screens, the the content has the opposite problem: 4rem isn't nearly enough, so the container looks very stretched out.

  Fortunately, there's an easy way to address both issues. In two steps:

  1. Figure out how wide the container should ever get, and set a max-width. For example: max-width: 900px;.

  2. Rather than use an explicit margin for .container's inline axis, use auto. That is: 'margin: auto 1.5rem;. Using auto tells the browser to figure out how much horizontal space is left over (after subtracting the container's width), and split it evenly between the left- and right- margins.

  ---

  Finally, there were two smaller things I noticed:

  • On your button elements, it might be slightly better UX to add the cursor: pointer property to it, since mouse-users tend to associate the pointer icon with clickability.

  • Also on your buttons, you might have noticed a bit of "jitter" when mousing over them, causing the containers' height to grow slightly when the :hover styles are showing. This is down to the button's border contributing to the overall size - it is literally two pixels taller than usual in its :hover state. There are a few ways you could fix this, but personally I'd set the border explicitly in the default state, but make the color invisible: border: 1px solid transparent. Then in the :hover styles, you can just set border-color: $lightGray;.

  Hope you were able to make use of some of this.

  Marked as helpful

  1

  ---

  

  John Lois Floro•300

  @loifloroPosted over 2 years ago

  @Sakeran thanks for your feedback, its definitely the best feedback I received so far here on Frontend Mentor

  0