Responsive 3 column layout using CSS Grid and Sass

Community feedback

• 

  Naveen Gumaste•10,480

  @NaveenGumastePosted almost 3 years ago

  Hay ! Srikar Good Job on challenge

  Marked as helpful

  0
• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted almost 3 years ago

  Hello @srikargunnam ,

  I have some suggestions regarding your solution:

  • For any decorative images, each img tag should have empty alt="" and aria-hidden="true" attributes to make all web assistive technologies such as screen reader ignore those images .

  • you can add a <h1> with class="sr-only" (Hidden visually, but present for assistive tech).and use <h2 > instead of <h1 >this helps search engines better understand the primary subject matter of each page,

  
  .sr-only {
  	border: 0 !important;
  	clip: rect(1px, 1px, 1px, 1px) !important;
  	-webkit-clip-path: inset(50%) !important;
  		clip-path: inset(50%) !important; 
  	height: 1px !important;
  	margin: -1px !important;
  	overflow: hidden !important;
  	padding: 0 !important;
  	position: absolute !important;
  	width: 1px !important;
  	white-space: nowrap !important;            
  }
  
  

  • swap the buttons for anchor tags. Clicking those "learn more" buttons would trigger navigation not do an action so button elements would not be right.

  • To center the card on the middle of the page , you can use the flexbox properties and min-height: 100vh like this : (no need for the absolute position )

  
  body{
  display:flex;
  align-items: center;
  justify-content: center;
  min-height: 100vh;
  }
  

  for this part:

  * {
    -webkit-box-sizing: border-box;
            box-sizing: border-box;
    font-size: 62.5%;/* changing base html size. This has huge accessibility implications for those of us with different font size or zoom requirements.*/
    font-family: "Lexend Deca", sans-serif;
    margin: 0;
  }
  
  body {
  /*  min-height: 135rem; */
  min-height: 100vh;/*This  allows the body to to grow taller if the content outgrows the visible page.*/
  
  }
  
  

  Hopefully this feedback helps.

  Marked as helpful

  0

  ---

  

  Srikar Gunnam•150

  @srikargunnamPosted almost 3 years ago

  Hi @PhoenixDev22,

  Thank you very much for taking time to go through my code and help me understand my mistakes, I made changes in my code as you mentioned except for few, I mentioned my reasons below for those.

  Reasons:

  1. Display flex to center and 100vh min height : In my case when i switch to mobile view, i need to see a padding on top as in the challenge screenshot and i need to see my attribution at the bottom, but using flex and min-height 100vh is not giving me the output as expected.

  2. Font size 62.5% : I used this just to make myself comfortable using rem units, but i don't get how this is going to create issues in zooming, i have used this after seeing many developers using this hack for smooth working with rem units. here is the article where i found this trick

  Please do reply me if i misunderstood any concept in some or the other way

  0
• Account deleted

  Hi there 👋

  Congratulate on finishing your project 🎉. You did a great job 🔨

  I give some suggestions that I hope help you take your project design to the next level 📈.

  • Let's change the background color to ``#F2F2F2`
  • You also can add some hover effects to the buttons 🚀👍

  Happy coding ☕

  Maqsud

  Marked as helpful

  0

  ---

  

  Srikar Gunnam•150

  @srikargunnamPosted almost 3 years ago

  Hi @maqsudtolipov, I have made the changes you mentioned, thanks for correcting me with the background color, and suggestion on button hover effects :)

  0