Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

Submitted

Styled Components, React, hosted on Heroku

P
Ahmed Abdelaalβ€’ 220

@shrki416

Desktop design screenshot for the GitHub user search app coding challenge

This is a solution for...

  • HTML
  • CSS
  • JS
  • API
2junior
View challenge

Design comparison


SolutionDesign

Solution retrospective


Any suggestions, comments welcome :)

Community feedback

P
Alexβ€’ 1,990

@AlexKMarshall

Posted

Careful about accessibility. Make sure you always test your designs with a keyboard to make sure they work.

If you remove the built in :focus-visible styles, you must replace them with something equally high-contrast. On the search input you've removed the styling and so there's nothing to indicate that I'm focused on the field.

Make sure to use proper alternative text for images, or an empty string if they are decorative or convey no extra information. An alt text of "avatar" doesn't mean anything to someone viewing the site. If the Github API provides alt text for the user's avatar, then that should be used. If not then it should be an empty string.

Taking a look at your code, you're using a lot of very nested selectors in your CSS. That's going to cause problems with maintainability due to massively increased specificity. Prefer to use single class names and target those. What that means in styled-components is that you'll create individual styled components for the various elements. Rather than nest them all under a single styled component. That way it becomes far easier to override styles when you need to.

Marked as helpful

1

P
Ahmed Abdelaalβ€’ 220

@shrki416

Posted

@AlexKMarshall Excellent suggestions, thanks so much for taking the time to view my solution and offer feedback, very much appreciated :)

0
Agata Liberskaβ€’ 4,015

@AgataLiberska

Posted

Hi @shrki416!

Nice work, your solution looks great :) I only have a few suggestions:

  • On smaller end of mobile viewports, the app overflows the screen as you set a constant width. 320px is not an unpopular viewport size so it would be good to cater to it :)
  • Not sure what the brief was, but I think it would be better for name to stay blank if there is no name set, rather than defaulting to Octocat. You could provide initial state for Octocat info to display when app first load and then override that?
  • I'd love for the Twitter thing to be a link to the twitter profile if it's available and maybe don't have a link for the website if one isn't provided (have the text 'not available' rather than anchor tag)
  • Also, I'd love to be able to switch themes with keyboard to make it accessible - you could look into using a button or a pair of radio inputs there?

Hope this helps, and again, really nice work!

Marked as helpful

1

P
Ahmed Abdelaalβ€’ 220

@shrki416

Posted

@AgataLiberska wow! Excellent feedback. thanks so much for the suggestions above :) I'll go through and make those changes. Thanks again!

0

Please log in to post a comment

Log in with GitHub
Discord logo

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