More SASS. Getting more comfortable with nesting and variables

Community feedback

• 

  Lucas 👾•104,340

  @correlucasPosted about 2 years ago

  👾Hello Colin, Congratulations on completing this challenge!

  Amazing solution as always! Here are some suggestions for you:

  1.Remove the border radius from the card in the center only the first and third have it.

  2.You made your html structure entirely with div blocks but these div doesn't any semantic meaning, for this reason is better you use a better html markup improving your code, for example for each vehicle card you use <article> instead of the <div>.

  ✌️ I hope this helps you and happy coding!

  Marked as helpful

  0

  ---

  

  Colin McArthur•140

  @ColinMcArthur85Posted about 2 years ago

  @correlucas Great, #2 is good advice and something that I skip over when building these out. I have updated to have both article and section tags as recommended.

  Regarding #1, the design shows that the middle card hard a border radius. It does, however, overlap slightly with the other two cards which I suppose I could refine it to overlap slightly but otherwise, I think Im following the Figma file. Am I incorrect?

  0

  

  Lucas 👾•104,340

  @correlucasPosted about 2 years ago

  @ColinMcArthur85 Ah okay! I didnt remember all cards have the border-radius, sorry. I think the design seems better without the middle card rounded, but was a personal choice.

  1
• 

  バレンタイン 😈•64,170

  @VCaramesPosted about 2 years ago

  Hey @ColinMcArthur85, great job on this project!

  Some suggestions to improve you code:

  1. I noticed that your going back and forth between using px and rem. For accessibility purposes, it is a must to use rem/em instead of px for your CSS property values.

  2. To help keep your CSS code organized and easier to use, I suggest implementing CSS Variables. This will come in handy when building large websites, using light/dark mode, etc…

  It’ll look something like this:

  :root {
    --primary-color: value;
    --secondary-color: value;
    --tertiary-color: value;
  }
  

  And to use the variables you’ll use the var() function. So it’ll look like this.

  h1 {
  color: var(—primary-color);
  }
  

  Heres are some articles regarding CSS Variables.

  You can also take a look at my projects and see how I use it.

  https://developer.mozilla.org/en-US/docs/Web/CSS/Using_CSS_custom_properties

  https://www.w3schools.com/css/css3_variables.asp

  Happy Coding!

  0

  ---

  

  Colin McArthur•140

  @ColinMcArthur85Posted about 2 years ago

  @vcarames So the only instances of me using PX was on border-radius and for the media query which was used as these were the guidelines according to the style-guide.md file with the project. Is it still advisable to use rem/em for these elements?

  Regarding number two, I build this out using SASS and referenced all of my variables with SASS variables rather than CSS custom properties. I included my src folder when uploaded to GitHub to reference this work. It was my understanding that this was a preferred and common practice as SASS Variables do no bloat the CSS as they are not used until referenced. Also, in the interest of getting more practice with SASS I thought I would build this out as such.

  Am I incorrect about that? Should CSS custom properties be used?

  0

  

  バレンタイン 😈•64,170

  @VCaramesPosted about 2 years ago

  @ColinMcArthur85 I see it now. My mistake. I was looking at your code through my browser's developer tools and it didn't show your variables but once I went into your GitHub and saw your source code it was there.

  You did it the right way.

  For media queries, I definitely suggest using em for them. By using px your assuming that every users browser (mobile, tablet, laptop/desktop) is using a font size of 16px (this is the default size on browser). Em's will help with users whose default isn't 16px, which can sometimes cause the your content to overflow and negatively affect your layout.

  Heres a codepen link that shows what I mean. (Change your browsers font size and shrink the she screen) https://codepen.io/Iriswinterxdesign/pen/wvdaByZ

  Unfortunately, there's still isn't a set rule of which one should be used.

  0

  

  Colin McArthur•140

  @ColinMcArthur85Posted about 2 years ago

  @vcarames Ok cool! And Ill definitely adjust the media query to em's as recommended. Appreciate the guidance.

  0