Skip to content
Submitted almost 3 years ago

GitHub User Search App | HTML CSS Sass JavaScript (Async Await)

accessibility, bem, lighthouse, sass/scss
Vanza Setia•27,715
@vanzasetia
A solution to the GitHub user search app challenge
Preview (opens in new tab)Code (opens in new tab)

Solution retrospective

Hello Everyone! 👋

I finally finish my first API challenge! 🥳

This is a hard challenge for me, but I learned a lot of new things. 😀

Hopefully, I can get some feedback as well since I am still new at asynchronous programming. 😁

Now for the questions:

  • There's an article where it tells me that I should use input with type="text" instead of type="search". But, is this a good thing to follow?
  • Should I put the role="search" on the form element or on the div element (inside the form element)? I don't know the correct answer because MDN recommends putting the role="search" on the form element. But, Ahmad Shadeed recommends to put the role="search" on the form element to prevent overriding the default form semantic.
  • I follow the GitHub alternative text of the user's avatar which is like alt="@username". But, do you think it's good enough or is there a better alternative text for the avatar?
  • If you see the site, there are some statistics such as the total of public repositories, how many followers the user has, etc. My question is, should it be read as "8 Repos" or "Repos 8"? Currently, screenreaders will read the text as "Repos 8". But, if I should change it, then I would probably use flexbox to reverse the text visually. So, screenreaders would read the text as "8 Repos".
  • Should I have a hidden heading above each list? For example, <h3>User statistics</h3>. I got the inspiration from the @grace-snow's solution for the "Profile card component" challenge).
  • There are disabled links (or not available links) in this challenge. So, let's say the janedoe's GitHub account has no link in his/her profile. So, what do you think about the way I handle the disabled links? Currently, I make them as normal text (<span>Not Available</span>).
  • Also, you might notice that janedoe has no name. So, I visually hide the h2 if the user has no name, <h2 class="sr-only">Not Available</h2>. What are your thoughts?
  • Is there a better way to manage the colors for the dark mode and light mode? I used CSS variables in this project to make it easier because I could just switch the colors with prefers-color-scheme. You can see the CSS code to see how I handle the color for this project.
  • If you can help me suggest a better name for any function in my JavaScript, I would really appreciate it. For example, setThemeSwitcherStateForDarkMode() is a very long name. 😆

I am aware of the HTML issues that have been generated. I added the role="list" to each ul element to make sure the ul element still has the semantic meaning when I set list-style: none;.

If you find any bugs or notice I miss something, please let me know. Also, if you spot any bad practices in my code, feel free to tell me. I will try my best to make improvements to make this solution better. 🙂

Also, if you have finished this challenge and would like me to give feedback on it, please include a link to your solution. I would be glad to help you! 😀

Thanks!

Code

Please log in to post a comment

Log in with GitHub

Community feedback

  • Grace•32,130
    @grace-snow

    Nice enhancements! You don't need the aria-describedby on the theme switcher inputs though. They are part of a labelled fieldset already so this is just duplicating and making the legend repeat

    Marked as helpful
  • Grace•32,130
    @grace-snow

    Other questions...

    • I don't think you need hidden headings on this
    • role switch seems to be working well. I've not done any thorough testing or reading up about role switch before, but seems to be working well on voiceOver thanks to you adding that sr label that makes it all make sense
    • treatment of profiles with no info seems very sensible overall. I definitely would not have 'Not available' for a heading though. Instead do something like "janedoe Github profile" or similar
    • color theme management is great
    • I wouldn't worry much about names. As long as you are consistent in approach and they make sense that's fine
    Marked as helpful
  • Grace•32,130
    @grace-snow

    Re the search input question: https://adrianroselli.com/2019/07/ignore-typesearch.html

    I did have a little trouble using the input on my phone, but rarely use voiceover on there so it was probably my own fault! I found it strange not to have the search keyboard on there though... Adding inputmode="search" may help with that.

    Marked as helpful
  • Grace•32,130
    @grace-snow

    Overall, accessibility on this is excellent, well done. There's just a few things I think need tweaking :)

    1. I'm not sure why you've aria-hidden the 'search GitHub username' text then added a hidden label with aria-labelledby on the input. If there is visible text on the screen that can be a label, it's always much better to use that text. Speech tech users would say 'Enter search Github username' and it would activate the input using that label rather than them trying to use a hidden label which may use different wording

    2. There needs to be some kind of feedback to assistive tech users that their search has been successful. Either that can be done by adding an aria-live attribute to the results list or adding a hidden aria-live region that says something like 'results updated to user X'

    Other than that

    • I'd consider using a description list for the bottom section (location, website etc) as that is designed for key-value pairs. Not essential though
    • I'd use a different technique for that section's layout too, to prevent overflow when items are 2 long to fit in a 2 column layout
    • consider making external links open in a new window. If so, make sure you include some warning text to that effect (e.g. a title attribute)
    • I don't think you need as many media queries (you may not need any in fact). For example, doing body padding in % or using min() or clamp() etc would remove the need for it
    • on the document title, I'd put dev-finder before the profile name. The site is more important. And - it's an extra not really included in the challenge but would be nice - maybe make a new favicon for the site. Even just the dark blue background with a 'd' in the middle would be better than using the frontend mentor one I think

    Well done again on this, really nice work!

    Marked as helpful
  • Yazdun•1,310
    @Yazdun

    Hi Vanza, Great job on the challenge, I specially love the fact that you added a query in the url for the searched user, that's a really nice touch.

    Here is quick issue that Matt Seidel noticed on my solution and then I could fix it. Try searching mseidel819 in your app and you will see a weird overflow which doesn't look nice, you may wanna take care of that.

  • Grace•32,130
    @grace-snow

    I don't think the search role is necessary at all. You already have a clearly labelled input. That article you referenced is 6 years old and I generally steer away from anything that tells me to put a role on a div. It just isn't necessary. Don't over-complicate things

  • Enmanuel Otero Montano•2,155
    @Enmanuel-Otero-Montano

    Hello Vanza!

    Excellent. I went to see the code to see if you implemented the catch ( to handle the error ) since it is your first challenge consuming API and of course you implemented it 😂👌.

    On the other hand, I would like to read the article that says that an input type="text" is better than an input type="search", but it seems to me that implementing the input type="search" is a help in validation , so without reading the article I vote for the input type="search".

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