Skip to content
Submitted over 2 years ago

GitHub User Search

AceDubā€¢250
@AceDub
Preview site (opens in new tab)View code (opens in new tab)
This is a solution for...GitHub user search app
  • HTML
  • CSS
  • JS
  • API
2junior

Design comparison

SolutionDesign

Solution retrospective

My first time working with an API. Would appreciate any feedback or criticism.

Community feedback

  • Vanza Setiaā€¢27,775
    @vanzasetiaPosted about 2 years ago

    Hi, AceDub! šŸ‘‹

    Congratulations on completing this challenge! šŸŽ‰ Your solution looks pretty good! šŸ˜€ I like the idea of having a loading spinner! šŸ‘

    Here are some suggestions for improvements.

    • I recommend adding rel="noopener" to any anchor tags that have target="_blank". This is a security essential for external links. I suggest reading the web.dev article to learn more about this.
    • I would not recommend setting any height to the html element. I suggest treating the body element as the webpage of the site.
    • I would also suggest not setting any height to the body element. If you ever need to set a height then use min-height instead. This way, the body element is allowed to grow if ever needed.
    • Having the desktop layout at 1440px is too late. So, I suggest adjusting the breakpoint of the media query. As a side note, the design sizes are telling you that the site should look like this at this specific screen size. They have nothing to do with the media queries.
    • The statistic should be a list with three bullet points. Also, the statistic categories and the statistic numbers should not be headings. Use span instead (since it will be wrapped by li). Heading tags have meaning and are used to give structure to the page. You can think of headings like titles in documents. So, I recommend writing the HTML without any styling so that you can make a better decision on what HTML element you should use for the web content.

    Overall, great work. The site even still looks good at 320px which I would consider the smallest screen size.

    I hope this helps! Keep up the good work! šŸ‘

    0
    Vanza Setiaā€¢27,775
    @vanzasetiaPosted about 2 years ago

    @AceDub

    It's good to know that you realize that the users can get 403. Good job! šŸ‘

    0
  • mubizzyā€¢1,520
    @mubizzyPosted over 2 years ago

    Excellent job on this challenge! your report has a few issues though:

    1. wrap everything in your body in <main> or use semantics

    2.Ā Ā  it is a best practice to use both HTML 5 and ARIA landmarks to ensure all Ā Ā Ā  content is contained within a navigational region.

    Hope it helps:)...don't forget to mark it as helpful šŸ‘

    You can get more details here...click here

    0
    AceDubā€¢250
    @AceDubPosted over 2 years ago

    Hello @mubizzy,

    I do believe I have everything in my <body> wrapped in <main>. I took your advice and checked out the documentation you provided. Adding an ARIA landmark generated a new HTML issue for me which stated that it was unnecessary for the element.

    I appreciate you taking the time to provide some feedback!

    0
    mubizzyā€¢1,520
    @mubizzyPosted over 2 years ago

    @AceDub am glad šŸ˜Š you found my post helpful šŸ˜Š šŸ˜ƒ šŸ˜€ Happy coding šŸ˜ƒ šŸ˜Š šŸ˜€

    0

Please log in to post a comment

Log in with GitHub

Join our Discord community

Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!

Join our Discord