Skip to content
OverviewSolutions
9
Comments
3
Srikar Gunnam
@srikargunnam

All comments

  • Srikar Gunnam•150
    @srikargunnamPosted almost 3 years ago

    Hi CARLOS ALZATE,

    Congratulations 🎉 for completing the challenge.

    I have seen your github repo of the current project, it looks like you have not replaced the existing README.md file with README template modified as per your work, If you are not familiar with markdown, this would be great opportunity.

    Hope this helps, please mark as helpful if it does.

    Marked as helpful
    0
  • Srikar Gunnam•150
    @srikargunnamPosted almost 3 years ago

    Hi Oliver Nikolovski,

    First I would like to congratulate you on completing the challenge 🎉

    I have few suggestions for which you can work on

    1. You must use h1 tag (level one heading) in your HTML as per Web Accessibility standards, so better swap your h3 tag with h1 and use CSS for all the required styles.

    After you commit your code to GitHub with necessary changes, you need to check out the "view report" option under your submitted solution and re-generate the report to see if any accessibility issues still exists, this way you can make required changes to your code.

    Hope this helps 😉, please mark as helpful if it does.

    Marked as helpful
    1
  • Srikar Gunnam•150
    @srikargunnamPosted almost 3 years ago

    Hi Alexandro Munera,

    First I would like to congratulate you for completing this challenge 🎉

    I would like to give few suggestions based on my observations.

    1. It is must to use h1 tag (level one heading) as per web accessibility standards, you can swap h2 with h1, and my advice is to use a header tag instead of a div tag to wrap h1 and p tags.

    2. Your card looks stretched, that is because you used width as 90%, so there is no max-width, Instead of using width as 90% use min(90%, "the max width the content fits good"), so that your content looks good even or much wider screens.

    3. Breakpoint for mobile screens mentioned in the style guide is 375px, but used a breakpoint at 768px, it would be better if you can work on that, i can understand that the content looks like weird when it reached 768px, for this you could as a media query for tablet assuming it is given in this project, it will give you a new opportunity to work on your own ideas to make the content suit tablet view until it reach 375px.

    4. I have seen that you have used rem units at some places and px units at some places, it is a good practice to stick to one type of units, i would suggest to go with rem entirely.

    This link "https://www.sitepoint.com/understanding-and-using-rem-units-in-css" will help you get a better understanding of rem units, and how you can easily use a trick to make 10px = 1rem, this will help you easy to calculate and use rem units, but keep in mind that this trick will not work for media queries which is not mentioned in the above link.

    hope this helps 😉, please mark as helpful if it does.

    Marked as helpful
    1