HTML and CSS using flexbox

Community feedback

• 

  Manuel Gil•340

  @ManuGil22Posted over 2 years ago

  Hey Kohsey!

  Im pretty new to html and css too but have some feedback:

  • Im not sure which aproach is correct but i would rather use an <h1> tag instead of a <p> for the title.
  • Not need to have <div> outside the paragraphs. As I told you before im pretty new too but think its just no needed. Divs are mostly used to customize and style some sections, for e.g. if you want to change the background-color of the title. In this case you are not doing anything with them so, there just extra lines.
  • One of the accessibility issues you have is that every <img> tag should have an alternate text. Its used by screenreaders as a description of the image. I would do smth like alt="qr code".
  • Also u have an accessibility issue that its not important but u might want to fix it. It says you should have a main landmark in ur code. That can be easily fixed by adding a <main> tag around the div with id="block". Just like this:

  <main>
          <div id="block">
              ....
          </div>
  </main>
  

  There are all details, nothing big. You made an amazing work there!

  Keep pushing and happy coding!

  Marked as helpful

  3

  ---

  

  Kohsey•40

  @KohseyPowerPosted over 2 years ago

  @ManuGil22 Thanks for the feed back Manuel !! I got it, I will fix these issues !

  0
• 

  Vee•240

  @JunasVeePosted over 2 years ago

  Hi Kohsey!

  I've checked your preview site and its codes in your repository. It looks awesome to me, simple codes only a few lines and the CSS is short as well but everything works fine. Additionally, it's responsive to all screen sizes. I would say that your efficiency is at another level!

  Perhaps the things that you might want to fix are accessibility issues in the report but those issues aren't extremely important. Overall, absolutely well done.

  Marked as helpful

  1

  ---

  

  Kohsey•40

  @KohseyPowerPosted over 2 years ago

  @JunasVee thanks for the feedback !!

  0
• 

  P

  Fluffy Kas•7,715

  @FluffyKasPosted over 2 years ago

  Hiya,

  Well done on this one, it looks great! There are some best practices you may want to follow (apart from the ones already mentioned by @ManuGil22 which are all good advice):

  1. Don't use ids, use classes instead, especially if you use them for styling. It's the best to get used to designing reusable components and since ids are unique, they cannot be reused elsewhere. So as a general rule, avoid them unless you really need them (sometimes for javascript, for example).

  2. Don't use <strong> to make your text bold. It has a semantic meaning. You could just add font-weight to the text with css.

  3. Not super important here, but usually when you name classes/ids you don't name them after the styles they apply to the html elements (unless you're creating utility classes, but that's another topic). Better to name them after their function and such (for example: "main-title" might be an <h1> heading at the top level, "hero-title" is another heading in the hero section, "cta" for a call-to-action section, etc).

  4. Avoid using height when you can, it's usually not needed. Every element has a default height, which you can increase by adding padding, for example (giving min-height to your body is an exception of course, since it's needed for flexbox to center your component, but for most other things you don't need to do this).

  2