3 Column Responsive Desing (Flexbox, SaSS)

Community feedback

• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted over 2 years ago

  Hello @Alexuva ,

  Congratulation on completing this challenge,

  I have some suggestions regarding your solution:

  HTML

  • Page should contain a level-one heading . As this is not a whole webpage , you can use <h1> with class="sr-only" (Hidden visually, but present for assistive tech).

  • <svg> 's do not add important information to a document should be considered decorative. You can use aria-hidden="true" to hide the SVG from screen readers. focusable="false" is also used to ensure Internet Explorer won’t allow the Tab key to navigate into the SVG. In this challenge , all the svg's are decoratives.

  • Don't capitalise in html, let css text transform take care of that. Remember screen readers won't be able to Read capitalised text as they will often read them letter by letter thinking they are acronyms.

  • Clicking those "learn more" buttons would trigger navigation not do an action so button elements would not be right. And for future, it is essential if you include a button in a form element without specifying it's just a regular button, it defaults to a submit button, so it's a good idea to make a habit of specifying the type. So use the <a> .

  And it is essential that interactive elements have focus-visible styles as well as hover styles. These need to be really clear and obvious as they are needed to help a keyboard user know where is focused on the page.

  CSS

  • width:100vw; If you set a page width, choose 100% over 100vw to avoid surprise horizontal scrollbars

  • To center the component on the middle of the page, You can use the flexbox properties and Use min-height: 100vh to the body instead of height: 100vh; . This allows the body to to grow taller if the content outgrows the visible page.

  • Then you can use flexbox properties to the container that wraps the three card and give it flex-direction : row for the desktop and column for the mobile.

  • In line-height: 1.6em;use unitless value for the line-height , this is the preferred way to set line-height and avoid unexpected results due to inheritance.Read more in MDN.

  • width: 320px;an explicit width is not a good way . instead of setting so many widths for individual cards in this, consider using max-width to the main container . That will let the component grow up to a point and be limited.

  • height: 1502px;It's rarely ever a good practice to set heights on elements . Let the content inside the card element dictate the height of it.

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

  • It's recommend not to style on ID'S . The best way to do that is single class selectors

  Overall , your solution is good . Hopefully this feedback helps.

  Marked as helpful

  1

  ---

  

  Alejandro Hernández Rosa•180

  @AlexuvaPosted over 2 years ago

  Thank for the feedback @PhoenixDev22 ! I'll make it better next time ;)

  1