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

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

#accessibility#bem#lighthouse#sass/scss
Vanza Setiaβ€’ 27,795

@vanzasetia

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


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!

Community feedback

P
Graceβ€’ 28,590

@grace-snow

Posted

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

1

Vanza Setiaβ€’ 27,795

@vanzasetia

Posted

@grace-snow

Hi, Grace! Thank you for the feedback.

I removed the aria-describedby from the radio inputs.

By the way, I got the inspiration for the theme switcher from your website, FED Mentor .

Home - FED Mentor

I notice that the radio inputs have aria-describedby that is pointing the <legend>. Also, there is role="radiogroup" on the <fieldset> element.

I followed the HTML markup and the aria-describedby. πŸ˜…

0
P
Graceβ€’ 28,590

@grace-snow

Posted

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

1

Vanza Setiaβ€’ 27,795

@vanzasetia

Posted

@grace-snow

Hi Grace! πŸ‘‹

For the role="switch", I get the inspiration from Twitter. Jen Simmons created a tweet where she is explaining how to create a theme switcher with only HTML and CSS (using :has()). Then, there is someone called Mike-ιΊ₯-Mai/index.html, he is telling Jen Simmons to add role="switch" to the checkbox element.

For the heading, I have just fixed it. Now, if the user has no name, then the heading will have text content like "janedoe's GitHub Profile".

Thank you for answering my questions! πŸ‘

P.S. I would improve the project step-by-step instead of just trying to fix or improve the project at once. I am afraid that fixing all things at once will make me do a lot of git restore . (or git reset --hard). So, I will reply to each feedback once I have fixed the issue or when I have any questions. I hope that you are okay with this. πŸ™‚

1
P
Graceβ€’ 28,590

@grace-snow

Posted

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

1

Vanza Setiaβ€’ 27,795

@vanzasetia

Posted

@grace-snow

I have made the changes regarding the search input. Now, the input is using type="text" with inputmode="search" attribute. Also, I removed the div with role="search" element and I didn't add role="search" to the form element.

Also, thank you for the article! πŸ‘

0
P
Graceβ€’ 28,590

@grace-snow

Posted

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

1

Vanza Setiaβ€’ 27,795

@vanzasetia

Posted

@grace-snow

Thank you for telling me that the website has excellent accessibility! It motivates me to be better at web accessibility! πŸ™Œ

Regarding the way I label the input, as far as I know, it's best to have the label before the input element. So, I set up a hidden label to label the input. After that, the visible label element is used as the floating label (for visual users). Then, when the user fills the search input, the placeholder moves above the search input instead of going away (like a normal placeholder). What do you think about this?

I added a hidden live region. Here is the HTML markup.

<p class="sr-only" 
   role="status"
   aria-live="polite"
   aria-atomic="true"></p>

Then when the search is successful. The text will get injected.

<p class="sr-only" 
   role="status"
   aria-live="polite"
   aria-atomic="true">
  results updated to janedoe's GitHub profile
</p>

I tested it. The result is, after the search is successful, both TalkBack and Narrator pronounce it the same way, "results updated to janedoe's GitHub profile".

Regarding the description list (dl), I tried to use it and it turned out that it causes HTML issues. (I ran the HTML code through validator)

The HTML markup is the following.

<dl class="result__details" role="list">
  <div class="result__item js-location">
    <svg
      class="result__icon"
      width="14"
      height="20"
      aria-hidden="true"
      focusable="false"
    >
      <use href="/svg/sprite.svg#location"></use>
    </svg>
    <dt class="sr-only">Location:</dt>
    <dd class="result__location js-value">San Francisco</dd>
  </div>
</dl>

The error:

Error: Element svg not allowed as child of element div in this context. (Suppressing further errors from this subtree.)

From line 230, column 15; to line 236, column 15

          <svg↩                class="result__icon"↩                width="14"↩                height="20"↩                aria-hidden="true"↩                focusable="false"↩              >↩     

Content model for element div:
  - If the element is a child of a dl element: one or more dt elements followed by one or more dd elements, optionally intermixed with script-supporting elements.
  - If the element is not a child of a dl element: flow content.

So, the site is still using ul element.

For external links, I don't make those links open in a new window because I simply don't see any benefits to doing it. πŸ˜…

(Based on my research, there are some situations where it is preferable from an accessibility perspective to open a new window or tab.)

For the title, I do it intentionally because I notice the pattern in the "Writing for Web Accessibility" article that the site name is always at the end (e.g. Latest News β€’ Space Teddy Inc., Buy Your Bear (Step 1 of 3) β€’ Space Teddy Inc.).

For the styling, I would try improving the responsiveness of the site using slinky (e.g. clamp, CSS Grid, etc). Currently, I haven't done anything for the styling. But, hopefully, in the near future, the site is much more responsive with fewer media queries (or even without media queries).

About the favicon, I changed the favicon to the search icon with dark blue background. It's a nice little touch for the site! πŸ™Œ

Once again, thank you so much for the feedback. I couldn’t be at this point without your guidance. I can’t thank you enough!

0
P
Graceβ€’ 28,590

@grace-snow

Posted

@vanzasetia yeah you would need to use role presentation on anything that isn’t allowed as a child of an element

0
Vanza Setiaβ€’ 27,795

@vanzasetia

Posted

@grace-snow I have tried adding role="presentation" to the svg and it is still producing the same error.

0
Yazdunβ€’ 1,310

@Yazdun

Posted

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.

3

Vanza Setiaβ€’ 27,795

@vanzasetia

Posted

@Yazdun

Hi Yazdun! Thank you for highlighting the issue! Also, It's good to see your comment on this solution as well! πŸ˜€

I have just fixed the overflowing text issue. So, if you take a look at Matt Seidel's GitHub profile on my website, it is no longer overflowing. 😁

Speaking of which, I find a GitHub user who has the longest text content for each data. I think this user will cause a lot of overflowing issues for people who are doing this challenge (including me). πŸ˜†

2
P
Graceβ€’ 28,590

@grace-snow

Posted

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

1

@Enmanuel-Otero-Montano

Posted

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".

1

Vanza Setiaβ€’ 27,795

@vanzasetia

Posted

@Enmanuel-Otero-Montano

Hi! Thank you for the comment! πŸ‘

Yeah, I also think search input would be a suitable choice (since it is a search form). That is why I chose to use the search input instead of the text input when I was building the project. Good to know you have the same opinion! πŸ˜€

0

@Enmanuel-Otero-Montano

Posted

@vanzasetia Hi!

After I made the comment I kept thinking about what I said about input type="search" and validation and it doesn't really help validation, when I said that I was thinking about **input type="url"**πŸ€¦β€β™‚οΈπŸ˜‚, but what if the input type="search" helps in the user experience, since when the site is viewed from a cell phone the keyboard changes depending on the type of input that has the focus. So I'm still upvoting the input type="search" in this case.

Have a nice day/night Vanza!

1
Vanza Setiaβ€’ 27,795

@vanzasetia

Posted

@Enmanuel-Otero-Montano

Haha! You are right! It doesn't have any validation. But, as you said, it helps the mobile users to get the right keyboard layout which in this case they will get the "Search" button (like this website has shown, http://mobileinputtypes.com/).

Thank you! Have a nice weekend! πŸ˜„

0
Hamzat Lawalβ€’ 560

@EngineerHamziey

Posted

😁 thanks for always making helpful comments @Enmanuel-Otero-Montano and @vanzasetia reading through those comments helps me improve 😘. Concerning that I think one can also use the "inputmode" attribute of the input. Like <input type="text" inputmode="search"> it works for textarea too. There are also other values for the input mode.

I'm not saying input type search is a bad idea though

This 1 minute video explains it well.

1
Hamzat Lawalβ€’ 560

@EngineerHamziey

Posted

@vanzasetia lest I forget, I have sent you a connect request on LinkedIn l, kindly accept it when you're online

0

@Enmanuel-Otero-Montano

Posted

@EngineerHamziey Hello!

I am glad to know that they help you to improve our comments. Regarding the video, it is information that I did not know, but I cannot think of a situation in which it could help me, since there are the native html elements that are adjusted according to the situation. I also believe that the simpler, the better.

Anyway, thank you for the information.

Cheers!

1
Vanza Setiaβ€’ 27,795

@vanzasetia

Posted

@EngineerHamziey Hi Hamzat! πŸ‘‹

You are welcome! I am glad to know that my comments help you improve your front-end skill! πŸ˜„

Thank you for telling me about inputmode attribute! I never heard about this attribute before.

I did quick research about inputmode and found that it is used to give hints about the type of data that might be entered by the user. This can helps mobile users to get the right virtual keyboard layout (the same benefit as using type attribute).

But, unlike input with type attribute, it doesn't have any validations. So, for example, if I have <input inputmode="email"> then it doesn't have any native validation unlike <input type="email">.

In this case, I think your suggestion is fine. But, I would stick to what I already did (which is by using <input type="search">) because the users require to input a search query. ("Inputs that require a search query should typically use <input type="search"> instead.", from MDN documentation).

Lastly, about the LinkedIn invitation, I checked the "Invitation" and I didn't see your invitation (my invitation was empty). So, you may want to try to create a new invitation. πŸ™‚

2

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