Community feedback

• 

  Ehtisham Ali•150

  @EhtishPosted about 2 years ago

  After analyzing your code.

  "Important Suggestion and code for you"

  ** Write your responsive.css (media query) code in the end of style.css file & remove extra file responsive.css ...? why?? see in point 2 **

  1. In style.css. Remove flex direction:column; in .section__container class

  2. Save your time (Follow DRY principle):

  • DRY = Don't Repeat Yourself.
  • if you add your media query code in the end of style.css file.
  • then you have only write few lines of code mention below:

    Mobile

    	.section__container {
        	flex-direction: row;
    	/* there is no need to write
    	display:flex; 
    	and 
    	other extra codes because we already 
    	write above....(style.css)
    	As a result 
    		- save time 
    		- improve code quality
    		- fast development */	
    	}
    }
    

  Marked as helpful

  1
• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted about 2 years ago

  Hi Jorge Muñoz,

  Congratulation on finishing another frontend mentor challlenge. I have some suggestions regarding your solution:

  • Page should contain a level-one heading. In this challenge , as it’s not a whole page, you can have<h1>visually hidden with sr-only.

  • In my opinion, the images are much likely to be decorative. 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.

  • What would happen when the user click those learn more? In my opinion, clicking those "learn more" would likely trigger navigation not do an action so button elements would not be right. So you should use the <a>. For future use , it's a good habit of specifying the type of the button to avoid any unpredictable bugs.

  • There are some unnecessary div’s needed to be removed.

  • Add border-radius andoverflow hiddento the main container that wraps the three cards so you don't have to set border-radius to individual corners.

  • line-height: 1.6rem;Use a unitless line-height value to Avoid unexpected results. You can read more in mdn

  • There are so many repeated style rules , better to use reusable and manageable classes. For example: each column have the same styles, So you can use a class .card for the shared styles, then for each distinct styles like (background color) use another class . card__sedans, . card__suvs. card__luxury.

  hopefully this feedback helps.

  Marked as helpful

  0