Responsive web design using CSS flexbox and SASS

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, awesome work on this one. Layout in general looks great.

  Other already gave their feedbacks on this which is nice, just going to add some suggestions as well:

  • Always have a main element to wrap the main content of your page. On this one, the .container should be using the main instead of div.
  • On the .container instead of using width: 22rem use max-width: 22rem so that it will adapt to a small screen-width, because by using width you are setting a fixed size.
  • footer should be placed on its own row in the markup and not inside the .container which is supposed to be a main. A typical structure of a site looks like:

  <header />
  <main />
  <footer />
  

  This way, all element that has content are inside their respective landmark elements.

  • Vector image should be hidden since it is decorative only. Decorative image must be hidden at all times by using alt="" and extra aria-hidden="true" attribute on the img tag.
  • Also when using alt attribute, avoid using words that relates to "graphic" such as "illustration" and others. An img is already an image/graphic so no need to describe it as one.
  • Music-icon should be hidden so use the method I mentioned above.
  • Lastly, just maybe making the layout a bit bigger like the original and the interactive elements be bigger as well.

  Aside from those, great work again on this one.

  Marked as helpful

  1

  ---

  

  Michael•100

  @mikeyxxPosted about 3 years ago

  @pikamart Thank you so much! Most of these are new to me especially about hiding decorative images. I will definitely lock them down and remember to follow these specifications going forward in my subsequent projects. I have made all the edits you suggested and I think my output looks so much like the design now.

  1
• 

  CyrusKabir•1,885

  @CyrusKabirPosted about 3 years ago

  hello my dear friend ♥ your project need some work to get close as possible to main design

  • it's better to know very good units in css when you use them i mean it's not neccessary to use em or rem or even px in "img" they should fill the parent size
  • actually in define fonts for your project it's very good to define it in body not in "*" or universal selector and you have an invalid value in the font-family you set it's because the ( ",") between the main and the backup font;
  • don't give padding to the main container of your component it's make the img get press in card if you want padding on your card content add them into one single tag with class name for example (card-content) and add padding to that tag

  Marked as helpful

  1

  ---

  

  Michael•100

  @mikeyxxPosted about 3 years ago

  @CyrusKabir Thank you so much for these pointers. I had so much issues trying to figure out why the image was not fitting perfectly with the parent container, I had no idea it was the padding. I had to add the width to the img as a last resort. Also thanks for the font, slight mistake there. I would pay more attention. I have made all the edits you pointed out. Thank you:)

  1

  

  CyrusKabir•1,885

  @CyrusKabirPosted about 3 years ago

  @mikeyxx your welcom my dear friend ♥ good luck

  0
• 

  Carlos•1,110

  @Carlos-A-PPosted about 3 years ago

  This looks pretty good. All I'd suggest is fixing the font-family issue

  Marked as helpful

  1

  ---

  

  Michael•100

  @mikeyxxPosted about 3 years ago

  @Carlos-A-P Thanks, I would pay more attention to that

  0